Merge branch 'bug2616'

src/or/circuitbuild.c

@@ -3129,13 +3129,11 @@ choose_good_middle_server(uint8_t purpose,
   log_debug(LD_CIRC, "Contemplating intermediate hop: random choice.");
   excluded = smartlist_create();
   if ((r = build_state_get_exit_node(state))) {
-    smartlist_add(excluded, (void*) r);
-    nodelist_add_node_family(excluded, r);
+    nodelist_add_node_and_family(excluded, r);
   }
   for (i = 0, cpath = head; i < cur_len; ++i, cpath=cpath->next) {
     if ((r = node_get_by_id(cpath->extend_info->identity_digest))) {
-      smartlist_add(excluded, (void*)r);
-      nodelist_add_node_family(excluded, r);
+      nodelist_add_node_and_family(excluded, r);
     }
   }
 
@@ -3179,8 +3177,7 @@ choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state)
   if (state && (node = build_state_get_exit_node(state))) {
     /* Exclude the exit node from the state, if we have one.  Also exclude its
      * family. */
-    smartlist_add(excluded, (void*)node);
-    nodelist_add_node_family(excluded, node);
+    nodelist_add_node_and_family(excluded, node);
   }
   if (firewall_is_fascist_or()) {
     /* Exclude all ORs that we can't reach through our firewall */
@@ -3195,8 +3192,7 @@ choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state)
     SMARTLIST_FOREACH(entry_guards, entry_guard_t *, entry,
       {
         if ((node = node_get_by_id(entry->identity))) {
-          smartlist_add(excluded, (void*)node);
-          nodelist_add_node_family(excluded, node);
+          nodelist_add_node_and_family(excluded, node);
         }
       });
   }
@@ -4121,7 +4117,7 @@ choose_random_entry(cpath_build_state_t *state)
   int preferred_min, consider_exit_family = 0;
 
   if (chosen_exit) {
-    nodelist_add_node_family(exit_family, chosen_exit);
+    nodelist_add_node_and_family(exit_family, chosen_exit);
     consider_exit_family = 1;
   }
 

src/or/nodelist.h

@@ -53,7 +53,7 @@ smartlist_t *nodelist_get_list(void);
 /* XXXX These need to move out of routerlist.c */
 void nodelist_refresh_countries(void);
 void node_set_country(node_t *node);
-void nodelist_add_node_family(smartlist_t *nodes, const node_t *node);
+void nodelist_add_node_and_family(smartlist_t *nodes, const node_t *node);
 int nodes_in_same_family(const node_t *node1, const node_t *node2);
 
 #endif

src/or/routerlist.c

@@ -1357,12 +1357,17 @@ addrs_in_same_network_family(const tor_addr_t *a1,
   return 0 == tor_addr_compare_masked(a1, a2, 16, CMP_SEMANTIC);
 }
 
-/** Add all the family of <b>router</b> to the smartlist <b>sl</b>.
- * This is used to make sure we don't pick siblings in a single path,
- * or pick more than one relay from a family for our entry guard list.
+/**
+ * Add all the family of <b>node</b>, including <b>node</b> itself, to
+ * the smartlist <b>sl</b>.
+ *
+ * This is used to make sure we don't pick siblings in a single path, or
+ * pick more than one relay from a family for our entry guard list.
+ * Note that a node may be added to <b>sl</b> more than once if it is
+ * part of <b>node</b>'s family for more than one reason.
  */
 void
-nodelist_add_node_family(smartlist_t *sl, const node_t *node)
+nodelist_add_node_and_family(smartlist_t *sl, const node_t *node)
 {
   /* XXXX MOVE */
   const smartlist_t *all_nodes = nodelist_get_list();
@@ -1373,6 +1378,13 @@ nodelist_add_node_family(smartlist_t *sl, const node_t *node)
 
   declared_family = node_get_declared_family(node);
 
+  /* Let's make sure that we have the node itself, if it's a real node. */
+  {
+    const node_t *real_node = node_get_by_id(node->identity);
+    if (real_node)
+      smartlist_add(sl, (node_t*)real_node);
+  }
+
   /* First, add any nodes with similar network addresses. */
   if (options->EnforceDistinctSubnets) {
     tor_addr_t node_addr;
@@ -1417,13 +1429,14 @@ nodelist_add_node_family(smartlist_t *sl, const node_t *node)
   }
 }
 
-/** Given a <b>router</b>, add every node_t in its family to <b>sl</b>.
+/** Given a <b>router</b>, add every node_t in its family (including the
+ * node itself</b>) to <b>sl</b>.
  *
  * Note the type mismatch: This function takes a routerinfo, but adds nodes
  * to the smartlist!
  */
 static void
-routerlist_add_nodes_in_family(smartlist_t *sl, const routerinfo_t *router)
+routerlist_add_node_and_family(smartlist_t *sl, const routerinfo_t *router)
 {
   /* XXXX MOVE ? */
   node_t fake_node;
@@ -1434,7 +1447,7 @@ routerlist_add_nodes_in_family(smartlist_t *sl, const routerinfo_t *router)
     memcpy(fake_node.identity, router->cache_info.identity_digest, DIGEST_LEN);
     node = &fake_node;
   }
-  nodelist_add_node_family(sl, node);
+  nodelist_add_node_and_family(sl, node);
 }
 
 /** Return true iff <b>node</b> is named by some nickname in <b>lst</b>. */
@@ -2171,12 +2184,8 @@ router_choose_random_node(smartlist_t *excludedsmartlist,
       });
   }
 
-  if ((r = routerlist_find_my_routerinfo())) {
-    const node_t *me = node_get_by_id(r->cache_info.identity_digest);
-    if (me)
-      smartlist_add(excludednodes, (void *)me);
-    routerlist_add_nodes_in_family(excludednodes, r);
-  }
+  if ((r = routerlist_find_my_routerinfo()))
+    routerlist_add_node_and_family(excludednodes, r);
 
   router_add_running_nodes_to_smartlist(sl, allow_invalid,
                                         need_uptime, need_capacity,

src/or/routerlist.h

@@ -36,7 +36,6 @@ const routerstatus_t *router_pick_trusteddirserver(dirinfo_type_t type,
 int router_get_my_share_of_directory_requests(double *v2_share_out,
                                               double *v3_share_out);
 void router_reset_status_download_failures(void);
-void routerlist_add_family(smartlist_t *sl, const routerinfo_t *router);
 int routers_have_same_or_addr(const routerinfo_t *r1, const routerinfo_t *r2);
 int router_nickname_is_in_list(const routerinfo_t *router, const char *list);
 const routerinfo_t *routerlist_find_my_routerinfo(void);