Try to re-approximate the older semantics of nodelist_add_routerinfo

3 files changed, 37 insertions, 49 deletions

diff --git a/src/or/nodelist.c b/src/or/nodelist.c
index bbbb9ebc3..6ce8dcf2c 100644
--- a/src/or/nodelist.c
+++ b/src/or/nodelist.c
@@ -115,63 +115,48 @@ node_get_or_create(const char *identity_digest)
   return node;
 }
 
-/** Replace <b>old</b> router with <b>new</b> in nodelist.  If
- *  <b>old</b> and <b>new</b> in fact are the same relays (having the
- *  same identity_digest) the node_t of <b>old</b> is used for
- *  <b>new</b>.  Otherwise the node_t of <b>old</b> is dropped and
- *  <b>new</b> gets a new one (which might be a recycled node_t in
- *  case we already have one matching its identity).
- */
-node_t *
-nodelist_replace_routerinfo(routerinfo_t *old, routerinfo_t *new)
-{
-  node_t *node = NULL;
-  tor_assert(old);
-  tor_assert(new);
-
-  if (tor_memeq(old->cache_info.identity_digest,
-                new->cache_info.identity_digest, DIGEST_LEN)) {
-    /* NEW == OLD, reuse node_t.  */
-    node = node_get_mutable_by_id(old->cache_info.identity_digest);
-    if (node) {
-      tor_assert(node->ri == old);
-      if (!routers_have_same_or_addrs(old, new)) {
-        /* These mustn't carry over when the address and orport
-           change. */
-        node->last_reachable = node->last_reachable6 = 0;
-        node->testing_since = node->testing_since6 = 0;
-      }
-    }
-  } else {
-    /* NEW != OLD, get a new node_t.  */
-    nodelist_remove_routerinfo(old);
-  }
-  node = nodelist_add_routerinfo(node, new);
-
-  return node;
+/** Called when a node's address changes. */
+static void
+node_addrs_changed(node_t *node)
+{
+  node->last_reachable = node->last_reachable6 = 0;
+  node->testing_since = node->testing_since6 = 0;
+  node->country = -1;
 }
 
-
-/** Add <b>ri</b> to the nodelist.  If <b>node_in</b> is not NULL, use
-    that node rather than creating a new.  */
+/** Add <b>ri</b> to an appropriate node in the nodelist.  If we replace an
+ * old routerinfo, and <b>ri_old_out</b> is not NULL, set *<b>ri_old_out</b>
+ * to the previous routerinfo.
+ */
 node_t *
-nodelist_add_routerinfo(node_t *node_in, routerinfo_t *ri)
+nodelist_set_routerinfo(routerinfo_t *ri, routerinfo_t **ri_old_out)
 {
-  node_t *node = NULL;
+  node_t *node;
+  const char *id_digest;
+  int had_router = 0;
+  tor_assert(ri);
 
-  if (node_in) {
-    node = node_in;
+  init_nodelist();
+  id_digest = ri->cache_info.identity_digest;
+  node = node_get_or_create(id_digest);
+
+  if (node->ri) {
+    if (!routers_have_same_or_addrs(node->ri, ri)) {
+      node_addrs_changed(node);
+    }
+    had_router = 1;
+    if (ri_old_out)
+      *ri_old_out = node->ri;
   } else {
-    tor_assert(ri);
-    init_nodelist();
-    node = node_get_or_create(ri->cache_info.identity_digest);
+    if (ri_old_out)
+      *ri_old_out = NULL;
   }
   node->ri = ri;
 
   if (node->country == -1)
     node_set_country(node);
 
-  if (authdir_mode(get_options())) {
+  if (authdir_mode(get_options()) && !had_router) {
     const char *discard=NULL;
     uint32_t status = dirserv_router_get_status(ri, &discard);
     dirserv_set_node_flags_from_authoritative_status(node, status);
diff --git a/src/or/nodelist.h b/src/or/nodelist.h
index b110fe52f..6c1d54148 100644
--- a/src/or/nodelist.h
+++ b/src/or/nodelist.h
@@ -15,8 +15,7 @@
 node_t *node_get_mutable_by_id(const char *identity_digest);
 const node_t *node_get_by_id(const char *identity_digest);
 const node_t *node_get_by_hex_id(const char *identity_digest);
-node_t *nodelist_replace_routerinfo(routerinfo_t *old, routerinfo_t *new);
-node_t *nodelist_add_routerinfo(node_t *node, routerinfo_t *ri);
+node_t *nodelist_set_routerinfo(routerinfo_t *ri, routerinfo_t **ri_old_out);
 node_t *nodelist_add_microdesc(microdesc_t *md);
 void nodelist_set_consensus(networkstatus_t *ns);
 
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index a349a4d0d..c96a7268b 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -2877,7 +2877,7 @@ routerlist_insert(routerlist_t *rl, routerinfo_t *ri)
               &ri->cache_info);
   smartlist_add(rl->routers, ri);
   ri->cache_info.routerlist_index = smartlist_len(rl->routers) - 1;
-  nodelist_add_routerinfo(NULL, ri);
+  nodelist_set_routerinfo(ri, NULL);
   router_dir_info_changed();
 #ifdef DEBUG_ROUTERLIST
   routerlist_assert_ok(rl);
@@ -3106,7 +3106,11 @@ routerlist_replace(routerlist_t *rl, routerinfo_t *ri_old,
   tor_assert(0 <= idx && idx < smartlist_len(rl->routers));
   tor_assert(smartlist_get(rl->routers, idx) == ri_old);
 
-  nodelist_replace_routerinfo(ri_old, ri_new);
+  {
+    routerinfo_t *ri_old_tmp=NULL;
+    nodelist_set_routerinfo(ri_new, &ri_old_tmp);
+    tor_assert(ri_old == ri_old_tmp);
+  }
 
   router_dir_info_changed();
   if (idx >= 0) {