bugfix: add a circ->resolving_streams field, and make dns resolving streams not actually in the connection_array until they're done resolving.

bugfix: actually complain if we duplicate mark-for-close a circuit

add more logging for relay ends that claim dns resolve failed, so we can
find out why they're not being retried.


svn:r1798

10 files changed, 144 insertions, 71 deletions

diff --git a/src/or/circuit.c b/src/or/circuit.c
index 5987019488..7643f15df0 100644
--- a/src/or/circuit.c
+++ b/src/or/circuit.c
@@ -227,6 +227,10 @@ circuit_t *circuit_get_by_circ_id_conn(uint16_t circ_id, connection_t *conn) {
         if(tmpconn == conn)
           return circ;
       }
+      for(tmpconn = circ->resolving_streams; tmpconn; tmpconn = tmpconn->next_stream) {
+        if(tmpconn == conn)
+          return circ;
+      }
     }
   }
   return NULL;
@@ -250,6 +254,9 @@ circuit_t *circuit_get_by_conn(connection_t *conn) {
     for(tmpconn = circ->n_streams; tmpconn; tmpconn=tmpconn->next_stream)
       if(tmpconn == conn)
         return circ;
+    for(tmpconn = circ->resolving_streams; tmpconn; tmpconn=tmpconn->next_stream)
+      if(tmpconn == conn)
+        return circ;
   }
   return NULL;
 }
@@ -905,6 +912,12 @@ relay_lookup_conn(circuit_t *circ, cell_t *cell, int cell_direction)
       return tmpconn;
     }
   }
+  for(tmpconn = circ->resolving_streams; tmpconn; tmpconn=tmpconn->next_stream) {
+    if(rh.stream_id == tmpconn->stream_id) {
+      log_fn(LOG_DEBUG,"found conn for stream %d.", rh.stream_id);
+      return tmpconn;
+    }
+  }
   return NULL; /* probably a begin relay cell */
 }
 
@@ -985,7 +998,7 @@ int _circuit_mark_for_close(circuit_t *circ) {
   connection_t *conn;
 
   assert_circuit_ok(circ);
-  if (circ->marked_for_close < 0)
+  if (circ->marked_for_close)
     return -1;
 
   if(circ->state == CIRCUIT_STATE_ONIONSKIN_PENDING) {
@@ -1010,14 +1023,18 @@ int _circuit_mark_for_close(circuit_t *circ) {
 
   if(circ->n_conn)
     connection_send_destroy(circ->n_circ_id, circ->n_conn);
-  for(conn=circ->n_streams; conn; conn=conn->next_stream) {
+  for(conn=circ->n_streams; conn; conn=conn->next_stream)
     connection_edge_destroy(circ->n_circ_id, conn);
+  while(circ->resolving_streams) {
+    conn = circ->resolving_streams;
+    circ->resolving_streams = conn->next_stream;
+    log_fn(LOG_INFO,"Freeing resolving-conn.");
+    connection_free(conn);
   }
   if(circ->p_conn)
     connection_send_destroy(circ->p_circ_id, circ->p_conn);
-  for(conn=circ->p_streams; conn; conn=conn->next_stream) {
+  for(conn=circ->p_streams; conn; conn=conn->next_stream)
     connection_edge_destroy(circ->p_circ_id, conn);
-  }
 
   circ->marked_for_close = 1;
 
@@ -1042,16 +1059,38 @@ void circuit_detach_stream(circuit_t *circ, connection_t *conn) {
     circ->n_streams = conn->next_stream;
     return;
   }
-  for(prevconn = circ->p_streams; prevconn && prevconn->next_stream && prevconn->next_stream != conn; prevconn = prevconn->next_stream) ;
+  if(conn == circ->resolving_streams) {
+    circ->resolving_streams = conn->next_stream;
+    return;
+  }
+
+  for(prevconn = circ->p_streams;
+      prevconn && prevconn->next_stream && prevconn->next_stream != conn;
+      prevconn = prevconn->next_stream)
+    ;
   if(prevconn && prevconn->next_stream) {
     prevconn->next_stream = conn->next_stream;
     return;
   }
-  for(prevconn = circ->n_streams; prevconn && prevconn->next_stream && prevconn->next_stream != conn; prevconn = prevconn->next_stream) ;
+
+  for(prevconn = circ->n_streams;
+      prevconn && prevconn->next_stream && prevconn->next_stream != conn;
+      prevconn = prevconn->next_stream)
+    ;
   if(prevconn && prevconn->next_stream) {
     prevconn->next_stream = conn->next_stream;
     return;
   }
+
+  for(prevconn = circ->resolving_streams;
+      prevconn && prevconn->next_stream && prevconn->next_stream != conn;
+      prevconn = prevconn->next_stream)
+    ;
+  if(prevconn && prevconn->next_stream) {
+    prevconn->next_stream = conn->next_stream;
+    return;
+  }
+
   log_fn(LOG_ERR,"edge conn not in circuit's list?");
   tor_assert(0); /* should never get here */
 }
diff --git a/src/or/connection.c b/src/or/connection.c
index c163cd8636..0b4e88dd30 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -79,6 +79,7 @@ connection_t *connection_new(int type) {
   conn = tor_malloc_zero(sizeof(connection_t));
   conn->magic = CONNECTION_MAGIC;
   conn->s = -1; /* give it a default of 'not used' */
+  conn->poll_index = -1; /* also default to 'not used' */
 
   conn->type = type;
   if(!connection_is_listener(conn)) { /* listeners never use their buf */
@@ -631,8 +632,8 @@ int connection_read_to_buf(connection_t *conn) {
     switch(result) {
       case TOR_TLS_ERROR:
       case TOR_TLS_CLOSE:
-        log_fn(LOG_INFO,"tls error. breaking (nickname %s).",
-               conn->nickname ? conn->nickname : "not set");
+        log_fn(LOG_INFO,"tls error. breaking (nickname %s, address %s).",
+               conn->nickname ? conn->nickname : "not set", conn->address);
         return -1; /* XXX deal with close better */
       case TOR_TLS_WANTWRITE:
         connection_start_writing(conn);
@@ -864,7 +865,7 @@ connection_t *connection_get_by_type_state_lastwritten(int type, int state) {
   return best;
 }
 
-connection_t *connection_get_by_type_rendquery(int type, char *rendquery) {
+connection_t *connection_get_by_type_rendquery(int type, const char *rendquery) {
   int i, n;
   connection_t *conn;
   connection_t **carray;
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index 38f8f90059..b07fcf391d 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -1,4 +1,4 @@
-/* Copyright 2001,2002,2003 Roger Dingledine, Matej Pfajfar. */
+/* Copyright 2001 Matej Pfajfar, 2001-2004 Roger Dingledine. */
 /* See LICENSE for licensing information */
 /* $Id$ */
 
@@ -165,10 +165,10 @@ int connection_edge_end(connection_t *conn, char reason, crypt_path_t *cpath_lay
 }
 
 /* Make a relay cell out of 'relay_command' and 'payload', and
- * send it onto the open circuit 'circ'. If it's a control cell,
- * set fromconn to NULL, else it's the stream that's sending the
- * relay cell. Use cpath_layer NULL if you're responding to the OP;
- * If it's an outgoing cell it must specify the destination hop.
+ * send it onto the open circuit 'circ'. 'fromconn' is the stream
+ * that's sending the relay cell, or NULL if it's a control cell,
+ * 'cpath_layer' is NULL for OR->OP cells, or the destination hop
+ * for OP->OR cells.
  *
  * If you can't send the cell, mark the circuit for close and
  * return -1. Else return 0.
@@ -229,12 +229,15 @@ int connection_edge_process_relay_cell_not_open(
     reason = *(cell->payload+RELAY_HEADER_SIZE);
     /* We have to check this here, since we aren't connected yet. */
     if (rh->length >= 5 && reason == END_STREAM_REASON_EXITPOLICY) {
+      log_fn(LOG_INFO,"Address %s refused due to exit policy. Retrying.",
+             conn->socks_request->address);
       addr = ntohl(get_uint32(cell->payload+RELAY_HEADER_SIZE+1));
       client_dns_set_entry(conn->socks_request->address, addr);
       conn->state = AP_CONN_STATE_CIRCUIT_WAIT;
       circuit_detach_stream(circ,conn);
       if(connection_ap_handshake_attach_circuit(conn) >= 0)
         return 0;
+      log_fn(LOG_INFO,"Giving up on retrying (from exitpolicy); conn can't be handled.");
       /* else, conn will get closed below */
     } else if (rh->length && reason == END_STREAM_REASON_RESOLVEFAILED) {
       if (client_dns_incr_failures(conn->socks_request->address)
@@ -245,6 +248,10 @@ int connection_edge_process_relay_cell_not_open(
         if(connection_ap_handshake_attach_circuit(conn) >= 0)
           return 0;
         /* else, conn will get closed below */
+        log_fn(LOG_INFO,"Giving up on retrying (from resolvefailed); conn can't be handled.");
+      } else {
+        log_fn(LOG_WARN,"Have tried resolving address %s at %d different places. Giving up.",
+               conn->socks_request->address, MAX_RESOLVE_FAILURES);
       }
     }
     log_fn(LOG_INFO,"Edge got end (%s) before we're connected. Marking for close.",
@@ -253,8 +260,6 @@ int connection_edge_process_relay_cell_not_open(
       circuit_log_path(LOG_INFO,circ);
     conn->has_sent_end = 1; /* we just got an 'end', don't need to send one */
     connection_mark_for_close(conn, 0);
-    /* XXX here we should send a socks reject back if necessary, and hold
-     * open til flushed */
     return 0;
   }
 
@@ -280,12 +285,12 @@ int connection_edge_process_relay_cell_not_open(
       return 0;
     }
     return 0;
-  } else {
-    log_fn(LOG_WARN,"Got an unexpected relay command %d, in state %d (%s). Closing.",
-           rh->command, conn->state, conn_state_to_string[conn->type][conn->state]);
-    connection_mark_for_close(conn, END_STREAM_REASON_MISC);
-    return -1;
   }
+
+  log_fn(LOG_WARN,"Got an unexpected relay command %d, in state %d (%s). Closing.",
+         rh->command, conn->state, conn_state_to_string[conn->type][conn->state]);
+  connection_mark_for_close(conn, END_STREAM_REASON_MISC);
+  return -1;
 }
 
 /* an incoming relay cell has arrived. return -1 if you want to tear down the
@@ -893,7 +898,7 @@ int connection_ap_handshake_attach_circuit(connection_t *conn) {
   if(conn_age > 60) {
     /* XXX make this cleaner than '60' */
     log_fn(LOG_WARN,"Giving up on unattached conn (%d sec old).", conn_age);
-    connection_mark_for_close(conn, 0);
+    return -1;
   }
 
   if(!connection_edge_is_rendezvous_stream(conn)) { /* we're a general conn */
@@ -1180,13 +1185,6 @@ static int connection_exit_begin_conn(cell_t *cell, circuit_t *circ) {
   n_stream->package_window = STREAMWINDOW_START;
   n_stream->deliver_window = STREAMWINDOW_START;
 
-  log_fn(LOG_DEBUG,"finished adding conn");
-
-  /* add it into the linked list of streams on this circuit */
-  n_stream->next_stream = circ->n_streams;
-  circ->n_streams = n_stream;
-  assert_circuit_ok(circ);
-
   if(circ->purpose == CIRCUIT_PURPOSE_S_REND_JOINED) {
     log_fn(LOG_DEBUG,"begin is for rendezvous. configuring stream.");
     n_stream->address = tor_strdup("(rendezvous)");
@@ -1204,6 +1202,12 @@ static int connection_exit_begin_conn(cell_t *cell, circuit_t *circ) {
     assert_circuit_ok(circ);
     log_fn(LOG_DEBUG,"Finished assigning addr/port");
     n_stream->cpath_layer = circ->cpath->prev; /* link it */
+
+    /* add it into the linked list of n_streams on this circuit */
+    n_stream->next_stream = circ->n_streams;
+    circ->n_streams = n_stream;
+    assert_circuit_ok(circ);
+
     connection_exit_connect(n_stream);
     return 0;
   }
@@ -1214,6 +1218,12 @@ static int connection_exit_begin_conn(cell_t *cell, circuit_t *circ) {
   /* send it off to the gethostbyname farm */
   switch(dns_resolve(n_stream)) {
     case 1: /* resolve worked */
+
+      /* add it into the linked list of n_streams on this circuit */
+      n_stream->next_stream = circ->n_streams;
+      circ->n_streams = n_stream;
+      assert_circuit_ok(circ);
+
       connection_exit_connect(n_stream);
       return 0;
     case -1: /* resolve failed */
@@ -1222,6 +1232,10 @@ static int connection_exit_begin_conn(cell_t *cell, circuit_t *circ) {
       connection_free(n_stream);
       break;
     case 0: /* resolve added to pending list */
+      /* add it into the linked list of resolving_streams on this circuit */
+      n_stream->next_stream = circ->resolving_streams;
+      circ->resolving_streams = n_stream;
+      assert_circuit_ok(circ);
       ;
   }
   return 0;
@@ -1234,6 +1248,8 @@ void connection_exit_connect(connection_t *conn) {
       router_compare_to_my_exit_policy(conn) == ADDR_POLICY_REJECTED) {
     log_fn(LOG_INFO,"%s:%d failed exit policy. Closing.", conn->address, conn->port);
     connection_mark_for_close(conn, END_STREAM_REASON_EXITPOLICY);
+    circuit_detach_stream(circuit_get_by_conn(conn), conn);
+    connection_free(conn);
     return;
   }
 
@@ -1241,6 +1257,7 @@ void connection_exit_connect(connection_t *conn) {
   switch(connection_connect(conn, conn->address, conn->addr, conn->port)) {
     case -1:
       connection_mark_for_close(conn, END_STREAM_REASON_CONNECTFAILED);
+      circuit_detach_stream(circuit_get_by_conn(conn), conn);
       connection_free(conn);
       return;
     case 0:
@@ -1368,7 +1385,10 @@ static int client_dns_incr_failures(const char *address)
 {
   struct client_dns_entry *ent;
   ent = _get_or_create_ent(address);
-  return ++ent->n_failures;
+  ++ent->n_failures;
+  log_fn(LOG_DEBUG,"Address %s now has %d resolve failures.",
+         address, ent->n_failures);
+  return ent->n_failures;
 }
 
 static void client_dns_set_entry(const char *address, uint32_t val)
diff --git a/src/or/cpuworker.c b/src/or/cpuworker.c
index 1138fd8969..f872d14195 100644
--- a/src/or/cpuworker.c
+++ b/src/or/cpuworker.c
@@ -167,7 +167,7 @@ done_processing:
  * Read and writes from fdarray[1].  Reads requests, writes answers.
  *
  *   Request format:
- *          Task type           [1 byte, always ONIONSKIN_CHALLENGE_LEN]
+ *          Task type           [1 byte, always CPUWORKER_TASK_ONION]
  *          Opaque tag          TAG_LEN
  *          Onionskin challenge ONIONSKIN_CHALLENGE_LEN
  *   Response format:
diff --git a/src/or/directory.c b/src/or/directory.c
index 5e9c399b3a..9f0014195c 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -221,8 +221,8 @@ int connection_dir_process_inbuf(connection_t *conn) {
 
   /* Directory clients write, then read data until they receive EOF;
    * directory servers read data until they get an HTTP command, then
-   * write their response, mark the conn for close (?) and hold the
-   * conn open till it's flushed. (??)XXXXNMNM
+   * write their response (when it's finished flushing, they mark for
+   * close).
    */
   if(conn->inbuf_reached_eof) {
     if(conn->state != DIR_CONN_STATE_CLIENT_READING) {
@@ -360,10 +360,10 @@ static char answer404[] = "HTTP/1.0 404 Not found\r\n\r\n";
 static char answer503[] = "HTTP/1.0 503 Directory unavailable\r\n\r\n";
 
 /* Helper function: called when a dirserver gets a complete HTTP GET
- * request.  Looks for a request for a directory or for a rendezvous
- * service descriptor.  On finding one, writes a response into
+ * request.  Look for a request for a directory or for a rendezvous
+ * service descriptor.  On finding one, write a response into
  * conn->outbuf.  If the request is unrecognized, send a 404.
- * Always returns 0. */
+ * Always return 0. */
 static int directory_handle_command_get(connection_t *conn,
                                         char *headers, char *body,
                                         int body_len) {
@@ -425,11 +425,11 @@ static int directory_handle_command_get(connection_t *conn,
   return 0;
 }
 
-/* Helper function: called when a dirserver gets a complete HTTP GET
- * request.  Looks for an uploaded server descriptor or rendezvous
- * service descriptor.  On finding one, processes it and writes a
- * response into conn->outbuf.  If the request is unrecognized, sends a
- * 404.  Always returns 0. */
+/* Helper function: called when a dirserver gets a complete HTTP POST
+ * request.  Look for an uploaded server descriptor or rendezvous
+ * service descriptor.  On finding one, process it and write a
+ * response into conn->outbuf.  If the request is unrecognized, send a
+ * 404.  Always return 0. */
 static int directory_handle_command_post(connection_t *conn,
                                          char *headers, char *body,
                                          int body_len) {
@@ -519,7 +519,7 @@ static int directory_handle_command(connection_t *conn) {
 }
 
 /* Write handler for directory connections; called when all data has
- * been flushed.  Handle a completed connection; close the connection,
+ * been flushed.  Handle a completed connection: close the connection
  * or wait for a response as appropriate.
  */
 int connection_dir_finished_flushing(connection_t *conn) {
diff --git a/src/or/dns.c b/src/or/dns.c
index 41972ad3aa..0c3d77edc2 100644
--- a/src/or/dns.c
+++ b/src/or/dns.c
@@ -384,6 +384,7 @@ static void dns_found_answer(char *address, uint32_t addr, char outcome) {
   struct cached_resolve search;
   struct cached_resolve *resolve;
   connection_t *pendconn;
+  circuit_t *circ;
 
   strncpy(search.address, address, MAX_ADDRESSLEN);
   search.address[MAX_ADDRESSLEN-1] = 0;
@@ -426,11 +427,21 @@ static void dns_found_answer(char *address, uint32_t addr, char outcome) {
       pendconn = pend->conn; /* don't pass complex things to the
                                 connection_mark_for_close macro */
       /* prevent double-remove. */
-      pend->conn->state = EXIT_CONN_STATE_RESOLVEFAILED;
+      pendconn->state = EXIT_CONN_STATE_RESOLVEFAILED;
       connection_mark_for_close(pendconn, END_STREAM_REASON_RESOLVEFAILED);
+      connection_free(pendconn);
     } else {
       /* prevent double-remove. */
       pend->conn->state = EXIT_CONN_STATE_CONNECTING;
+
+      circ = circuit_get_by_conn(pend->conn);
+      assert(circ);
+      /* unlink pend->conn from resolving_streams, */
+      circuit_detach_stream(circ, pend->conn);
+      /* and link it to n_streams */
+      pend->conn->next_stream = circ->n_streams;
+      circ->n_streams = pend->conn;
+
       connection_exit_connect(pend->conn);
     }
     resolve->pending_connections = pend->next;
diff --git a/src/or/main.c b/src/or/main.c
index e21b95d064..17000ab792 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -77,11 +77,13 @@ int connection_add(connection_t *conn) {
     return -1;
   }
 
+  tor_assert(conn->poll_index == -1); /* can only connection_add once */
   conn->poll_index = nfds;
   connection_array[nfds] = conn;
 
-  /* zero these out here, because otherwise we'll inherit values from the previously freed one */
   poll_array[nfds].fd = conn->s;
+
+  /* zero these out here, because otherwise we'll inherit values from the previously freed one */
   poll_array[nfds].events = 0;
   poll_array[nfds].revents = 0;
 
@@ -105,12 +107,8 @@ int connection_remove(connection_t *conn) {
 
   log_fn(LOG_INFO,"removing socket %d (type %s), nfds now %d",
          conn->s, CONN_TYPE_TO_STRING(conn->type), nfds-1);
-  /* if it's an edge conn, remove it from the list
-   * of conn's on this circuit. If it's not on an edge,
-   * flush and send destroys for all circuits on this conn
-   */
-  circuit_about_to_close_connection(conn);
 
+  tor_assert(conn->poll_index >= 0);
   current_index = conn->poll_index;
   if(current_index == nfds-1) { /* this is the end */
     nfds--;
@@ -141,20 +139,20 @@ void get_connection_array(connection_t ***array, int *n) {
  */
 void connection_watch_events(connection_t *conn, short events) {
 
-  tor_assert(conn && conn->poll_index < nfds);
+  tor_assert(conn && conn->poll_index >= 0 && conn->poll_index < nfds);
 
   poll_array[conn->poll_index].events = events;
 }
 
 /* Return true iff the 'conn' is listening for read events. */
 int connection_is_reading(connection_t *conn) {
+  tor_assert(conn && conn->poll_index >= 0);
   return poll_array[conn->poll_index].events & POLLIN;
 }
 
 /* Tell the main loop to stop notifying 'conn' of any read events. */
 void connection_stop_reading(connection_t *conn) {
-
-  tor_assert(conn && conn->poll_index < nfds);
+  tor_assert(conn && conn->poll_index >= 0 && conn->poll_index < nfds);
 
   log(LOG_DEBUG,"connection_stop_reading() called.");
   if(poll_array[conn->poll_index].events & POLLIN)
@@ -163,9 +161,7 @@ void connection_stop_reading(connection_t *conn) {
 
 /* Tell the main loop to start notifying 'conn' of any read events. */
 void connection_start_reading(connection_t *conn) {
-
-  tor_assert(conn && conn->poll_index < nfds);
-
+  tor_assert(conn && conn->poll_index >= 0 && conn->poll_index < nfds);
   poll_array[conn->poll_index].events |= POLLIN;
 }
 
@@ -177,7 +173,7 @@ int connection_is_writing(connection_t *conn) {
 /* Tell the main loop to stop notifying 'conn' of any write events. */
 void connection_stop_writing(connection_t *conn) {
 
-  tor_assert(conn && conn->poll_index < nfds);
+  tor_assert(conn && conn->poll_index >= 0 && conn->poll_index < nfds);
 
   if(poll_array[conn->poll_index].events & POLLOUT)
     poll_array[conn->poll_index].events -= POLLOUT;
@@ -185,15 +181,13 @@ void connection_stop_writing(connection_t *conn) {
 
 /* Tell the main loop to start notifying 'conn' of any write events. */
 void connection_start_writing(connection_t *conn) {
-
-  tor_assert(conn && conn->poll_index < nfds);
-
+  tor_assert(conn && conn->poll_index >= 0 && conn->poll_index < nfds);
   poll_array[conn->poll_index].events |= POLLOUT;
 }
 
-/* Called when the connection at connection_array[i] has a read event:
- * checks for validity, catches numerous errors, and dispatches to
- * connection_handle_read.
+/* Called when the connection at connection_array[i] has a read event,
+ * or it has pending tls data waiting to be read: checks for validity,
+ * catches numerous errors, and dispatches to connection_handle_read.
  */
 static void conn_read(int i) {
   connection_t *conn = connection_array[i];
@@ -314,6 +308,11 @@ static void conn_close_if_marked(int i) {
              conn->marked_for_close);
     }
   }
+  /* if it's an edge conn, remove it from the list
+   * of conn's on this circuit. If it's not on an edge,
+   * flush and send destroys for all circuits on this conn
+   */
+  circuit_about_to_close_connection(conn);
   connection_remove(conn);
   if(conn->type == CONN_TYPE_EXIT) {
     assert_connection_edge_not_dns_pending(conn);
diff --git a/src/or/or.h b/src/or/or.h
index 50ec98e8a2..124ed89741 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -528,6 +528,7 @@ struct circuit_t {
   connection_t *n_conn; /* for the OR conn, if there is one */
   connection_t *p_streams;
   connection_t *n_streams;
+  connection_t *resolving_streams;
   uint16_t next_stream_id;
   int package_window;
   int deliver_window;
@@ -812,7 +813,7 @@ connection_t *connection_exact_get_by_addr_port(uint32_t addr, uint16_t port);
 connection_t *connection_get_by_type(int type);
 connection_t *connection_get_by_type_state(int type, int state);
 connection_t *connection_get_by_type_state_lastwritten(int type, int state);
-connection_t *connection_get_by_type_rendquery(int type, char *rendquery);
+connection_t *connection_get_by_type_rendquery(int type, const char *rendquery);
 
 #define connection_speaks_cells(conn) ((conn)->type == CONN_TYPE_OR)
 #define connection_has_pending_tls_data(conn) \
diff --git a/src/or/router.c b/src/or/router.c
index dbbe73196f..fedaa13607 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -389,7 +389,8 @@ static void router_add_exit_policy_from_config_helper(const char *s, routerinfo_
  */
 static void router_add_exit_policy_from_config(routerinfo_t *router) {
   router_add_exit_policy_from_config_helper(options.ExitPolicy, router);
-  /* XXXX This is wrong; you can spell *;* many ways. */
+  /* XXXX This is wrong; you can spell *:* many ways. -NM
+   * So? If they spell it sneakily, then their exit policy is bulkier. -RD */
   if(strstr(options.ExitPolicy," *:*") == NULL) {
     /* if exitpolicy includes a *:* line, then we're done. Else, append
      * the default exitpolicy. */
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 61a6ecd892..8ac46c30c1 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -62,10 +62,10 @@ typedef enum {
 typedef struct directory_token_t {
   directory_keyword tp;        /* Type of the token. */
   int n_args;                  /* Number of elements in args */
-  char **args;                 /* Array of aguments from keyword line. */
+  char **args;                 /* Array of arguments from keyword line. */
   char *object_type;           /* -----BEGIN [object_type]-----*/
-  int object_size;             /* Bytes in object_boody */
-  char *object_body;           /* Contents of object, base65-decoded. */
+  int object_size;             /* Bytes in object_body */
+  char *object_body;           /* Contents of object, base64-decoded. */
   crypto_pk_env_t *key;        /* For public keys only. */
   char *error;                 /* For _ERR tokens only. */
 } directory_token_t;
@@ -85,11 +85,11 @@ typedef enum {
 typedef enum {
   NO_OBJ,      /*    (1) no object, ever */
   NEED_OBJ,    /*    (2) object is required */
-  NEED_KEY,    /*    (3) object is requierd, and must be a public key. */
+  NEED_KEY,    /*    (3) object is required, and must be a public key. */
   OBJ_OK,      /* or (4) object is optional. */
 } obj_syntax;
 
-/* Rules for where a keyword can apper. */
+/* Rules for where a keyword can appear. */
 typedef enum {
   ANY = 0,    /* Appears in router descriptor or in directory sections. */
   DIR_ONLY,   /* Appears only in directory. */
@@ -208,7 +208,7 @@ static routerinfo_t *router_pick_directory_server_impl(void) {
   log_fn(LOG_INFO,"No dirservers are reachable. Trying them all again.");
 
   /* No running dir servers found? go through and mark them all as up,
-   * and next time, we'll cycle through the list again. */
+   * so we cycle through the list again. */
   for(i=0; i < smartlist_len(routerlist->routers); i++) {
     router = smartlist_get(routerlist->routers, i);
     if(router->dir_port > 0) {
@@ -587,7 +587,7 @@ router_resolve_routerlist(routerlist_t *rl)
  * don't know the IP of the target address.
  *
  * Returns -1 for 'rejected', 0 for accepted, 1 for 'maybe' (since IP is
- * unknown.
+ * unknown).
  */
 int router_compare_addr_to_exit_policy(uint32_t addr, uint16_t port,
                                        struct exit_policy_t *policy)
@@ -674,6 +674,7 @@ int router_exit_policy_rejects_all(routerinfo_t *router) {
 /* Parse a date of the format 'YYYY-MM-DD hh:mm:ss" and store the result into
  * *t.
  */
+/* XXX this should go in util.c, yes? -RD */
 static int parse_time(const char *cp, time_t *t)
 {
   struct tm st_tm;