Commit fixes for several pending tor core tasks: document all DOCDOCed functions; time out uncontrolled unattached streams; feed reasons to SOCKS5 (refactoring connection_ap_handshake_socks_reply in the process); change DirFetchPeriod/StatusFetchPeriod to have a special "Be smart" value.

svn:r3769

12 files changed, 164 insertions, 72 deletions

diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index 4d1af4b591..6f812cb6b1 100644
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -183,7 +183,8 @@ circuit_free_cpath_node(crypt_path_t *victim) {
   tor_free(victim);
 }
 
-/** DOCDOC **/
+/** Return the circuit whose global ID is <b>id</b>, or NULL if no
+ * such circuit exists. */
 circuit_t *
 circuit_get_by_global_id(uint32_t id)
 {
diff --git a/src/or/circuituse.c b/src/or/circuituse.c
index 7eb5f597c4..c91680dc87 100644
--- a/src/or/circuituse.c
+++ b/src/or/circuituse.c
@@ -975,7 +975,9 @@ consider_recording_trackhost(connection_t *conn, circuit_t *circ) {
                       time(NULL) + options->TrackHostExitsExpire);
 }
 
-/* DOCDOC. Return as for connection_ap_handshake_attach_chosen_circuit. */
+/** Attempt to attach the connection <b>conn</b> to <b>circ</b>, and
+ * send a begin or resolve cell as appropriate.  Return values for
+ * connection_ap_handshake_attach_chosen_circuit. */
 int
 connection_ap_handshake_attach_chosen_circuit(connection_t *conn,
                                               circuit_t *circ)
diff --git a/src/or/config.c b/src/or/config.c
index d822f39141..09e6183e40 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -108,9 +108,8 @@ static config_var_t config_vars[] = {
   VAR("DirAllowPrivateAddresses",BOOL, DirAllowPrivateAddresses, NULL),
   VAR("DirPort",             UINT,     DirPort,              "0"),
   VAR("DirBindAddress",      LINELIST, DirBindAddress,       NULL),
-/* XXX we'd like dirfetchperiod to be higher for people with dirport not
- * set, but low for people with dirport set. how to have two defaults? */
-  VAR("DirFetchPeriod",      INTERVAL, DirFetchPeriod,       "1 hour"),
+  /** DOCDOC **/
+  VAR("DirFetchPeriod",      INTERVAL, DirFetchPeriod,       "0 seconds"),
   VAR("DirPostPeriod",       INTERVAL, DirPostPeriod,        "20 minutes"),
   VAR("RendPostPeriod",      INTERVAL, RendPostPeriod,       "20 minutes"),
   VAR("DirPolicy",           LINELIST, DirPolicy,            NULL),
@@ -169,9 +168,8 @@ static config_var_t config_vars[] = {
   VAR("SocksPort",           UINT,     SocksPort,            "9050"),
   VAR("SocksBindAddress",    LINELIST, SocksBindAddress,     NULL),
   VAR("SocksPolicy",         LINELIST, SocksPolicy,          NULL),
-/* XXX as with dirfetchperiod, we want this to be 15 minutes for people
- * with a dirport open, but higher for people without a dirport open. */
-  VAR("StatusFetchPeriod",   INTERVAL, StatusFetchPeriod,    "15 minutes"),
+  /** DOCDOC */
+  VAR("StatusFetchPeriod",   INTERVAL, StatusFetchPeriod,    "0 seconds"),
   VAR("SysLog",              LINELIST_S, OldLogOptions,      NULL),
   OBSOLETE("TrafficShaping"),
   VAR("User",                STRING,   User,                 NULL),
@@ -1375,11 +1373,13 @@ options_validate(or_options_t *options)
 #define MAX_CACHE_DIR_FETCH_PERIOD 3600
 #define MAX_CACHE_STATUS_FETCH_PERIOD 900
 
-  if (options->DirFetchPeriod < MIN_DIR_FETCH_PERIOD) {
+  if (options->DirFetchPeriod &&
+      options->DirFetchPeriod < MIN_DIR_FETCH_PERIOD) {
     log(LOG_WARN, "DirFetchPeriod option must be at least %d seconds. Clipping.", MIN_DIR_FETCH_PERIOD);
     options->DirFetchPeriod = MIN_DIR_FETCH_PERIOD;
   }
-  if (options->StatusFetchPeriod < MIN_STATUS_FETCH_PERIOD) {
+  if (options->StatusFetchPeriod &&
+      options->StatusFetchPeriod < MIN_STATUS_FETCH_PERIOD) {
     log(LOG_WARN, "StatusFetchPeriod option must be at least %d seconds. Clipping.", MIN_STATUS_FETCH_PERIOD);
     options->StatusFetchPeriod = MIN_STATUS_FETCH_PERIOD;
   }
diff --git a/src/or/connection.c b/src/or/connection.c
index 4f95f10be4..402ae61b55 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -283,8 +283,7 @@ void connection_about_to_close_connection(connection_t *conn)
         conn->hold_open_until_flushed = 1;
         /* XXX this socks_reply never gets sent, since conn
          * gets removed right after this function finishes. */
-        connection_ap_handshake_socks_reply(conn, NULL, 0, -1);
-        conn->socks_request->has_finished = 1;
+        connection_ap_handshake_socks_reply(conn, NULL, 0, SOCKS5_GENERAL_ERROR);
       } else {
         control_event_stream_status(conn, STREAM_EVENT_CLOSED);
       }
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index fe77f43669..f3efb0498b 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -162,7 +162,10 @@ connection_edge_end(connection_t *conn, char reason, crypt_path_t *cpath_layer)
   return 0;
 }
 
-/** DOCDOC **/
+/** An error has just occured on an operation on an edge connection
+ * <b>conn</b>.  Extract the errno; convert it to an end reason, and send
+ * an appropriate relay end cell to <b>cpath_layer</b>.
+ **/
 int
 connection_edge_end_errno(connection_t *conn, crypt_path_t *cpath_layer)
 {
@@ -265,7 +268,15 @@ void connection_ap_expire_beginning(void) {
     conn = carray[i];
     if (conn->type != CONN_TYPE_AP)
       continue;
-    if (conn->state != AP_CONN_STATE_RESOLVE_WAIT &&
+    if (conn->state == AP_CONN_STATE_CONTROLLER_WAIT) {
+      if (now - conn->timestamp_lastread >= 120) {
+        log_fn(LOG_NOTICE, "Closing unattached stream.");
+        connection_mark_for_close(conn);
+      }
+      continue;
+    }
+
+    else if (conn->state != AP_CONN_STATE_RESOLVE_WAIT &&
         conn->state != AP_CONN_STATE_CONNECT_WAIT)
       continue;
     if (now - conn->timestamp_lastread < 15)
@@ -338,17 +349,21 @@ void connection_ap_attach_pending(void)
   }
 }
 
-/** DOCDOC
- * -1 on err, 1 on success, 0 on not-yet-sure.
+/** The AP connection <b>conn</b> has just failed while attaching or
+ * sending a BEGIN or resolving on <b>circ</b>, but another circuit
+ * might work.  Detach the circuit, and either reattach it, launch a
+ * new circuit, tell the controller, or give up as a appropriate.
+ *
+ * Returns -1 on err, 1 on success, 0 on not-yet-sure.
  */
 int
 connection_ap_detach_retriable(connection_t *conn, circuit_t *circ)
 {
   control_event_stream_status(conn, STREAM_EVENT_FAILED_RETRIABLE);
+  conn->timestamp_lastread = time(NULL);
   if (get_options()->ManageConnections) {
     conn->state = AP_CONN_STATE_CIRCUIT_WAIT;
     circuit_detach_stream(circ,conn);
-    /* Muck with timestamps? */
     return connection_ap_handshake_attach_circuit(conn);
   } else {
     conn->state = AP_CONN_STATE_CONTROLLER_WAIT;
@@ -678,7 +693,15 @@ addressmap_get_virtual_address(int type)
   }
 }
 
-/* DOCDOC */
+/** A controller has requested that we map some address of type
+ * <b>type</b> to the address <b>new_address</b>.  Choose an address
+ * that is unlikely to be used, and map it, and return it in a newly
+ * allocated string.  If another address of the same type is already
+ * mapped to <b>new_address</b>, try to return a copy of that address.
+ *
+ * The string in <b>new_address</b> may be freed, or inserted into a map
+ * as appropriate.
+ **/
 char *
 addressmap_register_virtual_address(int type, char *new_address)
 {
@@ -720,7 +743,7 @@ address_is_invalid_destination(const char *address) {
   return 0;
 }
 
-/* Iterate over all address mapings which have exipry times between
+/** Iterate over all address mapings which have exipry times between
  * min_expires and max_expires, inclusive.  If sl is provided, add an
  * "old-addr new-addr" string to sl for each mapping.  If sl is NULL,
  * remove the mappings.
@@ -779,19 +802,23 @@ static int connection_ap_handshake_process_socks(connection_t *conn) {
   log_fn(LOG_DEBUG,"entered.");
 
   sockshere = fetch_from_buf_socks(conn->inbuf, socks);
-  if (sockshere == -1 || sockshere == 0) {
+  if (sockshere == 0) {
+    if (socks->replylen) {
+      connection_write_to_buf(socks->reply, socks->replylen, conn);
+    } else {
+      log_fn(LOG_DEBUG,"socks handshake not all here yet.");
+    }
+    return 0;
+  } else if (sockshere == -1) {
     if (socks->replylen) { /* we should send reply back */
       log_fn(LOG_DEBUG,"reply is already set for us. Using it.");
-      connection_ap_handshake_socks_reply(conn, socks->reply, socks->replylen, 0);
+      connection_ap_handshake_socks_reply(conn, socks->reply, socks->replylen,
+                                          SOCKS5_GENERAL_ERROR);
       socks->replylen = 0; /* zero it out so we can do another round of negotiation */
-    } else if (sockshere == -1) { /* send normal reject */
-      log_fn(LOG_WARN,"Fetching socks handshake failed. Closing.");
-      connection_ap_handshake_socks_reply(conn, NULL, 0, -1);
     } else {
-      log_fn(LOG_DEBUG,"socks handshake not all here yet.");
+      log_fn(LOG_WARN,"Fetching socks handshake failed. Closing.");
+      connection_ap_handshake_socks_reply(conn, NULL, 0, SOCKS5_GENERAL_ERROR);
     }
-    if (sockshere == -1)
-      socks->has_finished = 1;
     return sockshere;
   } /* else socks handshake is done, continue processing */
 
@@ -1080,7 +1107,11 @@ int connection_ap_make_bridge(char *address, uint16_t port) {
   return fd[1];
 }
 
-/* DOCDOC */
+/** Send an answer to an AP connection that has requested a DNS lookup
+ * via SOCKS.  The type should be one of RESOLVED_TYPE_(IPV4|IPV6) or
+ * -1 for unreachable; the answer should be in the format specified
+ * in the socks extensions document.
+ **/
 void connection_ap_handshake_socks_resolved(connection_t *conn,
                                             int answer_type,
                                             size_t answer_len,
@@ -1132,15 +1163,15 @@ void connection_ap_handshake_socks_resolved(connection_t *conn,
     }
   }
   connection_ap_handshake_socks_reply(conn, buf, replylen,
-                                      (answer_type == RESOLVED_TYPE_IPV4 ||
-                                      answer_type == RESOLVED_TYPE_IPV6) ? 1 : -1);
+          (answer_type == RESOLVED_TYPE_IPV4 ||
+           answer_type == RESOLVED_TYPE_IPV6) ?
+                               SOCKS5_SUCCEEDED : SOCKS5_HOST_UNREACHABLE);
   conn->socks_request->has_finished = 1;
 }
 
 /** Send a socks reply to stream <b>conn</b>, using the appropriate
- * socks version, etc.
- *
- * Status can be 1 (succeeded), -1 (failed), or 0 (not sure yet).
+ * socks version, etc, and mark <b>conn</b> as completed with SOCKS
+ * handshaking.
  *
  * If <b>reply</b> is defined, then write <b>replylen</b> bytes of it
  * to conn and return, else reply based on <b>status</b>.
@@ -1148,32 +1179,34 @@ void connection_ap_handshake_socks_resolved(connection_t *conn,
  * If <b>reply</b> is undefined, <b>status</b> can't be 0.
  */
 void connection_ap_handshake_socks_reply(connection_t *conn, char *reply,
-                                         size_t replylen, int status) {
+                                         size_t replylen,
+                                         socks5_reply_status_t status) {
   char buf[256];
 
-  if (status) /* it's either 1 or -1 */
-    control_event_stream_status(conn,
-                       status==1 ? STREAM_EVENT_SUCCEEDED : STREAM_EVENT_FAILED);
+  control_event_stream_status(conn,
+     status==SOCKS5_SUCCEEDED ? STREAM_EVENT_SUCCEEDED : STREAM_EVENT_FAILED);
 
+  tor_assert(conn->socks_request);
+  if (conn->socks_request->has_finished) {
+    log_fn(LOG_WARN, "Harmless bug: duplicate calls to connection_ap_handshake_socks_reply.");
+    return;
+  }
   if (replylen) { /* we already have a reply in mind */
     connection_write_to_buf(reply, replylen, conn);
+    conn->socks_request->has_finished = 1;
     return;
   }
-  tor_assert(conn->socks_request);
-  tor_assert(status == 1 || status == -1);
   if (conn->socks_request->socks_version == 4) {
     memset(buf,0,SOCKS4_NETWORK_LEN);
 #define SOCKS4_GRANTED          90
 #define SOCKS4_REJECT           91
-    buf[1] = (status==1 ? SOCKS4_GRANTED : SOCKS4_REJECT);
+    buf[1] = (status==SOCKS5_SUCCEEDED ? SOCKS4_GRANTED : SOCKS4_REJECT);
     /* leave version, destport, destip zero */
     connection_write_to_buf(buf, SOCKS4_NETWORK_LEN, conn);
   }
   if (conn->socks_request->socks_version == 5) {
     buf[0] = 5; /* version 5 */
-#define SOCKS5_SUCCESS          0
-#define SOCKS5_GENERIC_ERROR    1
-    buf[1] = status==1 ? SOCKS5_SUCCESS : SOCKS5_GENERIC_ERROR;
+    buf[1] = (char)status;
     buf[2] = 0;
     buf[3] = 1; /* ipv4 addr */
     memset(buf+4,0,6); /* Set external addr/port to 0.
@@ -1182,6 +1215,7 @@ void connection_ap_handshake_socks_reply(connection_t *conn, char *reply,
   }
   /* If socks_version isn't 4 or 5, don't send anything.
    * This can happen in the case of AP bridges. */
+  conn->socks_request->has_finished = 1;
   return;
 }
 
diff --git a/src/or/control.c b/src/or/control.c
index b1f9d608e0..bf784a9cb3 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -926,7 +926,10 @@ control_event_logmsg(int severity, const char *msg)
   send_control_event(EVENT_WARNING, (uint32_t)(len+1), msg);
 }
 
-/** DOCDOC */
+/** Called whenever we receive new router descriptors: tell any
+ * interested control connections.  <b>routers</b> is a list of
+ * DIGEST_LEN-byte identity digests.
+ */
 int control_event_descriptors_changed(smartlist_t *routers)
 {
   size_t len;
diff --git a/src/or/main.c b/src/or/main.c
index e013ee225f..973c4fc469 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -195,7 +195,7 @@ static void connection_unlink(connection_t *conn, int remove) {
   connection_free(conn);
 }
 
-/** DOCDOC **/
+/** Schedule <b>conn</b> to be closed. **/
 void
 add_connection_to_closeable_list(connection_t *conn)
 {
@@ -318,7 +318,7 @@ void connection_start_writing(connection_t *conn) {
            (int)conn->s);
 }
 
-/** DOCDOC */
+/** Close all connections that have been schedule to get closed */
 static void
 close_closeable_connections(void)
 {
@@ -337,7 +337,8 @@ close_closeable_connections(void)
   }
 }
 
-/** DOCDOC */
+/** Libevent callback: this gets invoked when (connection_t*)<b>conn</b> has
+ * some data to read. */
 static void
 conn_read_callback(int fd, short event, void *_conn)
 {
@@ -371,6 +372,8 @@ conn_read_callback(int fd, short event, void *_conn)
     close_closeable_connections();
 }
 
+/** Libevent callback: this gets invoked when (connection_t*)<b>conn</b> has
+ * some data to write. */
 static void conn_write_callback(int fd, short events, void *_conn)
 {
   connection_t *conn = _conn;
@@ -512,7 +515,6 @@ static void conn_write(int i) {
  *    - Otherwise, remove the connection from connection_array and from
  *      all other lists, close it, and free it.
  * Returns 1 if the connection was closed, 0 otherwise.
- * DOCDOC closeable_list
  */
 static int conn_close_if_marked(int i) {
   connection_t *conn;
@@ -580,6 +582,35 @@ void directory_all_unreachable(time_t now) {
   }
 }
 
+INLINE int
+get_dir_fetch_period(or_options_t *options)
+{
+  if (options->DirFetchPeriod)
+    /* Value from config file. */
+    return options->DirFetchPeriod;
+  else if (options->DirPort)
+    /* Default for directory server */
+    return 20*60;
+  else
+    /* Default for average user. */
+    return 40*60;
+}
+
+INLINE int
+get_status_fetch_period(or_options_t *options)
+{
+  if (options->StatusFetchPeriod)
+    /* Value from config file. */
+    return options->StatusFetchPeriod;
+  else if (options->DirPort)
+    /* Default for directory server */
+    return 15*60;
+  else
+    /* Default for average user. */
+    return 30*60;
+}
+
+
 /** This function is called whenever we successfully pull down a directory.
  * If <b>identity_digest</b> is defined, it contains the digest of the
  * router that just gave us this directory. */
@@ -593,13 +624,13 @@ void directory_has_arrived(time_t now, char *identity_digest) {
    * after the directory we had when we started.
    */
   if (!time_to_fetch_directory)
-    time_to_fetch_directory = now + options->DirFetchPeriod;
+    time_to_fetch_directory = now + get_dir_fetch_period(options);
 
   if (!time_to_force_upload_descriptor)
     time_to_force_upload_descriptor = now + options->DirPostPeriod;
 
   if (!time_to_fetch_running_routers)
-    time_to_fetch_running_routers = now + options->StatusFetchPeriod;
+    time_to_fetch_running_routers = now + get_status_fetch_period(options);
 
   if (identity_digest) { /* if this is us, then our dirport is reachable */
     routerinfo_t *router = router_get_by_digest(identity_digest);
@@ -720,6 +751,7 @@ static void run_scheduled_events(time_t now) {
    * new running-routers list, and/or force-uploading our descriptor
    * (if we've passed our internal checks). */
   if (time_to_fetch_directory < now) {
+    time_t next_status_fetch;
     /* purge obsolete entries */
     routerlist_remove_old_routers(ROUTER_MAX_AGE);
 
@@ -734,9 +766,10 @@ static void run_scheduled_events(time_t now) {
     }
 
     directory_get_from_dirserver(DIR_PURPOSE_FETCH_DIR, NULL, 1);
-    time_to_fetch_directory = now + options->DirFetchPeriod;
-    if (time_to_fetch_running_routers < now + options->StatusFetchPeriod) {
-      time_to_fetch_running_routers = now + options->StatusFetchPeriod;
+    time_to_fetch_directory = now + get_dir_fetch_period(options);
+    next_status_fetch = now + get_status_fetch_period(options);
+    if (time_to_fetch_running_routers < next_status_fetch) {
+      time_to_fetch_running_routers = next_status_fetch;
     }
 
     /* Also, take this chance to remove old information from rephist. */
@@ -747,7 +780,7 @@ static void run_scheduled_events(time_t now) {
     if (!authdir_mode(options)) {
       directory_get_from_dirserver(DIR_PURPOSE_FETCH_RUNNING_LIST, NULL, 1);
     }
-    time_to_fetch_running_routers = now + options->StatusFetchPeriod;
+    time_to_fetch_running_routers = now + get_status_fetch_period(options);
   }
 
   if (time_to_force_upload_descriptor < now) {
@@ -816,7 +849,7 @@ static void run_scheduled_events(time_t now) {
   close_closeable_connections();
 }
 
-/** DOCDOC */
+/** Libevent callback: invoked once every second. */
 static void second_elapsed_callback(int fd, short event, void *args)
 {
   static struct event *timeout_event = NULL;
diff --git a/src/or/or.h b/src/or/or.h
index 68111c68b0..76261f1c66 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -700,8 +700,7 @@ typedef struct {
    */
   time_t published_on;
   time_t running_routers_updated_on;
-  /** DOCDOC
-   */
+  /** What is the most recently received running_routers structure? */
   running_routers_t *running_routers;
   /** Which router is claimed to have signed it? */
   char *signing_router;
@@ -1308,7 +1307,8 @@ int connection_ap_handshake_send_resolve(connection_t *ap_conn, circuit_t *circ)
 
 int connection_ap_make_bridge(char *address, uint16_t port);
 void connection_ap_handshake_socks_reply(connection_t *conn, char *reply,
-                                         size_t replylen, int status);
+                                         size_t replylen,
+                                         socks5_reply_status_t status);
 void connection_ap_handshake_socks_resolved(connection_t *conn,
                                             int answer_type,
                                             size_t answer_len,
diff --git a/src/or/relay.c b/src/or/relay.c
index 1d43339cf3..4c33b0381a 100644
--- a/src/or/relay.c
+++ b/src/or/relay.c
@@ -481,8 +481,11 @@ connection_edge_end_reason_str(char *payload, uint16_t length) {
   }
 }
 
-socks5_reply_status_t
-connection_edge_end_reason_sock5_response(char *payload, uint16_t length) {
+/** Translate the <b>payload</b> of length <b>length</b>, which
+ * came from a relay 'end' cell, into an appropriate SOCKS5 reply code.
+ */
+static socks5_reply_status_t
+connection_edge_end_reason_socks5_response(char *payload, uint16_t length) {
   if (length < 1) {
     log_fn(LOG_WARN,"End cell arrived with length 0. Should be at least 1.");
     return SOCKS5_GENERAL_ERROR;
@@ -641,8 +644,6 @@ connection_edge_process_relay_cell_not_open(
         circuit_log_path(LOG_INFO,circ);
         tor_assert(circ->timestamp_dirty);
         circ->timestamp_dirty -= get_options()->MaxCircuitDirtiness;
-        /* make sure not to expire/retry the stream quite yet */
-        conn->timestamp_lastread = time(NULL);
 
         if (connection_ap_detach_retriable(conn, circ) >= 0)
           return 0;
@@ -685,8 +686,7 @@ connection_edge_process_relay_cell_not_open(
                                 conn->chosen_exit_name);
     }
     circuit_log_path(LOG_INFO,circ);
-    connection_ap_handshake_socks_reply(conn, NULL, 0, 1);
-    conn->socks_request->has_finished = 1;
+    connection_ap_handshake_socks_reply(conn, NULL, 0, SOCKS5_SUCCEEDED);
     /* handle anything that might have queued */
     if (connection_edge_package_raw_inbuf(conn, 1) < 0) {
       /* (We already sent an end cell if possible) */
@@ -821,7 +821,12 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
              connection_edge_end_reason_str(cell->payload+RELAY_HEADER_SIZE,
                                             rh.length),
              conn->stream_id, (int)conn->stream_size);
-
+      if (conn->socks_request && !conn->socks_request->has_finished) {
+        socks5_reply_status_t status =
+          connection_edge_end_reason_socks5_response(
+                              cell->payload+RELAY_HEADER_SIZE, rh.length);
+        connection_ap_handshake_socks_reply(conn, NULL, 0, status);
+      }
 #ifdef HALF_OPEN
       conn->done_sending = 1;
       shutdown(conn->s, 1); /* XXX check return; refactor NM */
diff --git a/src/or/router.c b/src/or/router.c
index 785851682d..e0b500f869 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -57,6 +57,9 @@ crypto_pk_env_t *get_previous_onion_key(void) {
   return lastonionkey;
 }
 
+/** Store a copy of the current onion key into *<b>key</b>, and a copy
+ * of the most recent onion key into *<b>last</b>.
+ */
 void dup_onion_keys(crypto_pk_env_t **key, crypto_pk_env_t **last)
 {
   tor_assert(key);
@@ -516,6 +519,8 @@ void router_retry_connections(void) {
   }
 }
 
+/** Return true iff this OR should try to keep connections open to all
+ * other ORs. */
 int router_is_clique_mode(routerinfo_t *router) {
   if (router_digest_is_trusted_dir(router->identity_digest))
     return 1;
@@ -533,8 +538,9 @@ static int desc_is_dirty = 1;
 /** Boolean: do we need to regenerate the above? */
 static int desc_needs_upload = 0;
 
-/** OR only: try to upload our signed descriptor to all the directory servers
- * we know about. DOCDOC force
+/** OR only: If <b>force</b> is true, or we haven't uploaded this
+ * descriptor successfully yet, try to upload our signed descriptor to
+ * all the directory servers we know about.
  */
 void router_upload_dir_desc_to_dirservers(int force) {
   const char *s;
@@ -630,8 +636,9 @@ const char *router_get_my_descriptor(void) {
   return desc_routerinfo->signed_descriptor;
 }
 
-/** Rebuild a fresh routerinfo and signed server descriptor for this
- * OR.  Return 0 on success, -1 on error. DOCDOC force
+/** If <b>force</b> is true, or our descriptor is out-of-date, rebuild
+ * a fresh routerinfo and signed server descriptor for this OR.
+ * Return 0 on success, -1 on error.
  */
 int router_rebuild_descriptor(int force) {
   routerinfo_t *ri;
@@ -694,7 +701,7 @@ int router_rebuild_descriptor(int force) {
   return 0;
 }
 
-/** DOCDOC */
+/** Call when the current descriptor is out of date. */
 void
 mark_my_descriptor_dirty(void)
 {
@@ -919,6 +926,7 @@ int router_dump_router_to_string(char *s, size_t maxlen, routerinfo_t *router,
   return written+1;
 }
 
+/** Return true iff <b>s</b> is a legally valid server nickname. */
 int is_legal_nickname(const char *s)
 {
   size_t len;
@@ -927,6 +935,8 @@ int is_legal_nickname(const char *s)
   return len > 0 && len <= MAX_NICKNAME_LEN &&
     strspn(s,LEGAL_NICKNAME_CHARACTERS)==len;
 }
+/** Return true iff <b>s</b> is a legally valid server nickname or
+ * hex-encoded identity-key digest. */
 int is_legal_nickname_or_hexdigest(const char *s)
 {
   size_t len;
@@ -938,6 +948,7 @@ int is_legal_nickname_or_hexdigest(const char *s)
   return len == HEX_DIGEST_LEN+1 && strspn(s+1,HEX_CHARACTERS)==len-1;
 }
 
+/** Release all resources held in router keys. */
 void router_free_all_keys(void)
 {
   if (onionkey)
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 5c8a46a743..5f7df4fb54 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -1192,7 +1192,8 @@ void running_routers_free(running_routers_t *rr)
   tor_free(rr);
 }
 
-/** DOCDOC*/
+/** We've just got a running routers list in <b>rr</b>; update the
+ * status of the routers in <b>list</b>, and cache <b>rr</b> */
 void
 routerlist_set_runningrouters(routerlist_t *list, running_routers_t *rr)
 {
@@ -1389,6 +1390,7 @@ add_trusted_dir_server(const char *address, uint16_t port, const char *digest)
   smartlist_add(trusted_dir_servers, ent);
 }
 
+/** Remove all members from the list of trusted dir servers. */
 void clear_trusted_dir_servers(void)
 {
   if (trusted_dir_servers) {
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index 651ef52313..95d19d48d8 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -541,7 +541,9 @@ router_parse_routerlist_from_directory(const char *str,
   return r;
 }
 
-/* DOCDOC */
+/** Read a signed router status statement from <b>str</b>.  On
+ * success, return it, and cache the original string if
+ * <b>write_to_cache</b> is set.  Otherwise, return NULL.  */
 running_routers_t *
 router_parse_runningrouters(const char *str, int write_to_cache)
 {