[tor-commits] [tor/master] Remove the return value of node_get_prim_orport() and node_get_prim_dirport()

[nickm at torproject.org]

commit c2fa743806accfbb119c925fbd8466b0f48b7ef7
Author: Neel Chauhan <neel at neelc.org>
Date:   Tue Feb 6 08:13:55 2018 -0500

    Remove the return value of node_get_prim_orport() and node_get_prim_dirport()
---
 changes/ticket23873   |  6 ++++++
 src/or/circuitbuild.c |  6 ++++--
 src/or/nodelist.c     | 26 +++++++++++---------------
 src/or/nodelist.h     |  4 ++--
 4 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/changes/ticket23873 b/changes/ticket23873
new file mode 100644
index 000000000..ffe65e5ed
--- /dev/null
+++ b/changes/ticket23873
@@ -0,0 +1,6 @@
+  o Code simplification and refactoring:
+    - We remove the return value of node_get_prim_orport() and
+      node_get_prim_dirport(), and introduce node_get_prim_orport()
+      in node_ipv6_or_preferred() and node_ipv6_dir_preferred() in
+      order to check for a null address. Closes ticket 23873. Patch
+      by Neel Chauhan.
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index 6d6fd033b..b3b543348 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -2866,8 +2866,10 @@ extend_info_from_node(const node_t *node, int for_direct_connect)
     valid_addr = fascist_firewall_choose_address_node(node,
                                                       FIREWALL_OR_CONNECTION,
                                                       0, &ap);
-  else
-    valid_addr = !node_get_prim_orport(node, &ap);
+  else {
+    node_get_prim_orport(node, &ap);
+    valid_addr = tor_addr_port_is_valid_ap(&ap, 0);
+  }
 
   if (valid_addr)
     log_debug(LD_CIRC, "using %s for %s",
diff --git a/src/or/nodelist.c b/src/or/nodelist.c
index 391b31d68..40b56f3d1 100644
--- a/src/or/nodelist.c
+++ b/src/or/nodelist.c
@@ -1456,9 +1456,11 @@ node_ipv6_or_preferred(const node_t *node)
   /* XX/teor - node->ipv6_preferred is set from
    * fascist_firewall_prefer_ipv6_orport() each time the consensus is loaded.
    */
+  node_get_prim_orport(node, &ipv4_addr);
   if (!fascist_firewall_use_ipv6(options)) {
     return 0;
-  } else if (node->ipv6_preferred || node_get_prim_orport(node, &ipv4_addr)) {
+  } else if (node->ipv6_preferred ||
+             !tor_addr_port_is_valid_ap(&ipv4_addr, 0)) {
     return node_has_ipv6_orport(node);
   }
   return 0;
@@ -1469,14 +1471,12 @@ node_ipv6_or_preferred(const node_t *node)
     if (r && tor_addr_port_is_valid_ipv4h((r)->addr, (r)->port_field, 0)) { \
       tor_addr_from_ipv4h(&(ap_out)->addr, (r)->addr); \
       (ap_out)->port = (r)->port_field; \
-      return 0; \
     } \
   STMT_END
 
-/** Copy the primary (IPv4) OR port (IP address and TCP port) for
- * <b>node</b> into *<b>ap_out</b>. Return 0 if a valid address and
- * port was copied, else return non-zero.*/
-int
+/** Copy the primary (IPv4) OR port (IP address and TCP port) for <b>node</b>
+ * into *<b>ap_out</b>. */
+void
 node_get_prim_orport(const node_t *node, tor_addr_port_t *ap_out)
 {
   node_assert_ok(node);
@@ -1493,8 +1493,6 @@ node_get_prim_orport(const node_t *node, tor_addr_port_t *ap_out)
   RETURN_IPV4_AP(node->ri, or_port, ap_out);
   RETURN_IPV4_AP(node->rs, or_port, ap_out);
   /* Microdescriptors only have an IPv6 address */
-
-  return -1;
 }
 
 /** Copy the preferred OR port (IP address and TCP port) for
@@ -1566,19 +1564,19 @@ node_ipv6_dir_preferred(const node_t *node)
    * so we can't use it to determine DirPort IPv6 preference.
    * This means that bridge clients will use IPv4 DirPorts by default.
    */
+  node_get_prim_dirport(node, &ipv4_addr);
   if (!fascist_firewall_use_ipv6(options)) {
     return 0;
-  } else if (node_get_prim_dirport(node, &ipv4_addr)
+  } else if (!tor_addr_port_is_valid_ap(&ipv4_addr, 0)
       || fascist_firewall_prefer_ipv6_dirport(get_options())) {
     return node_has_ipv6_dirport(node);
   }
   return 0;
 }
 
-/** Copy the primary (IPv4) Dir port (IP address and TCP port) for
- * <b>node</b> into *<b>ap_out</b>. Return 0 if a valid address and
- * port was copied, else return non-zero.*/
-int
+/** Copy the primary (IPv4) Dir port (IP address and TCP port) for <b>node</b>
+ * into *<b>ap_out</b>. */
+void
 node_get_prim_dirport(const node_t *node, tor_addr_port_t *ap_out)
 {
   node_assert_ok(node);
@@ -1590,8 +1588,6 @@ node_get_prim_dirport(const node_t *node, tor_addr_port_t *ap_out)
   RETURN_IPV4_AP(node->ri, dir_port, ap_out);
   RETURN_IPV4_AP(node->rs, dir_port, ap_out);
   /* Microdescriptors only have an IPv6 address */
-
-  return -1;
 }
 
 #undef RETURN_IPV4_AP
diff --git a/src/or/nodelist.h b/src/or/nodelist.h
index dc20eaf0a..ff26c8cc7 100644
--- a/src/or/nodelist.h
+++ b/src/or/nodelist.h
@@ -77,11 +77,11 @@ int node_has_ipv6_dirport(const node_t *node);
 /* Deprecated - use node_ipv6_or_preferred or node_ipv6_dir_preferred */
 #define node_ipv6_preferred(node) node_ipv6_or_preferred(node)
 int node_ipv6_or_preferred(const node_t *node);
-int node_get_prim_orport(const node_t *node, tor_addr_port_t *ap_out);
+void node_get_prim_orport(const node_t *node, tor_addr_port_t *ap_out);
 void node_get_pref_orport(const node_t *node, tor_addr_port_t *ap_out);
 void node_get_pref_ipv6_orport(const node_t *node, tor_addr_port_t *ap_out);
 int node_ipv6_dir_preferred(const node_t *node);
-int node_get_prim_dirport(const node_t *node, tor_addr_port_t *ap_out);
+void node_get_prim_dirport(const node_t *node, tor_addr_port_t *ap_out);
 void node_get_pref_dirport(const node_t *node, tor_addr_port_t *ap_out);
 void node_get_pref_ipv6_dirport(const node_t *node, tor_addr_port_t *ap_out);
 int node_has_curve25519_onion_key(const node_t *node);