Add a few more evmap/changelist comments

changelist-internal.h

@@ -88,7 +88,7 @@ void event_changelist_init(struct event_changelist *changelist);
  * after making all the changes in the changelist. */
 void event_changelist_remove_all(struct event_changelist *changelist,
     struct event_base *base);
-/** Free all memory held in a changelist, and return it. */
+/** Free all memory held in a changelist. */
 void event_changelist_freemem(struct event_changelist *changelist);
 
 /** Implementation of eventop_add that queues the event in a changelist. */

evmap-internal.h

@@ -50,34 +50,40 @@ void evmap_io_clear(struct event_io_map* ctx);
 void evmap_signal_clear(struct event_signal_map* ctx);
 
 /** Add an IO event (some combination of EV_READ or EV_WRITE) to an
-	event_base's list of events on a given file descriptor, and tell the
+    event_base's list of events on a given file descriptor, and tell the
-	underlying eventops about the fd if its state has changed.
+    underlying eventops about the fd if its state has changed.
 
-	@param base the event_base to operate on.
+    Requires that ev is not already added.
-	@param fd the file descriptor corresponding to ev.
+
-	@param ev the event to add.
+    @param base the event_base to operate on.
- */
+    @param fd the file descriptor corresponding to ev.
+    @param ev the event to add.
+*/
 int evmap_io_add(struct event_base *base, evutil_socket_t fd, struct event *ev);
 /** Remove an IO event (some combination of EV_READ or EV_WRITE) to an
-	event_base's list of events on a given file descriptor, and tell the
+    event_base's list of events on a given file descriptor, and tell the
-	underlying eventops about the fd if its state has changed.
+    underlying eventops about the fd if its state has changed.
 
-	@param base the event_base to operate on.
+    @param base the event_base to operate on.
-	@param fd the file descriptor corresponding to ev.
+    @param fd the file descriptor corresponding to ev.
-	@param ev the event to remove.
+    @param ev the event to remove.
  */
 int evmap_io_del(struct event_base *base, evutil_socket_t fd, struct event *ev);
 /** Active the set of events waiting on an event_base for a given fd.
 
-	@param base the event_base to operate on.
+    @param base the event_base to operate on.
-	@param fd the file descriptor that has become active.
+    @param fd the file descriptor that has become active.
-	@param events a bitmask of EV_READ|EV_WRITE|EV_ET.
+    @param events a bitmask of EV_READ|EV_WRITE|EV_ET.
- */
+*/
 void evmap_io_active(struct event_base *base, evutil_socket_t fd, short events);
 
+
+/* These functions behave in the same way as evmap_io_*, except they work on
+ * signals rather than fds.  signals use a linear map everywhere; fds use
+ * either a linear map or a hashtable. */
 int evmap_signal_add(struct event_base *base, int signum, struct event *ev);
 int evmap_signal_del(struct event_base *base, int signum, struct event *ev);
-void evmap_signal_active(struct event_base *base, evutil_socket_t fd, int ncalls);
+void evmap_signal_active(struct event_base *base, evutil_socket_t signum, int ncalls);
 
 void *evmap_io_get_fdinfo(struct event_io_map *ctx, evutil_socket_t fd);
 

evmap.c

@@ -83,6 +83,8 @@ struct event_map_entry {
 	} ent;
 };
 
+/* Helper used by the event_io_map hashtable code; tries to return a good hash
+ * of the fd in e->fd. */
 static inline unsigned
 hashsocket(struct event_map_entry *e)
 {
@@ -94,6 +96,8 @@ hashsocket(struct event_map_entry *e)
 	return h;
 }
 
+/* Helper used by the event_io_map hashtable code; returns true iff e1 and e2
+ * have the same e->fd. */
 static inline int
 eqsocket(struct event_map_entry *e1, struct event_map_entry *e2)
 {
@@ -261,9 +265,7 @@ evmap_io_add(struct event_base *base, evutil_socket_t fd, struct event *ev)
 	int nread, nwrite, retval = 0;
 	short res = 0, old = 0;
 
-	EVUTIL_ASSERT(fd == ev->ev_fd); /*XXX(nickm) always true? */
+	EVUTIL_ASSERT(fd == ev->ev_fd);
-	/*XXX(nickm) Should we assert that ev is not already inserted, or should
-	 * we make this function idempotent? */
 
 	if (fd < 0)
 		return 0;
@@ -300,7 +302,7 @@ evmap_io_add(struct event_base *base, evutil_socket_t fd, struct event *ev)
 		 * level-triggered, we should probably assert on
 		 * this. */
 		if (evsel->add(base, ev->ev_fd,
-					   old, (ev->ev_events & EV_ET) | res, extra) == -1)
+			old, (ev->ev_events & EV_ET) | res, extra) == -1)
 			return (-1);
 		retval = 1;
 	}
@@ -326,9 +328,7 @@ evmap_io_del(struct event_base *base, evutil_socket_t fd, struct event *ev)
 	if (fd < 0)
 		return 0;
 
-	EVUTIL_ASSERT(fd == ev->ev_fd); /*XXX(nickm) always true? */
+	EVUTIL_ASSERT(fd == ev->ev_fd);
-	/*XXX(nickm) Should we assert that ev is not already inserted, or should
-	 * we make this function idempotent? */
 
 #ifndef EVMAP_USE_HT
 	if (fd >= io->nentries)
@@ -674,11 +674,12 @@ event_changelist_del(struct event_base *base, evutil_socket_t fd, short old, sho
 
 	/* A delete removes any previous add, rather than replacing it:
 	   on those platforms where "add, delete, dispatch" is not the same
-	   as "no-op" dispatch, we want the no-op behavior.
+	   as "no-op, dispatch", we want the no-op behavior.
 
-	   If we have a no-op item, we could it from the list entirely, but
+	   If we have a no-op item, we could remove it it from the list
-	   really there's not much point: skipping the no-op change when we do
+	   entirely, but really there's not much point: skipping the no-op
-	   the dispatch later is far cheaper than rejuggling the array now.
+	   change when we do the dispatch later is far cheaper than rejuggling
+	   the array now.
 	 */
 
 	if (events & (EV_READ|EV_SIGNAL)) {