Refactor routerlist access slightly: always use router_get_by_routerlist(); change its interface; add modifier functions to add/remove elements from the current routerlist (so we can add indices).

svn:r5276

6 files changed, 74 insertions, 92 deletions

diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index b2a8b07fd5..fc433c9547 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -1254,14 +1254,9 @@ static int
 onion_pick_cpath_exit(circuit_t *circ, extend_info_t *exit)
 {
   cpath_build_state_t *state = circ->build_state;
-  routerlist_t *rl;
+  routerlist_t *rl = router_get_routerlist();
   int r;
 
-  router_get_routerlist(&rl);
-  if (!rl) {
-    log_fn(LOG_WARN,"router_get_routerlist returned empty list; closing circ.");
-    return -1;
-  }
   r = new_route_len(get_options()->PathlenCoinWeight, circ->purpose,
                     exit, rl->routers);
   if (r < 1) /* must be at least 1 */
@@ -1439,13 +1434,9 @@ choose_good_entry_server(cpath_build_state_t *state)
   }
   if (firewall_is_fascist()) {
     /* exclude all ORs that listen on the wrong port */
-    routerlist_t *rl;
+    routerlist_t *rl = router_get_routerlist();
     int i;
 
-    router_get_routerlist(&rl);
-    if (!rl)
-      return NULL;
-
     for (i=0; i < smartlist_len(rl->routers); i++) {
       r = smartlist_get(rl->routers, i);
       if (!fascist_firewall_allows_address(r->addr,r->or_port))
@@ -1745,9 +1736,7 @@ helper_nodes_set_status_from_directory(void)
   if (! helper_nodes)
     return;
 
-  router_get_routerlist(&routers);
-  if (! routers)
-    return;
+  routers = router_get_routerlist();
 
   now = time(NULL);
 
diff --git a/src/or/control.c b/src/or/control.c
index cb08ebbfd6..545e767fd6 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -1261,8 +1261,7 @@ handle_getinfo_helper(const char *question, char **answer)
     *answer = dirserver_getinfo_unregistered(question +
                                              strlen("unregistered-servers-"));
   } else if (!strcmp(question, "network-status")) {
-    routerlist_t *routerlist;
-    router_get_routerlist(&routerlist);
+    routerlist_t *routerlist = router_get_routerlist();
     if (!routerlist || !routerlist->routers ||
         list_server_status(routerlist->routers, answer) < 0) {
       return -1;
diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index dab99477ae..e0bf71fe58 100644
--- a/src/or/dirserv.c
+++ b/src/or/dirserv.c
@@ -333,16 +333,6 @@ dirserv_free_fingerprint_list(void)
  *    Descriptor list
  */
 
-static smartlist_t *
-get_descriptor_list(void)
-{
-  routerlist_t *routerlist;
-  router_get_routerlist(&routerlist);
-  if (!routerlist)
-    return NULL;
-  return routerlist->routers;
-}
-
 /** Return -1 if <b>ri</b> has a private or otherwise bad address,
  * unless we're configured to not care. Return 0 if all ok. */
 static int
@@ -487,21 +477,18 @@ directory_remove_invalid(void)
 {
   int i;
   int changed = 0;
-  smartlist_t *descriptor_list = get_descriptor_list();
-
-  if (!descriptor_list)
-    return;
+  routerlist_t *rl = router_get_routerlist();
 
-  for (i = 0; i < smartlist_len(descriptor_list); ++i) {
+  for (i = 0; i < smartlist_len(rl->routers); ++i) {
     const char *msg;
-    routerinfo_t *ent = smartlist_get(descriptor_list, i);
+    routerinfo_t *ent = smartlist_get(rl->routers, i);
     router_status_t r = dirserv_router_get_status(ent, &msg);
     switch (r) {
       case FP_REJECT:
         log(LOG_INFO, "Router '%s' is now rejected: %s",
             ent->nickname, msg?msg:"");
+        routerlist_remove(rl, ent, i--);
         routerinfo_free(ent);
-        smartlist_del(descriptor_list, i--);
         changed = 1;
         break;
       case FP_NAMED:
@@ -539,21 +526,15 @@ directory_remove_invalid(void)
 char *
 dirserver_getinfo_unregistered(const char *question)
 {
-  int i;
   router_status_t r;
   smartlist_t *answerlist;
   char buf[1024];
   char *answer;
-  routerinfo_t *ent;
   int min_bw = atoi(question);
-  smartlist_t *descriptor_list = get_descriptor_list();
-
-  if (!descriptor_list)
-    return tor_strdup("");
+  routerlist_t *rl = router_get_routerlist();
 
   answerlist = smartlist_create();
-  for (i = 0; i < smartlist_len(descriptor_list); ++i) {
-    ent = smartlist_get(descriptor_list, i);
+  SMARTLIST_FOREACH(rl->routers, routerinfo_t *, ent, {
     r = dirserv_router_get_status(ent, NULL);
     if (ent->bandwidthcapacity >= (size_t)min_bw &&
         ent->bandwidthrate >= (size_t)min_bw &&
@@ -565,7 +546,7 @@ dirserver_getinfo_unregistered(const char *question)
                    ent->platform ? ent->platform : "");
       smartlist_add(answerlist, tor_strdup(buf));
     }
-  }
+  });
   answer = smartlist_join_strings(answerlist, "\r\n", 0, NULL);
   SMARTLIST_FOREACH(answerlist, char *, cp, tor_free(cp));
   smartlist_free(answerlist);
@@ -754,15 +735,12 @@ dirserv_dump_directory_to_string(char **dir_out,
   char *buf = NULL;
   size_t buf_len;
   size_t identity_pkey_len;
-  smartlist_t *descriptor_list = get_descriptor_list();
+  routerlist_t *rl = router_get_routerlist();
 
   tor_assert(dir_out);
   *dir_out = NULL;
 
-  if (!descriptor_list)
-    return -1;
-
-  if (list_server_status(descriptor_list, &router_status))
+  if (list_server_status(rl->routers, &router_status))
     return -1;
 
   if (crypto_pk_write_public_key_to_string(private_key,&identity_pkey,
@@ -778,7 +756,7 @@ dirserv_dump_directory_to_string(char **dir_out,
 
   buf_len = 2048+strlen(recommended_versions)+
     strlen(router_status);
-  SMARTLIST_FOREACH(descriptor_list, routerinfo_t *, ri,
+  SMARTLIST_FOREACH(rl->routers, routerinfo_t *, ri,
                     buf_len += ri->signed_descriptor_len+1);
   buf = tor_malloc(buf_len);
   /* We'll be comparing against buf_len throughout the rest of the
@@ -800,7 +778,7 @@ dirserv_dump_directory_to_string(char **dir_out,
   tor_free(identity_pkey);
 
   cp = buf + strlen(buf);
-  SMARTLIST_FOREACH(descriptor_list, routerinfo_t *, ri,
+  SMARTLIST_FOREACH(rl->routers, routerinfo_t *, ri,
     {
       if (cp+ri->signed_descriptor_len+1 >= buf+buf_len)
         goto truncated;
@@ -1067,9 +1045,9 @@ generate_runningrouters(void)
   crypto_pk_env_t *private_key = get_identity_key();
   char *identity_pkey; /* Identity key, DER64-encoded. */
   size_t identity_pkey_len;
-  smartlist_t *descriptor_list = get_descriptor_list();
+  routerlist_t *rl = router_get_routerlist();
 
-  if (list_server_status(descriptor_list, &router_status)) {
+  if (list_server_status(rl->routers, &router_status)) {
     goto err;
   }
   if (crypto_pk_write_public_key_to_string(private_key,&identity_pkey,
@@ -1178,17 +1156,12 @@ generate_v2_networkstatus(void)
   struct in_addr in;
   uint32_t addr;
   crypto_pk_env_t *private_key = get_identity_key();
-  smartlist_t *descriptor_list = get_descriptor_list();
+  routerlist_t *rl = router_get_routerlist();
   time_t now = time(NULL);
   int naming = options->NamingAuthoritativeDir;
   int versioning = options->VersioningAuthoritativeDir;
   const char *contact;
 
-  if (!descriptor_list) {
-    log_fn(LOG_WARN, "Couldn't get router list.");
-    goto done;
-  }
-
   if (resolve_my_address(options, &addr, &hostname)<0) {
     log_fn(LOG_WARN, "Couldn't resolve my hostname");
     goto done;
@@ -1217,7 +1190,7 @@ generate_v2_networkstatus(void)
     contact = "(none)";
 
   len = 2048+strlen(client_versions)+strlen(server_versions)+identity_pkey_len*2;
-  len += (RS_ENTRY_LEN)*smartlist_len(descriptor_list) ;
+  len += (RS_ENTRY_LEN)*smartlist_len(rl->routers);
 
   status = tor_malloc(len);
   tor_snprintf(status, len,
@@ -1242,7 +1215,7 @@ generate_v2_networkstatus(void)
   outp = status + strlen(status);
   endp = status + len;
 
-  SMARTLIST_FOREACH(descriptor_list, routerinfo_t *, ri, {
+  SMARTLIST_FOREACH(rl->routers, routerinfo_t *, ri, {
       int f_exit = router_is_general_exit(ri);
       int f_stable = !router_is_unreliable(ri, 1, 0);
       int f_fast = !router_is_unreliable(ri, 0, 1);
@@ -1345,9 +1318,10 @@ dirserv_get_networkstatus_v2(smartlist_t *result,
                                     the_v2_networkstatus_is_dirty,
                                     generate_v2_networkstatus,
                                     "network status list", 0);
-      log_fn(LOG_WARN, "Unable to generate an authoritative network status.");
       if (d)
         smartlist_add(result, d);
+      else
+        log_fn(LOG_WARN,"Unable to generate an authoritative network status.");
     }
   } else if (!strcmp(key, "all")) {
     strmap_iter_t *iter = strmap_iter_init(cached_v2_networkstatus);
@@ -1403,16 +1377,11 @@ int
 dirserv_get_routerdescs(smartlist_t *descs_out, const char *key,
   const char **msg)
 {
-  smartlist_t *complete_list = get_descriptor_list();
   *msg = NULL;
 
-  if (!complete_list) {
-    *msg = "No server descriptors available";
-    return -1;
-  }
-
   if (!strcmp(key, "/tor/server/all")) {
-    smartlist_add_all(descs_out, complete_list);
+    routerlist_t *rl = router_get_routerlist();
+    smartlist_add_all(descs_out, rl->routers);
   } else if (!strcmp(key, "/tor/server/authority")) {
     routerinfo_t *ri = router_get_my_routerinfo();
     if (ri)
@@ -1475,18 +1444,15 @@ dirserv_orconn_tls_done(const char *address,
                         int as_advertised)
 {
   int i;
-  smartlist_t *descriptor_list = get_descriptor_list();
+  routerlist_t *rl = router_get_routerlist();
   tor_assert(address);
   tor_assert(digest_rcvd);
   tor_assert(nickname_rcvd);
 
-  if (!descriptor_list)
-    return;
-
   // XXXXNM We should really have a better solution here than dropping
   // XXXXNM whole routers; otherwise, they come back way too easily.
-  for (i = 0; i < smartlist_len(descriptor_list); ++i) {
-    routerinfo_t *ri = smartlist_get(descriptor_list, i);
+  for (i = 0; i < smartlist_len(rl->routers); ++i) {
+    routerinfo_t *ri = smartlist_get(rl->routers, i);
     int drop = 0;
     if (strcasecmp(address, ri->address) || or_port != ri->or_port)
       continue;
@@ -1503,8 +1469,8 @@ dirserv_orconn_tls_done(const char *address,
       }
     }
     if (drop) {
+      routerlist_remove(rl, ri, i--);
       routerinfo_free(ri);
-      smartlist_del(descriptor_list, i--);
       directory_set_dirty();
     } else { /* correct nickname and digest. mark this router reachable! */
       log_fn(LOG_INFO,"Found router %s to be reachable. Yay.", ri->nickname);
diff --git a/src/or/or.h b/src/or/or.h
index 45519fc5e5..47009592d5 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -2135,9 +2135,10 @@ routerinfo_t *router_get_by_hexdigest(const char *hexdigest);
 routerinfo_t *router_get_by_digest(const char *digest);
 routerinfo_t *router_get_by_descriptor_digest(const char *digest);
 int router_digest_is_trusted_dir(const char *digest);
-void router_get_routerlist(routerlist_t **prouterlist);
+routerlist_t *router_get_routerlist(void);
 void routerlist_reset_warnings(void);
 void routerlist_free(routerlist_t *routerlist);
+void routerlist_remove(routerlist_t *rl, routerinfo_t *ri, int idx);
 void routerinfo_free(routerinfo_t *router);
 void routerstatus_free(routerstatus_t *routerstatus);
 void networkstatus_free(networkstatus_t *networkstatus);
diff --git a/src/or/router.c b/src/or/router.c
index 947137af76..92ce6e0b45 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -610,18 +610,13 @@ consider_publishable_server(time_t now, int force)
 void
 router_retry_connections(int force)
 {
-  int i;
   time_t now = time(NULL);
-  routerinfo_t *router;
-  routerlist_t *rl;
+  routerlist_t *rl = router_get_routerlist();
   or_options_t *options = get_options();
 
   tor_assert(server_mode(options));
 
-  router_get_routerlist(&rl);
-  if (!rl) return;
-  for (i=0;i < smartlist_len(rl->routers);i++) {
-    router = smartlist_get(rl->routers, i);
+  SMARTLIST_FOREACH(rl->routers, routerinfo_t *, router, {
     if (router_is_me(router))
       continue;
     if (!clique_mode(options) && !router_is_clique_mode(router))
@@ -637,7 +632,7 @@ router_retry_connections(int force)
         router->testing_since = now;
       connection_or_connect(router->addr, router->or_port, router->identity_digest);
     }
-  }
+  });
 }
 
 /** Return true iff this OR should try to keep connections open to all
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index e6521e3964..3737ba0132 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -255,8 +255,7 @@ router_reload_router_list(void)
   int j;
 
   if (!routerlist) {
-    routerlist = tor_malloc_zero(sizeof(routerlist_t));
-    routerlist->routers = smartlist_create();
+    router_get_routerlist();
   }
 
   router_journal_len = router_store_len = 0;
@@ -1024,11 +1023,15 @@ router_get_by_descriptor_digest(const char *digest)
   return NULL;
 }
 
-/** Set *<b>prouterlist</b> to the current list of all known routers. */
-void
-router_get_routerlist(routerlist_t **prouterlist)
+/** Return the current list of all known routers. */
+routerlist_t *
+router_get_routerlist(void)
 {
-  *prouterlist = routerlist;
+  if (!routerlist) {
+    routerlist = tor_malloc_zero(sizeof(routerlist_t));
+    routerlist->routers = smartlist_create();
+  }
+  return routerlist;
 }
 
 /** Free all storage held by <b>router</b>. */
@@ -1101,6 +1104,35 @@ routerlist_free(routerlist_t *rl)
   tor_free(rl);
 }
 
+/** Insert an item <b>ri</b> into the routerlist <b>rl</b>, updating indices
+ * as needed. */
+static void
+routerlist_insert(routerlist_t *rl, routerinfo_t *ri)
+{
+  smartlist_add(rl->routers, ri);
+}
+
+/** Remove an item <b>ri</b> into the routerlist <b>rl</b>, updating indices
+ * as needed. If <b>idx</b> is nonnegative and smartlist_get(rl-&gt;routers,
+ * idx) == ri, we don't need to do a linear search over the list to decide
+ * which to remove.  We fill the gap rl-&gt;routers with a later element in
+ * the list, if any exists. */
+void
+routerlist_remove(routerlist_t *rl, routerinfo_t *ri, int idx)
+{
+  if (idx < 0 || smartlist_get(rl->routers, idx) != ri) {
+    idx = -1;
+    SMARTLIST_FOREACH(rl->routers, routerinfo_t *, r,
+                      if (r == ri) {
+                        idx = r_sl_idx;
+                        break;
+                      });
+    if (idx < 0)
+      return;
+  }
+  smartlist_del(rl->routers, idx);
+}
+
 /** Free all memory held by the rouerlist module */
 void
 routerlist_free_all(void)
@@ -1333,8 +1365,8 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg,
                  old_router->nickname);
           connection_mark_for_close(conn);
         }
+        routerlist_remove(routerlist, old_router, i--);
         routerinfo_free(old_router);
-        smartlist_del_keeporder(routerlist->routers, i--);
       } else if (old_router->is_named) {
         /* Can't replace a verified router with an unverified one. */
         log_fn(LOG_DEBUG, "Skipping unverified entry for verified router '%s'",
@@ -1347,7 +1379,7 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg,
   }
   /* We haven't seen a router with this name before.  Add it to the end of
    * the list. */
-  smartlist_add(routerlist->routers, router);
+  routerlist_insert(routerlist, router);
   if (!from_cache)
     router_append_to_journal(router->signed_descriptor,
                              router->signed_descriptor_len);
@@ -1373,8 +1405,8 @@ routerlist_remove_old_routers(int age)
     if (router->published_on <= cutoff) {
       /* Too old.  Remove it. */
       log_fn(LOG_INFO,"Forgetting obsolete routerinfo for router '%s'", router->nickname);
+      routerlist_remove(routerlist, router, i--);
       routerinfo_free(router);
-      smartlist_del(routerlist->routers, i--);
     }
   }
 }