Simplify calls to routerset_equal

The routerset_equal function explicitly handles NULL inputs, so
there's no need to check inputs for NULL before calling it.

Also fix a bug in routerset_equal where a non-NULL routerset with no
entries didn't get counted as equal to a NULL routerset.  This was
untriggerable, I think, but potentially annoying down the road.

2 files changed, 12 insertions, 13 deletions

diff --git a/src/or/config.c b/src/or/config.c
index 404e648dba..44cde85dc8 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -1260,15 +1260,11 @@ options_act(or_options_t *old_options)
   /* Check for transitions that need action. */
   if (old_options) {
     if ((options->UseEntryGuards && !old_options->UseEntryGuards) ||
-        (options->ExcludeNodes &&
-         !routerset_equal(old_options->ExcludeNodes,options->ExcludeNodes)) ||
-        (options->ExcludeExitNodes &&
-         !routerset_equal(old_options->ExcludeExitNodes,
-                          options->ExcludeExitNodes)) ||
-        (options->EntryNodes &&
-         !routerset_equal(old_options->EntryNodes, options->EntryNodes)) ||
-        (options->ExitNodes &&
-         !routerset_equal(old_options->ExitNodes, options->ExitNodes)) ||
+        !routerset_equal(old_options->ExcludeNodes,options->ExcludeNodes) ||
+        !routerset_equal(old_options->ExcludeExitNodes,
+                         options->ExcludeExitNodes) ||
+        !routerset_equal(old_options->EntryNodes, options->EntryNodes) ||
+        !routerset_equal(old_options->ExitNodes, options->ExitNodes) ||
         options->StrictNodes != old_options->StrictNodes) {
       log_info(LD_CIRC,
                "Changed to using entry guards, or changed preferred or "
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index a9a216b2a1..d5e8a6b051 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -5473,14 +5473,12 @@ routerset_needs_geoip(const routerset_t *set)
   return set && smartlist_len(set->country_names);
 }
 
-#if 0
 /** Return true iff there are no entries in <b>set</b>. */
 static int
 routerset_is_empty(const routerset_t *set)
 {
   return !set || smartlist_len(set->list) == 0;
 }
-#endif
 
 /** Helper.  Return true iff <b>set</b> contains a router based on the other
  * provided fields.  Return higher values for more specific subentries: a
@@ -5659,10 +5657,15 @@ routerset_to_string(const routerset_t *set)
 int
 routerset_equal(const routerset_t *old, const routerset_t *new)
 {
-  if (old == NULL && new == NULL)
+  if (routerset_is_empty(old) && routerset_is_empty(new)) {
+    /* Two empty sets are equal */
     return 1;
-  else if (old == NULL || new == NULL)
+  } else if (routerset_is_empty(old) || routerset_is_empty(new)) {
+    /* An empty set is equal to nothing else. */
     return 0;
+  }
+  tor_assert(old != NULL);
+  tor_assert(new != NULL);
 
   if (smartlist_len(old->list) != smartlist_len(new->list))
     return 0;