Fix a bunch of log messages. Deprecate some routerlist fields; remove others, and status_set_at from routerinfo_t. Compress routerlist.c cleanup functions. Update cached networkstatus mtime when we download the same one twice. Change some interfaces.

svn:r5068

src/or/circuitbuild.c

@@ -1740,15 +1740,14 @@ helper_nodes_set_status_from_directory(void)
       routerinfo_t *r = router_get_by_digest(helper->identity);
       if (! r) {
         if (! helper->unlisted_since) {
-        /* Watch out for skew here. XXXX */
-          helper->unlisted_since = routers->published_on;
+          helper->unlisted_since = time(NULL);
           ++changed;
-          log_fn(LOG_WARN,"Helper node '%s' is not published in latest directory",
+          log_fn(LOG_WARN,"Helper node '%s' is not listed by directories",
                  helper->nickname);
         }
       } else {
         if (helper->unlisted_since) {
-          log_fn(LOG_WARN,"Helper node '%s' is listed again in latest directory",
+          log_fn(LOG_WARN,"Helper node '%s' is listed again by directories",
                  helper->nickname);
           ++changed;
         }
@@ -1761,7 +1760,7 @@ helper_nodes_set_status_from_directory(void)
           }
         } else {
           if (helper->down_since) {
-            log_fn(LOG_WARN,"Helper node '%s' is up in latest directory",
+            log_fn(LOG_WARN,"Helper node '%s' is up in latest directories",
                    helper->nickname);
             ++changed;
           }

src/or/main.c

@@ -1340,8 +1340,7 @@ tor_init(int argc, char *argv[])
 void
 tor_free_all(int postfork)
 {
-  routerlist_free_current();
-  free_trusted_dir_servers();
+  routerlist_free_all();
   addressmap_free_all();
   set_exit_redirects(NULL); /* free the registered exit redirects */
   free_socks_policy();

src/or/or.h

@@ -772,8 +772,6 @@ typedef struct {
   /*XXXX Make this get used once we think we do naming right. NM */
   unsigned int is_named:1; /* Do we believe the nickname that this OR gives us? */
 
-  time_t status_set_at; /**< When did we last update is_running? */
-
   /* The below items are used only by authdirservers for
    * reachability testing. */
   /** When was the last time we could reach this OR? */
@@ -847,7 +845,9 @@ typedef struct {
   smartlist_t *routers;
   /** When was the most recent directory that contributed to this list
    * published? */
-  time_t published_on;
+  /* XXXX011 NM This field is only used in moribund code; remove it
+   * once the moribund code is dead. */
+  time_t published_on_xx; 
 #if 0
   /** Which versions of tor are recommended by this directory? */
   char *software_versions;
@@ -2079,13 +2079,14 @@ routerinfo_t *router_get_by_hexdigest(const char *hexdigest);
 routerinfo_t *router_get_by_digest(const char *digest);
 int router_digest_is_trusted_dir(const char *digest);
 void router_get_routerlist(routerlist_t **prouterlist);
+#if 0
 time_t routerlist_get_published_time(void);
+#endif
 void routerlist_free(routerlist_t *routerlist);
 void routerinfo_free(routerinfo_t *router);
 void routerstatus_free(routerstatus_t *routerstatus);
 void networkstatus_free(networkstatus_t *networkstatus);
-void routerlist_free_current(void);
-void free_trusted_dir_servers(void);
+void routerlist_free_all(void);
 routerinfo_t *routerinfo_copy(const routerinfo_t *router);
 void router_mark_as_down(const char *digest);
 void routerlist_remove_old_routers(int age);
@@ -2153,7 +2154,7 @@ int router_get_networkstatus_v2_hash(const char *s, char *digest);
 int router_append_dirobj_signature(char *buf, size_t buf_len, const char *digest,
                                    crypto_pk_env_t *private_key);
 int router_parse_list_from_string(const char **s,
-                                  routerlist_t **dest,
+                                  smartlist_t *dest,
                                   time_t published);
 int router_parse_routerlist_from_directory(const char *s,
                                            routerlist_t **dest,

src/or/routerlist.c

@@ -76,7 +76,7 @@ router_reload_router_list(void)
       log_fn(LOG_WARN, "Cached directory at '%s' was unparseable; ignoring.", filename);
     }
     if (routerlist &&
-        ((routerlist->published_on > time(NULL) - MIN_ONION_KEY_LIFETIME/2)
+        ((routerlist->published_on_xx > time(NULL) - MIN_ONION_KEY_LIFETIME/2)
          || is_recent)) {
       directory_has_arrived(st.st_mtime, NULL); /* do things we've been waiting to do */
     }
@@ -438,7 +438,6 @@ mark_all_trusteddirservers_up(void)
                  if (router_digest_is_trusted_dir(router->identity_digest) &&
                      router->dir_port > 0) {
                    router->is_running = 1;
-                   router->status_set_at = time(NULL);
                  });
   }
   if (trusted_dir_servers) {
@@ -840,6 +839,8 @@ router_get_by_nickname(const char *nickname)
 
   SMARTLIST_FOREACH(routerlist->routers, routerinfo_t *, router,
   {
+    /* XXXX001 NM Should this restrict by Named rouers, or warn on
+     * non-named routers, or something? */
     if (0 == strcasecmp(router->nickname, nickname) ||
         (maybedigest && 0 == memcmp(digest, router->identity_digest,
                                     DIGEST_LEN)))
@@ -905,6 +906,7 @@ router_get_routerlist(routerlist_t **prouterlist)
   *prouterlist = routerlist;
 }
 
+#if 0
 /** Return the publication time on the current routerlist, or 0 if we have no
  * routerlist. */
 time_t
@@ -912,6 +914,8 @@ routerlist_get_published_time(void)
 {
   return routerlist ? routerlist->published_on : 0;
 }
+#endif
+
 
 /** Free all storage held by <b>router</b>. */
 void
@@ -983,9 +987,9 @@ routerlist_free(routerlist_t *rl)
   tor_free(rl);
 }
 
-/** Free all entries in the current router list. */
+/** Free all memory held by the rouerlist module */
 void
-routerlist_free_current(void)
+routerlist_free_all(void)
 {
   if (routerlist)
     routerlist_free(routerlist);
@@ -995,6 +999,18 @@ routerlist_free_current(void)
     smartlist_free(warned_nicknames);
     warned_nicknames = NULL;
   }
+  if (trusted_dir_servers) {
+    SMARTLIST_FOREACH(trusted_dir_servers, trusted_dir_server_t *, ds,
+                      { tor_free(ds->address); tor_free(ds); });
+    smartlist_free(trusted_dir_servers);
+    trusted_dir_servers = NULL;
+  }
+  if (networkstatus_list) {
+    SMARTLIST_FOREACH(networkstatus_list, networkstatus_t *, ns,
+                      networkstatus_free(ns));
+    smartlist_free(networkstatus_list);
+    networkstatus_list = NULL;
+  }
 }
 
 /** Free all storage held by the routerstatus object <b>rs</b>. */
@@ -1021,18 +1037,6 @@ networkstatus_free(networkstatus_t *ns)
   tor_free(ns);
 }
 
-/** Free all entries in the list of trusted directory servers. */
-void
-free_trusted_dir_servers(void)
-{
-  if (trusted_dir_servers) {
-    SMARTLIST_FOREACH(trusted_dir_servers, trusted_dir_server_t *, ds,
-                      { tor_free(ds->address); tor_free(ds); });
-    smartlist_free(trusted_dir_servers);
-    trusted_dir_servers = NULL;
-  }
-}
-
 /** Mark the router with ID <b>digest</b> as non-running in our routerlist. */
 void
 router_mark_as_down(const char *digest)
@@ -1051,7 +1055,6 @@ router_mark_as_down(const char *digest)
   if (router_is_me(router) && !we_are_hibernating())
     log_fn(LOG_WARN, "We just marked ourself as down. Are your external addresses reachable?");
   router->is_running = 0;
-  router->status_set_at = time(NULL);
 }
 
 /** Add <b>router</b> to the routerlist, if we don't already have it.  Replace
@@ -1303,7 +1306,7 @@ router_load_routerlist_from_directory(const char *s,
         smartlist_add(changed, r);
     });
     smartlist_clear(new_list->routers);
-    routerlist->published_on = new_list->published_on;
+    routerlist->published_on_xx = new_list->published_on_xx;
     routerlist_free(new_list);
     control_event_descriptors_changed(changed);
     smartlist_free(changed);
@@ -1374,6 +1377,7 @@ router_set_networkstatus(const char *s, time_t arrived_at,
   int skewed = 0;
   trusted_dir_server_t *trusted_dir;
   char fp[HEX_DIGEST_LEN+1];
+  char published[ISO_TIME_LEN+1];
 
   ns = networkstatus_parse_from_string(s);
   if (!ns) {
@@ -1391,8 +1395,10 @@ router_set_networkstatus(const char *s, time_t arrived_at,
 
   ns->received_on = arrived_at;
 
+  format_iso_time(published, ns->published_on);
+
   if (ns->published_on > now + NETWORKSTATUS_ALLOW_SKEW) {
-    log_fn(LOG_WARN, "Network status was published in the future (?). Somebody is skewed here: check your clock. Not caching.");
+    log_fn(LOG_WARN, "Network status was published in the future (%s GMT). Somebody is skewed here: check your clock. Not caching.", published);
     skewed = 1;
   }
 
@@ -1412,7 +1418,7 @@ router_set_networkstatus(const char *s, time_t arrived_at,
       smartlist_string_remove(requested_fingerprints, fp);
     } else {
       char *requested = smartlist_join_strings(requested_fingerprints," ",0,NULL);
-      log_fn(LOG_WARN, "We received a network status with a fingerprint (%s) that we never requested. (%s) Dropping.", fp, requested);
+      log_fn(LOG_WARN, "We received a network status with a fingerprint (%s) that we never requested. (We asked for: %s.) Dropping.", fp, requested);
       tor_free(requested);
       return 0;
     }
@@ -1430,13 +1436,27 @@ router_set_networkstatus(const char *s, time_t arrived_at,
                   ns->networkstatus_digest, DIGEST_LEN)) {
         /* Same one we had before. */
         networkstatus_free(ns);
-        log_fn(LOG_NOTICE, "Dropping network-status (%s); already have it.",fp);
-        if (old_ns->received_on < arrived_at)
-          /* XXXX We should touch the cache file. NM */
+        log_fn(LOG_NOTICE,
+            "Dropping network-status from %s:%d (published %s); already have it.",
+               trusted_dir->address, trusted_dir->dir_port, published);
+        if (old_ns->received_on < arrived_at) {
+          if (source != NS_FROM_CACHE) {
+            char *fn = networkstatus_get_cache_filename(old_ns);
+            /* We use mtime to tell when it arrived, so update that. */
+            touch_file(fn);
+            tor_free(fn);
+          }
           old_ns->received_on = arrived_at;
+        }
         return 0;
       } else if (old_ns->published_on >= ns->published_on) {
-        log_fn(LOG_NOTICE, "Dropping network-status (%s); we have a newer one for this authority.", fp);
+        char old_published[ISO_TIME_LEN+1];
+        format_iso_time(old_published, old_ns->published_on);
+        log_fn(LOG_NOTICE,
+               "Dropping network-status from %s:%d (published %s);"
+               " we have a newer one (published %s) for this authority.",
+               trusted_dir->address, trusted_dir->dir_port, published,
+               old_published);
         networkstatus_free(ns);
         return 0;
       } else {
@@ -1452,10 +1472,10 @@ router_set_networkstatus(const char *s, time_t arrived_at,
     smartlist_add(networkstatus_list, ns);
 
   /*XXXX011 downgrade to INFO NM */
-  log_fn(LOG_NOTICE, "New networkstatus %s (%s).",
-         source == NS_FROM_CACHE?"from our cache":
-         (source==NS_FROM_DIR?"from a directory server":"from this authority"),
-         fp);
+  log_fn(LOG_NOTICE, "Setting networkstatus %s %s:%d (published %s)",
+         source == NS_FROM_CACHE?"cached from":
+         (source==NS_FROM_DIR?"downloaded from":"generated for"),
+         trusted_dir->address, trusted_dir->dir_port, published);
   networkstatus_list_has_changed = 1;
 
   smartlist_sort(networkstatus_list, _compare_networkstatus_published_on);

src/or/routerparse.c

@@ -533,13 +533,15 @@ router_parse_routerlist_from_directory(const char *str,
   /* Read the router list from s, advancing s up past the end of the last
    * router. */
   str = end;
-  if (router_parse_list_from_string(&str, &new_dir,
+  new_dir = tor_malloc_zero(sizeof(routerlist_t));
+  new_dir->routers = smartlist_create();
+  if (router_parse_list_from_string(&str, new_dir->routers,
                                     published_on)) {
     log_fn(LOG_WARN, "Error reading routers from directory");
     goto err;
   }
 
-  new_dir->published_on = published_on;
+  new_dir->published_on_xx = published_on;
 
   SMARTLIST_FOREACH(tokens, directory_token_t *, tok, token_free(tok));
   smartlist_free(tokens);
@@ -754,15 +756,12 @@ check_directory_signature(const char *digest,
 }
 
 /** Given a string *<b>s</b> containing a concatenated sequence of router
- * descriptors, parses them and stores the result in *<b>dest</b>.  If
- * good_nickname_list is provided, then routers are marked as
- * running/nonrunning and verified/unverified based on their status in the
- * list.  Otherwise, all routers are marked running and verified.  Advances
- * *s to a point immediately following the last router entry.  Returns 0 on
- * success and -1 on failure.
+ * descriptors, parses them and stores the result in <b>dest</b>.  All routers
+ * are marked running and verified.  Advances *s to a point immediately
+ * following the last router entry.  Returns 0 on success and -1 on failure.
  */
 int
-router_parse_list_from_string(const char **s, routerlist_t **dest,
+router_parse_list_from_string(const char **s, smartlist_t *dest,
                               time_t published_on)
 {
   routerinfo_t *router;
@@ -799,10 +798,7 @@ router_parse_list_from_string(const char **s, routerlist_t **dest,
 
   routers_update_status_from_networkstatus(routers);
 
-  if (*dest)
-    routerlist_free(*dest);
-  *dest = tor_malloc_zero(sizeof(routerlist_t));
-  (*dest)->routers = routers;
+  smartlist_add_all(dest, routers);
 
   return 0;
 }