commit e158f8de4b487450adeb89de204898263f6a87a5 Author: Nick Mathewson <nickm@torproject.org> Date: Mon Jul 11 11:21:47 2011 -0400 Rename and tweak nodelist_add_node_family() to add node It's very easy for nodelist_add_node_family(sl,node) to accidentally add 'node', and kind of hard to make sure that it omits it. Instead of taking pains to leave 'node' out, let's instead make sure that we always include it. I also rename the function to nodelist_add_node_and_family, and audit its users so that they don't add the node itself any longer, since the function will take care of that for them. Resolves bug 2616, which was not actually a bug. --- src/or/circuitbuild.c | 14 +++++--------- src/or/nodelist.h | 2 +- src/or/routerlist.c | 35 ++++++++++++++++++++++------------- src/or/routerlist.h | 1 - 4 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 90d9280..df33557 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -3102,13 +3102,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); } } @@ -3152,8 +3150,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 */ @@ -3168,8 +3165,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); } }); } @@ -4094,7 +4090,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; } diff --git a/src/or/nodelist.h b/src/or/nodelist.h index 08f11da..bd2e639 100644 --- a/src/or/nodelist.h +++ b/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 diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 5aaa4a9..c0a233c 100644 --- a/src/or/routerlist.c +++ b/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, diff --git a/src/or/routerlist.h b/src/or/routerlist.h index 3a8af6f..1129046 100644 --- a/src/or/routerlist.h +++ b/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);