r17338@aud-055: nickm | 2008-07-24 11:21:06 +0200

 Refactor the router_choose_random_node interface: any function with 10 parameters, most of which are boolean and one of which is unused, should get refactored like this.


svn:r16167

4 files changed, 75 insertions, 42 deletions

diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index 89a03b855e..d1ed06fa89 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -1355,19 +1355,26 @@ choose_good_exit_server(uint8_t purpose, routerlist_t *dir,
                         int need_uptime, int need_capacity, int is_internal)
 {
   or_options_t *options = get_options();
+  router_crn_flags_t flags = 0;
+  if (need_uptime)
+    flags |= CRN_NEED_UPTIME;
+  if (need_capacity)
+    flags |= CRN_NEED_CAPACITY;
+
   switch (purpose) {
     case CIRCUIT_PURPOSE_C_GENERAL:
+      if (options->_AllowInvalid & ALLOW_INVALID_MIDDLE)
+        flags |= CRN_ALLOW_INVALID;
       if (is_internal) /* pick it like a middle hop */
         return router_choose_random_node(NULL, NULL,
-              NULL, get_options()->ExcludeNodes, need_uptime, need_capacity, 0,
-              get_options()->_AllowInvalid & ALLOW_INVALID_MIDDLE, 0, 0);
+                                         options->ExcludeNodes, flags);
       else
         return choose_good_exit_server_general(dir,need_uptime,need_capacity);
     case CIRCUIT_PURPOSE_C_ESTABLISH_REND:
-      return router_choose_random_node(
-               NULL, NULL, NULL,
-               options->ExcludeNodes, need_uptime, need_capacity, 0,
-               options->_AllowInvalid & ALLOW_INVALID_RENDEZVOUS, 0, 0);
+      if (options->_AllowInvalid & ALLOW_INVALID_RENDEZVOUS)
+        flags |= CRN_ALLOW_INVALID;
+      return router_choose_random_node(NULL, NULL,
+                                       options->ExcludeNodes, flags);
   }
   log_warn(LD_BUG,"Unhandled purpose %d", purpose);
   tor_fragile_assert();
@@ -1569,6 +1576,7 @@ choose_good_middle_server(uint8_t purpose,
   smartlist_t *excluded;
   or_options_t *options = get_options();
   char *preferred = NULL;
+  router_crn_flags_t flags = 0;
   tor_assert(_CIRCUIT_PURPOSE_MIN <= purpose &&
              purpose <= _CIRCUIT_PURPOSE_MAX);
 
@@ -1586,10 +1594,15 @@ choose_good_middle_server(uint8_t purpose,
   }
   if (purpose == CIRCUIT_PURPOSE_TESTING)
     preferred = compute_preferred_testing_list(options->TestVia);
+
+  if (state->need_uptime)
+    flags |= CRN_NEED_UPTIME;
+  if (state->need_capacity)
+    flags |= CRN_NEED_CAPACITY;
+  if (options->_AllowInvalid & ALLOW_INVALID_MIDDLE)
+    flags |= CRN_ALLOW_INVALID;
   choice = router_choose_random_node(preferred,
-           NULL, excluded, options->ExcludeNodes,
-           state->need_uptime, state->need_capacity, 0,
-           options->_AllowInvalid & ALLOW_INVALID_MIDDLE, 0, 0);
+                                     excluded, options->ExcludeNodes, flags);
   tor_free(preferred);
   smartlist_free(excluded);
   return choice;
@@ -1609,6 +1622,7 @@ choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state)
   routerinfo_t *r, *choice;
   smartlist_t *excluded;
   or_options_t *options = get_options();
+  router_crn_flags_t flags = 0;
 
   if (state && options->UseEntryGuards &&
       (purpose != CIRCUIT_PURPOSE_TESTING || options->BridgeRelay)) {
@@ -1642,14 +1656,21 @@ choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state)
       });
   }
 
+  if (state) {
+    flags |= CRN_NEED_GUARD;
+    if (state->need_uptime)
+      flags |= CRN_NEED_UPTIME;
+    if (state->need_capacity)
+      flags |= CRN_NEED_CAPACITY;
+  }
+  if (options->_AllowInvalid & ALLOW_INVALID_ENTRY)
+    flags |= CRN_ALLOW_INVALID;
+
   choice = router_choose_random_node(
-           NULL, NULL,
+           NULL,
            excluded,
            options->ExcludeNodes,
-           state ? state->need_uptime : 0,
-           state ? state->need_capacity : 0,
-           state ? 0 : 1,
-           options->_AllowInvalid & ALLOW_INVALID_ENTRY, 0, 0);
+           flags);
   smartlist_free(excluded);
   return choice;
 }
diff --git a/src/or/or.h b/src/or/or.h
index 7f606525ae..6975b16fd6 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -4056,15 +4056,24 @@ typedef enum {
 routerinfo_t *routerlist_sl_choose_by_bandwidth(smartlist_t *sl,
                                                 bandwidth_weight_rule_t rule);
 routerstatus_t *routerstatus_sl_choose_by_bandwidth(smartlist_t *sl);
-/* XXXX021. This is a truly hideous interface. */
+
+/** Flags to be control router_choose_random_node */
+typedef enum {
+  CRN_NEED_UPTIME = 1<<0,
+  CRN_NEED_CAPACITY = 1<<1,
+  CRN_NEED_GUARD = 1<<2,
+  CRN_ALLOW_INVALID = 1<<3,
+  /* XXXX021 not used, apparently. */
+  CRN_STRICT_PREFERRED = 1<<4,
+  /* XXXX021 not used, apparently. */
+  CRN_WEIGHT_AS_EXIT = 1<<5
+} router_crn_flags_t;
+
 routerinfo_t *router_choose_random_node(const char *preferred,
-                                        const char *excluded,
                                         smartlist_t *excludedsmartlist,
                                         struct routerset_t *excludedset,
-                                        int need_uptime, int need_capacity,
-                                        int need_guard,
-                                        int allow_invalid, int strict,
-                                        int weight_for_exit);
+                                        router_crn_flags_t flags);
+
 routerinfo_t *router_get_by_nickname(const char *nickname,
                                      int warn_if_unnamed);
 int router_digest_version_as_new_as(const char *digest, const char *cutoff);
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index f20ed99f4a..c745a0eb4f 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -1275,10 +1275,11 @@ rend_services_introduce(void)
     smartlist_add_all(exclude_routers, intro_routers);
     /* The directory is now here. Pick three ORs as intro points. */
     for (j=prev_intro_nodes; j < NUM_INTRO_POINTS; ++j) {
-      router = router_choose_random_node(NULL, NULL, exclude_routers,
-               options->ExcludeNodes, 1, 0, 0,
-               get_options()->_AllowInvalid & ALLOW_INVALID_INTRODUCTION,
-               0, 0);
+      router_crn_flags_t flags = CRN_NEED_UPTIME;
+      if (get_options()->_AllowInvalid & ALLOW_INVALID_INTRODUCTION)
+        flags |= CRN_ALLOW_INVALID;
+      router = router_choose_random_node(NULL, exclude_routers,
+                                         options->ExcludeNodes, flags);
       if (!router) {
         log_warn(LD_REND,
                  "Could only establish %d introduction points for %s.",
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index b51daafbd9..896f0be4a1 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -1705,31 +1705,34 @@ routerstatus_sl_choose_by_bandwidth(smartlist_t *sl)
 
 /** Return a random running router from the routerlist.  If any node
  * named in <b>preferred</b> is available, pick one of those.  Never
- * pick a node named in <b>excluded</b>, or whose routerinfo is in
+ * pick a node whose routerinfo is in
  * <b>excludedsmartlist</b>, or whose routerinfo matches <b>excludedset</b>,
  * even if they are the only nodes
- * available.  If <b>strict</b> is true, never pick any node besides
- * those in <b>preferred</b>.
- * If <b>need_uptime</b> is non-zero and any router has more than
+ * available.  If <b>CRN_STRICT_PREFERRED</b> is set in flags, never pick
+ * any node besides those in <b>preferred</b>.
+ * If <b>CRN_NEED_UPTIME</b> is set in flags and any router has more than
  * a minimum uptime, return one of those.
- * If <b>need_capacity</b> is non-zero, weight your choice by the
+ * If <b>CRN_NEED_CAPACITY</b> is set in flags, weight your choice by the
  * advertised capacity of each router.
- * If ! <b>allow_invalid</b>, consider only Valid routers.
- * If <b>need_guard</b>, consider only Guard routers.
- * If <b>weight_for_exit</b>, we weight bandwidths as if picking an exit node,
- * otherwise we weight bandwidths for picking a relay node (that is, possibly
- * discounting exit nodes).
+ * If <b>CRN_ALLOW_INVALID</b> is not set in flags, consider only Valid routers.
+ * If <b>CRN_NEED_GUARD</b> is set in flags, consider only Guard routers.
+ * If <b>CRN_WEIGHT_AS_EXIT</b> is set in flags, we weight bandwidths as if
+ * picking an exit node, otherwise we weight bandwidths for picking a relay
+ * node (that is, possibly discounting exit nodes).
  */
 routerinfo_t *
 router_choose_random_node(const char *preferred,
-                          const char *excluded,
                           smartlist_t *excludedsmartlist,
                           routerset_t *excludedset,
-                          int need_uptime, int need_capacity,
-                          int need_guard,
-                          int allow_invalid, int strict,
-                          int weight_for_exit)
+                          router_crn_flags_t flags)
 {
+  const int need_uptime = (flags & CRN_NEED_UPTIME) != 0;
+  const int need_capacity = (flags & CRN_NEED_CAPACITY) != 0;
+  const int need_guard = (flags & CRN_NEED_GUARD) != 0;
+  const int allow_invalid = (flags & CRN_ALLOW_INVALID) != 0;
+  const int strict = (flags & CRN_STRICT_PREFERRED) != 0;
+  const int weight_for_exit = (flags & CRN_WEIGHT_AS_EXIT) != 0;
+
   smartlist_t *sl, *excludednodes;
   routerinfo_t *choice = NULL, *r;
   bandwidth_weight_rule_t rule;
@@ -1739,7 +1742,6 @@ router_choose_random_node(const char *preferred,
     (need_guard ? WEIGHT_FOR_GUARD : NO_WEIGHTING);
 
   excludednodes = smartlist_create();
-  add_nickname_list_to_smartlist(excludednodes,excluded,0);
 
   if ((r = routerlist_find_my_routerinfo())) {
     smartlist_add(excludednodes, r);
@@ -1786,9 +1788,9 @@ router_choose_random_node(const char *preferred,
                need_capacity?", fast":"",
                need_uptime?", stable":"",
                need_guard?", guard":"");
+      flags &= ~ (CRN_NEED_UPTIME|CRN_NEED_CAPACITY|CRN_NEED_GUARD);
       choice = router_choose_random_node(
-        NULL, excluded, excludedsmartlist, excludedset,
-        0, 0, 0, allow_invalid, 0, weight_for_exit);
+                       NULL, excludedsmartlist, excludedset, flags);
     }
   }
   smartlist_free(excludednodes);