[tor-commits] [tor/release-0.2.9] Fix policies.c instance of the "if (r=(a-b)) return r" pattern

[nickm at torproject.org]

commit 1afc2ed956a35b40dfd1d207652af5b50c295da7
Author: Nick Mathewson <nickm at torproject.org>
Date:   Mon Feb 13 13:53:30 2017 -0500

    Fix policies.c instance of the "if (r=(a-b)) return r" pattern
    
    I think this one probably can't underflow, since the input ranges
    are small.  But let's not tempt fate.
    
    This patch also replaces the "cmp" functions here with just "eq"
    functions, since nothing actually checked for anything besides 0 and
    nonzero.
    
    Related to 21278.
---
 src/or/policies.c      | 58 +++++++++++++++++++++++++-------------------------
 src/or/policies.h      |  2 +-
 src/or/routerlist.c    |  2 +-
 src/test/test_policy.c |  8 +++----
 4 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/src/or/policies.c b/src/or/policies.c
index 6037ee3..28770bb 100644
--- a/src/or/policies.c
+++ b/src/or/policies.c
@@ -1198,48 +1198,48 @@ policies_parse_from_options(const or_options_t *options)
   return ret;
 }
 
-/** Compare two provided address policy items, and return -1, 0, or 1
+/** Compare two provided address policy items, and renturn -1, 0, or 1
  * if the first is less than, equal to, or greater than the second. */
 static int
-cmp_single_addr_policy(addr_policy_t *a, addr_policy_t *b)
+single_addr_policy_eq(const addr_policy_t *a, const addr_policy_t *b)
 {
   int r;
-  if ((r=((int)a->policy_type - (int)b->policy_type)))
-    return r;
-  if ((r=((int)a->is_private - (int)b->is_private)))
-    return r;
+#define CMP_FIELD(field) do {                   \
+    if (a->field != b->field) {                 \
+      return 0;                                 \
+    }                                           \
+  } while (0)
+  CMP_FIELD(policy_type);
+  CMP_FIELD(is_private);
   /* refcnt and is_canonical are irrelevant to equality,
    * they are hash table implementation details */
   if ((r=tor_addr_compare(&a->addr, &b->addr, CMP_EXACT)))
-    return r;
-  if ((r=((int)a->maskbits - (int)b->maskbits)))
-    return r;
-  if ((r=((int)a->prt_min - (int)b->prt_min)))
-    return r;
-  if ((r=((int)a->prt_max - (int)b->prt_max)))
-    return r;
-  return 0;
+    return 0;
+  CMP_FIELD(maskbits);
+  CMP_FIELD(prt_min);
+  CMP_FIELD(prt_max);
+#undef CMP_FIELD
+  return 1;
 }
 
-/** Like cmp_single_addr_policy() above, but looks at the
- * whole set of policies in each case. */
+/** As single_addr_policy_eq, but compare every element of two policies.
+ */
 int
-cmp_addr_policies(smartlist_t *a, smartlist_t *b)
+addr_policies_eq(const smartlist_t *a, const smartlist_t *b)
 {
-  int r, i;
+  int i;
   int len_a = a ? smartlist_len(a) : 0;
   int len_b = b ? smartlist_len(b) : 0;
 
-  for (i = 0; i < len_a && i < len_b; ++i) {
-    if ((r = cmp_single_addr_policy(smartlist_get(a, i), smartlist_get(b, i))))
-      return r;
-  }
-  if (i == len_a && i == len_b)
+  if (len_a != len_b)
     return 0;
-  if (i < len_a)
-    return -1;
-  else
-    return 1;
+
+  for (i = 0; i < len_a; ++i) {
+    if (! single_addr_policy_eq(smartlist_get(a, i), smartlist_get(b, i)))
+      return 0;
+  }
+
+  return 1;
 }
 
 /** Node in hashtable used to store address policy entries. */
@@ -1255,7 +1255,7 @@ static HT_HEAD(policy_map, policy_map_ent_t) policy_root = HT_INITIALIZER();
 static inline int
 policy_eq(policy_map_ent_t *a, policy_map_ent_t *b)
 {
-  return cmp_single_addr_policy(a->policy, b->policy) == 0;
+  return single_addr_policy_eq(a->policy, b->policy);
 }
 
 /** Return a hashcode for <b>ent</b> */
@@ -1306,7 +1306,7 @@ addr_policy_get_canonical_entry(addr_policy_t *e)
     HT_INSERT(policy_map, &policy_root, found);
   }
 
-  tor_assert(!cmp_single_addr_policy(found->policy, e));
+  tor_assert(single_addr_policy_eq(found->policy, e));
   ++found->policy->refcnt;
   return found->policy;
 }
diff --git a/src/or/policies.h b/src/or/policies.h
index 850fa86..f73f850 100644
--- a/src/or/policies.h
+++ b/src/or/policies.h
@@ -76,7 +76,7 @@ void policy_expand_unspec(smartlist_t **policy);
 int policies_parse_from_options(const or_options_t *options);
 
 addr_policy_t *addr_policy_get_canonical_entry(addr_policy_t *ent);
-int cmp_addr_policies(smartlist_t *a, smartlist_t *b);
+int addr_policies_eq(const smartlist_t *a, const smartlist_t *b);
 MOCK_DECL(addr_policy_result_t, compare_tor_addr_to_addr_policy,
     (const tor_addr_t *addr, uint16_t port, const smartlist_t *policy));
 addr_policy_result_t compare_tor_addr_to_node_policy(const tor_addr_t *addr,
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index b876795..2365f28 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -5445,7 +5445,7 @@ router_differences_are_cosmetic(const routerinfo_t *r1, const routerinfo_t *r2)
       (r1->contact_info && r2->contact_info &&
        strcasecmp(r1->contact_info, r2->contact_info)) ||
       r1->is_hibernating != r2->is_hibernating ||
-      cmp_addr_policies(r1->exit_policy, r2->exit_policy) ||
+      ! addr_policies_eq(r1->exit_policy, r2->exit_policy) ||
       (r1->supports_tunnelled_dir_requests !=
        r2->supports_tunnelled_dir_requests))
     return 0;
diff --git a/src/test/test_policy.c b/src/test/test_policy.c
index f2d42b9..1ffdc2c 100644
--- a/src/test/test_policy.c
+++ b/src/test/test_policy.c
@@ -304,10 +304,10 @@ test_policies_general(void *arg)
   tt_assert(!exit_policy_is_general_exit(policy10));
   tt_assert(!exit_policy_is_general_exit(policy11));
 
-  tt_assert(cmp_addr_policies(policy, policy2));
-  tt_assert(cmp_addr_policies(policy, NULL));
-  tt_assert(!cmp_addr_policies(policy2, policy2));
-  tt_assert(!cmp_addr_policies(NULL, NULL));
+  tt_assert(!addr_policies_eq(policy, policy2));
+  tt_assert(!addr_policies_eq(policy, NULL));
+  tt_assert(addr_policies_eq(policy2, policy2));
+  tt_assert(addr_policies_eq(NULL, NULL));
 
   tt_assert(!policy_is_reject_star(policy2, AF_INET, 1));
   tt_assert(policy_is_reject_star(policy, AF_INET, 1));