Comments: cleanups and additions.

svn:r6174

src/or/circuituse.c

@@ -852,7 +852,6 @@ circuit_launch_by_nickname(uint8_t purpose, const char *exit_nickname,
   if (exit_nickname) {
     router = router_get_by_nickname(exit_nickname, 1);
     if (!router) {
-      /*XXXXNM domain? */
       log_warn(LD_GENERAL, "No such OR as '%s'", exit_nickname);
       return NULL;
     }

src/or/config.c

@@ -272,7 +272,8 @@ static config_var_t _state_vars[] = {
 #undef VAR
 #undef OBSOLETE
 
-/** DOCDOC*/
+/** Represents an English description of a configuration variable; used when
+ * generating configuration file comments. */
 typedef struct config_var_description_t {
   const char *name;
   const char *description;
@@ -434,10 +435,14 @@ static or_options_t *global_options = NULL;
 static char *torrc_fname = NULL;
 /** Persistent serialized state. */
 static or_state_t *global_state = NULL;
-/** DOCDOC */
+/** Parsed addr_policy_t describing which addresses we believe we can start
+ * circuits at. */
 static addr_policy_t *reachable_or_addr_policy = NULL;
+/** Parsed addr_policy_t describing which addresses we believe we can connect
+ * to directories at. */
 static addr_policy_t *reachable_dir_addr_policy = NULL;
 
+/** Allocate an empty configuration object of a given format type. */
 static void *
 config_alloc(config_format_t *fmt)
 {
@@ -481,6 +486,8 @@ set_options(or_options_t *new_val)
   return 0;
 }
 
+/** Release all memory and resources held by global configuration structures.
+ */
 void
 config_free_all(void)
 {
@@ -515,7 +522,7 @@ safe_str(const char *address)
     return address;
 }
 
-/** DOCDOC */
+/** Equivalent to escaped(safe_str(address)) */
 const char *
 escaped_safe_str(const char *address)
 {
@@ -952,7 +959,8 @@ config_free_lines(config_line_t *front)
   }
 }
 
-/** DOCDOC */
+/** Return the description for a given configuration variable, or NULL if no
+ * description exists. */
 static const char *
 config_find_description(config_format_t *fmt, const char *name)
 {
@@ -1782,7 +1790,8 @@ options_init(or_options_t *options)
   config_init(&options_format, options);
 }
 
-/* DOCDOC */
+/* Set all vars in the configuration object 'options' to their default
+ * values. */
 static void
 config_init(config_format_t *fmt, void *options)
 {
@@ -1798,7 +1807,8 @@ config_init(config_format_t *fmt, void *options)
   }
 }
 
-/* DOCDOC */
+/* Allocate and return a new string holding the written-out values of the vars
+ * in 'options' If 'minimal', do not write out any default-valued vars. */
 static char *
 config_dump(config_format_t *fmt, void *options, int minimal)
 {
@@ -1904,7 +1914,8 @@ validate_ports_csv(smartlist_t *sl, const char *name)
   return result;
 }
 
-/** DOCDOC */
+/** Helper: parse the Reachable(Dir|OR)?Addresses fields into
+ * reachable_(or|dir)_addr_policy. */
 static void
 parse_reachable_addresses(void)
 {
@@ -2013,10 +2024,10 @@ fascist_firewall_allows_address_dir(uint32_t addr, uint16_t port)
 /** Highest allowable value for StatusFetchPeriod for directory caches. */
 #define MAX_CACHE_STATUS_FETCH_PERIOD (15*60)
 
-/** Return 0 if every setting in <b>options</b> is reasonable.  Else
- * warn and return -1.  Should have no side effects, except for
- * normalizing the contents of <b>options</b>.
- * DOCDOC old_options.
+/** Return 0 if every setting in <b>options</b> is reasonable, and a
+ * permissible transition from <b>old_options</b>.  Else warn and return -1.
+ * Should have no side effects, except for normalizing the contents of
+ * <b>options</b>.
  *
  * XXX
  * If <b>from_setconf</b>, we were called by the controller, and our
@@ -3967,26 +3978,14 @@ check_libevent_version(const char *m, const char *v, int server)
 }
 #endif
 
-/* Versioning issues and state: we want to be able to understand old state
- * files, and not choke on new ones.
- *
- * We could preserve all unrecognized variables across invocations, but we
- * could screw up order, if their order is significant with respect to
- * existing options.
- *
- * We could just dump unrecognized variables if you downgrade.
- *
- * This needs thought. XXXX NM
- */
-
-/** DOCDOC */
+/** Return the persistent state struct for this Tor. */
 or_state_t *
 get_or_state(void)
 {
   return global_state;
 }
 
-/** DOCDOC */
+/** Return the filename used to write and read the persistent state. */
 static char *
 get_or_state_fname(void)
 {
@@ -3998,7 +3997,11 @@ get_or_state_fname(void)
   return fname;
 }
 
-/** DOCDOC */
+/** Return 0 if every setting in <b>state</b> is reasonable, and a
+ * permissible transition from <b>old_state</b>.  Else warn and return -1.
+ * Should have no side effects, except for normalizing the contents of
+ * <b>state</b>.
+ */
 /* XXX from_setconf is here because of bug 238 */
 static int
 or_state_validate(or_state_t *old_state, or_state_t *state, int from_setconf)
@@ -4016,7 +4019,7 @@ or_state_validate(or_state_t *old_state, or_state_t *state, int from_setconf)
   return 0;
 }
 
-/** DOCDOC */
+/** Replace the current persistent state with <b>new_state</b> */
 static void
 or_state_set(or_state_t *new_state)
 {
@@ -4031,7 +4034,9 @@ or_state_set(or_state_t *new_state)
     log_warn(LD_GENERAL,"Unparseable bandwidth history state: %s",err);
 }
 
-/* DOCDOC */
+/** Reload the persistent state from disk, generating a new state as needed.
+ * Return 0 on success, less than 0 on failure.
+ */
 int
 or_state_load(void)
 {
@@ -4091,7 +4096,7 @@ or_state_load(void)
   return r;
 }
 
-/** DOCDOC */
+/** Write the persistent state to disk. Return 0 for success, <0 on failure. */
 int
 or_state_save(void)
 {
@@ -4135,7 +4140,10 @@ or_state_save(void)
   return 0;
 }
 
-/** DOCDOC */
+/** Helper to implement GETINFO functions about configuration variables (not
+ * their values).  Given a "config/names" question, set *<b>answer</b> to a
+ * new string describing the supported configuration variables and their
+ * types. */
 int
 config_getinfo_helper(const char *question, char **answer)
 {

src/or/routerlist.c

@@ -79,9 +79,14 @@ static time_t last_routerdesc_download_attempted = 0;
  * mirrors).  Clients don't use this now. */
 static time_t last_networkstatus_download_attempted = 0;
 
-/* DOCDOC */
+/** True iff we have logged a warning about this OR not being verified or
+ * not being named. */
 static int have_warned_about_unverified_status = 0;
+/** True iff we have logged a warning about this OR's version being older than
+ * listed by the authorities  */
 static int have_warned_about_old_version = 0;
+/** True iff we have logged a warning about this OR's version being newer than
+ * listed by the authorities  */
 static int have_warned_about_new_version = 0;
 
 /** Repopulate our list of network_status_t objects from the list cached on
@@ -880,7 +885,6 @@ router_hex_digest_matches(routerinfo_t *router, const char *hexdigest)
   if (hexdigest[0] == '$')
     ++hexdigest;
 
-  /* XXXXNM Any place that uses this inside a loop could probably do better. */
   if (strlen(hexdigest) != HEX_DIGEST_LEN ||
       base16_decode(digest, DIGEST_LEN, hexdigest, HEX_DIGEST_LEN)<0)
     return 0;
@@ -1462,7 +1466,7 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg,
                          int from_cache, int from_fetch)
 {
   int i;
-  char id_digest[DIGEST_LEN];
+  const char *id_digest;
   int authdir = get_options()->AuthoritativeDir;
   int authdir_verified = 0;
 
@@ -1471,11 +1475,7 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg,
   if (!routerlist)
     router_get_routerlist();
 
-  /* XXXX NM If this assert doesn't trigger, we should remove the id_digest
-   * local. */
-  crypto_pk_get_digest(router->identity_pkey, id_digest);
-  tor_assert(!memcmp(id_digest, router->cache_info.identity_digest,
-                     DIGEST_LEN));
+  id_digest = router->cache_info.identity_digest;
 
   /* Make sure that we haven't already got this exact descriptor. */
   if (digestmap_get(routerlist->desc_digest_map,
@@ -1582,7 +1582,6 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg,
         connection_t *conn;
         while ((conn = connection_or_get_by_identity_digest(
                       old_router->cache_info.identity_digest))) {
-          // And LD_OR? XXXXNM
           log_info(LD_DIR,"Closing conn to router '%s'; there is now a named "
                    "router with that name.",
                    old_router->nickname);
@@ -2220,8 +2219,6 @@ signed_desc_digest_is_recognized(signed_descriptor_t *desc)
   return 0;
 }
 
-/* XXXX These should be configurable, perhaps? NM */
-
 /** How frequently do directory authorities re-download fresh networkstatus
  * documents? */
 #define AUTHORITY_NS_CACHE_INTERVAL (5*60)
@@ -3422,7 +3419,8 @@ router_list_client_downloadable(void)
   return downloadable;
 }
 
-/** Initiate new router downloads as needed.
+/** Initiate new router downloads as needed, using the strategy for
+ * non-directory-servers.
  *
  * We only allow one router descriptor download at a time.
  * If we have less than two network-status documents, we ask
@@ -3463,7 +3461,6 @@ update_router_descriptor_client_downloads(time_t now)
   }
 
   if (networkstatus_list && smartlist_len(networkstatus_list) < 2) {
-    /* XXXX Is this redundant? -NM */
     log_info(LD_DIR,
              "Not enough networkstatus documents to launch requests.");
   }
@@ -3515,7 +3512,9 @@ update_router_descriptor_client_downloads(time_t now)
   smartlist_free(downloadable);
 }
 
-/* DOCDOC */
+/** Launch downloads for router status as needed, using the strategy used by
+ * authorities and caches: download every descriptor we don't have but would
+ * serve from a random authoritiy that lists it. */
 static void
 update_router_descriptor_cache_downloads(time_t now)
 {
@@ -3637,7 +3636,7 @@ update_router_descriptor_cache_downloads(time_t now)
   digestmap_free(map,NULL);
 }
 
-/* DOCDOC */
+/** Launch downloads for router status as needed. */
 void
 update_router_descriptor_downloads(time_t now)
 {