Ticket #24734: b24734-001.patch

File b24734-001.patch, 13.8 KB (added by neel, 19 months ago)

[PATCH] Remove the return value from the fascist_firewall_choose_address_* family of functions (Revision 1)

  • new file changes/ticket24734

    From 08c9b74c2d1bab67bca15a398b389219d7d75888 Mon Sep 17 00:00:00 2001
    From: Neel Chauhan <neel@neelc.org>
    Date: Sat, 7 Apr 2018 21:00:40 -0400
    Subject: [PATCH] Remove the return value from the
     fascist_firewall_choose_address_* family of functions
    
    ---
     changes/ticket24734    |  5 ++++
     src/or/circuitbuild.c  |  6 ++--
     src/or/directory.c     | 12 ++++----
     src/or/policies.c      | 66 +++++++++++++++++-------------------------
     src/or/policies.h      | 14 ++++-----
     src/test/test_policy.c | 14 +++------
     6 files changed, 50 insertions(+), 67 deletions(-)
     create mode 100644 changes/ticket24734
    
    diff --git a/changes/ticket24734 b/changes/ticket24734
    new file mode 100644
    index 000000000..fd4bafd57
    - +  
     1  o Code simplification and refactoring:
     2    - Remove the return value for fascist_firewall_choose_address_base(),
     3      and sister functions such as fascist_firewall_choose_address_node()
     4      and fascist_firewall_choose_address_rs(). Closes ticket 24734. Patch
     5      by Neel Chauhan.
  • src/or/circuitbuild.c

    diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
    index 488f6a214..865d0a5e0 100644
    a b extend_info_from_node(const node_t *node, int for_direct_connect) 
    28042804  /* Choose a preferred address first, but fall back to an allowed address.
    28052805   * choose_address returns 1 on success, but get_prim_orport returns 0. */
    28062806  if (for_direct_connect)
    2807     valid_addr = fascist_firewall_choose_address_node(node,
    2808                                                       FIREWALL_OR_CONNECTION,
    2809                                                       0, &ap);
     2807    fascist_firewall_choose_address_node(node, FIREWALL_OR_CONNECTION, 0, &ap);
    28102808  else {
    28112809    node_get_prim_orport(node, &ap);
    2812     valid_addr = tor_addr_port_is_valid_ap(&ap, 0);
    28132810  }
     2811  valid_addr = tor_addr_port_is_valid_ap(&ap, 0);
    28142812
    28152813  if (valid_addr)
    28162814    log_debug(LD_CIRC, "using %s for %s",
  • src/or/directory.c

    diff --git a/src/or/directory.c b/src/or/directory.c
    index c419b61d0..d6ce9290a 100644
    a b directory_choose_address_routerstatus(const routerstatus_t *status, 
    794794     * Use the preferred address and port if they are reachable, otherwise,
    795795     * use the alternate address and port (if any).
    796796     */
    797     have_or = fascist_firewall_choose_address_rs(status,
    798                                                  FIREWALL_OR_CONNECTION, 0,
    799                                                  use_or_ap);
     797    fascist_firewall_choose_address_rs(status, FIREWALL_OR_CONNECTION, 0,
     798                                       use_or_ap);
     799    have_or = tor_addr_port_is_valid_ap(use_or_ap, 0);
    800800  }
    801801
    802802  /* DirPort connections
    directory_choose_address_routerstatus(const routerstatus_t *status, 
    805805      indirection == DIRIND_ANON_DIRPORT ||
    806806      (indirection == DIRIND_ONEHOP
    807807       && !directory_must_use_begindir(options))) {
    808     have_dir = fascist_firewall_choose_address_rs(status,
    809                                                   FIREWALL_DIR_CONNECTION, 0,
    810                                                   use_dir_ap);
     808    fascist_firewall_choose_address_rs(status, FIREWALL_DIR_CONNECTION, 0,
     809                                       use_dir_ap);
     810    have_dir = tor_addr_port_is_valid_ap(use_dir_ap, 0);
    811811  }
    812812
    813813  /* We rejected all addresses in the relay's status. This means we can't
  • src/or/policies.c

    diff --git a/src/or/policies.c b/src/or/policies.c
    index f718ded32..af8ceca29 100644
    a b fascist_firewall_choose_address(const tor_addr_port_t *a, 
    827827 * If both addresses could be chosen (they are both preferred or both allowed)
    828828 * choose IPv6 if pref_ipv6 is true, otherwise choose IPv4.
    829829 * If neither address is chosen, return 0, else return 1. */
    830 static int
     830static void
    831831fascist_firewall_choose_address_base(const tor_addr_t *ipv4_addr,
    832832                                     uint16_t ipv4_orport,
    833833                                     uint16_t ipv4_dirport,
    fascist_firewall_choose_address_base(const tor_addr_t *ipv4_addr, 
    865865  if (result) {
    866866    tor_addr_copy(&ap->addr, &result->addr);
    867867    ap->port = result->port;
    868     return 1;
    869868  } else {
    870869    tor_addr_make_null(&ap->addr, AF_UNSPEC);
    871870    ap->port = 0;
    872     return 0;
    873871  }
    874872}
    875873
    876874/** Like fascist_firewall_choose_address_base(), but takes a host-order IPv4
    877875 * address as the first parameter. */
    878 static int
     876static void
    879877fascist_firewall_choose_address_ipv4h(uint32_t ipv4h_addr,
    880878                                      uint16_t ipv4_orport,
    881879                                      uint16_t ipv4_dirport,
    fascist_firewall_choose_address_ipv4h(uint32_t ipv4h_addr, 
    889887{
    890888  tor_addr_t ipv4_addr;
    891889  tor_addr_from_ipv4h(&ipv4_addr, ipv4h_addr);
    892   return fascist_firewall_choose_address_base(&ipv4_addr, ipv4_orport,
    893                                               ipv4_dirport, ipv6_addr,
    894                                               ipv6_orport, ipv6_dirport,
    895                                               fw_connection, pref_only,
    896                                               pref_ipv6, ap);
     890  fascist_firewall_choose_address_base(&ipv4_addr, ipv4_orport,
     891                                       ipv4_dirport, ipv6_addr,
     892                                       ipv6_orport, ipv6_dirport,
     893                                       fw_connection, pref_only,
     894                                       pref_ipv6, ap);
    897895}
    898896
    899897/* Some microdescriptor consensus methods have no IPv6 addresses in rs: they
    node_awaiting_ipv6(const or_options_t* options, const node_t *node) 
    944942 * This should only happen when there's no valid consensus, and rs doesn't
    945943 * correspond to a bridge client's bridge.
    946944 */
    947 int
     945void
    948946fascist_firewall_choose_address_rs(const routerstatus_t *rs,
    949947                                   firewall_connection_t fw_connection,
    950948                                   int pref_only, tor_addr_port_t* ap)
    951949{
    952950  if (!rs) {
    953     return 0;
     951    return;
    954952  }
    955953
    956954  tor_assert(ap);
    fascist_firewall_choose_address_rs(const routerstatus_t *rs, 
    959957  const node_t *node = node_get_by_id(rs->identity_digest);
    960958
    961959  if (node && !node_awaiting_ipv6(options, node)) {
    962     return fascist_firewall_choose_address_node(node, fw_connection, pref_only,
    963                                                 ap);
     960    fascist_firewall_choose_address_node(node, fw_connection, pref_only, ap);
    964961  } else {
    965962    /* There's no node-specific IPv6 preference, so use the generic IPv6
    966963     * preference instead. */
    fascist_firewall_choose_address_rs(const routerstatus_t *rs, 
    970967
    971968    /* Assume IPv4 and IPv6 DirPorts are the same.
    972969     * Assume the IPv6 OR and Dir addresses are the same. */
    973     return fascist_firewall_choose_address_ipv4h(rs->addr,
    974                                                  rs->or_port,
    975                                                  rs->dir_port,
    976                                                  &rs->ipv6_addr,
    977                                                  rs->ipv6_orport,
    978                                                  rs->dir_port,
    979                                                  fw_connection,
    980                                                  pref_only,
    981                                                  pref_ipv6,
    982                                                  ap);
     970    fascist_firewall_choose_address_ipv4h(rs->addr, rs->or_port, rs->dir_port,
     971                                          &rs->ipv6_addr, rs->ipv6_orport,
     972                                          rs->dir_port, fw_connection,
     973                                          pref_only, pref_ipv6, ap);
    983974  }
    984975}
    985976
    986977/** Like fascist_firewall_choose_address_base(), but takes <b>node</b>, and
    987978 * looks up the node's IPv6 preference rather than taking an argument
    988979 * for pref_ipv6. */
    989 int
     980void
    990981fascist_firewall_choose_address_node(const node_t *node,
    991982                                     firewall_connection_t fw_connection,
    992983                                     int pref_only, tor_addr_port_t *ap)
    993984{
    994985  if (!node) {
    995     return 0;
     986    return;
    996987  }
    997988
    998989  node_assert_ok(node);
    fascist_firewall_choose_address_node(const node_t *node, 
    1006997   * descriptor (routerinfo), or is one of our configured bridges before
    1007998   * calling this function. */
    1008999  if (BUG(node_awaiting_ipv6(get_options(), node))) {
    1009     return 0;
     1000    return;
    10101001  }
    10111002
    10121003  const int pref_ipv6_node = (fw_connection == FIREWALL_OR_CONNECTION
    fascist_firewall_choose_address_node(const node_t *node, 
    10241015  node_get_pref_ipv6_dirport(node, &ipv6_dir_ap);
    10251016
    10261017  /* Assume the IPv6 OR and Dir addresses are the same. */
    1027   return fascist_firewall_choose_address_base(&ipv4_or_ap.addr,
    1028                                               ipv4_or_ap.port,
    1029                                               ipv4_dir_ap.port,
    1030                                               &ipv6_or_ap.addr,
    1031                                               ipv6_or_ap.port,
    1032                                               ipv6_dir_ap.port,
    1033                                               fw_connection,
    1034                                               pref_only,
    1035                                               pref_ipv6_node,
    1036                                               ap);
     1018  fascist_firewall_choose_address_base(&ipv4_or_ap.addr, ipv4_or_ap.port,
     1019                                       ipv4_dir_ap.port, &ipv6_or_ap.addr,
     1020                                       ipv6_or_ap.port, ipv6_dir_ap.port,
     1021                                       fw_connection, pref_only,
     1022                                       pref_ipv6_node, ap);
    10371023}
    10381024
    10391025/** Like fascist_firewall_choose_address_rs(), but takes <b>ds</b>. */
    1040 int
     1026void
    10411027fascist_firewall_choose_address_dir_server(const dir_server_t *ds,
    10421028                                           firewall_connection_t fw_connection,
    10431029                                           int pref_only,
    10441030                                           tor_addr_port_t *ap)
    10451031{
    10461032  if (!ds) {
    1047     return 0;
     1033    return;
    10481034  }
    10491035
    10501036  /* A dir_server_t always has a fake_status. As long as it has the same
    fascist_firewall_choose_address_dir_server(const dir_server_t *ds, 
    10521038   * (See #17867.)
    10531039   * This function relies on fascist_firewall_choose_address_rs looking up the
    10541040   * node if it can, because that will get the latest info for the relay. */
    1055   return fascist_firewall_choose_address_rs(&ds->fake_status, fw_connection,
    1056                                             pref_only, ap);
     1041  fascist_firewall_choose_address_rs(&ds->fake_status, fw_connection,
     1042                                     pref_only, ap);
    10571043}
    10581044
    10591045/** Return 1 if <b>addr</b> is permitted to connect to our dir port,
  • src/or/policies.h

    diff --git a/src/or/policies.h b/src/or/policies.h
    index 35220a812..4879acdd8 100644
    a b int fascist_firewall_allows_dir_server(const dir_server_t *ds, 
    5555                                       firewall_connection_t fw_connection,
    5656                                       int pref_only);
    5757
    58 int fascist_firewall_choose_address_rs(const routerstatus_t *rs,
    59                                        firewall_connection_t fw_connection,
    60                                        int pref_only, tor_addr_port_t* ap);
    61 int fascist_firewall_choose_address_node(const node_t *node,
    62                                          firewall_connection_t fw_connection,
    63                                          int pref_only, tor_addr_port_t* ap);
    64 int fascist_firewall_choose_address_dir_server(const dir_server_t *ds,
     58void fascist_firewall_choose_address_rs(const routerstatus_t *rs,
     59                                        firewall_connection_t fw_connection,
     60                                        int pref_only, tor_addr_port_t* ap);
     61void fascist_firewall_choose_address_node(const node_t *node,
     62                                          firewall_connection_t fw_connection,
     63                                          int pref_only, tor_addr_port_t* ap);
     64void fascist_firewall_choose_address_dir_server(const dir_server_t *ds,
    6565                                          firewall_connection_t fw_connection,
    6666                                          int pref_only, tor_addr_port_t* ap);
    6767
  • src/test/test_policy.c

    diff --git a/src/test/test_policy.c b/src/test/test_policy.c
    index f8aa8ac40..924ca356f 100644
    a b test_policies_fascist_firewall_allows_address(void *arg) 
    19231923    tor_addr_port_t chosen_rs_ap; \
    19241924    tor_addr_make_null(&chosen_rs_ap.addr, AF_INET); \
    19251925    chosen_rs_ap.port = 0; \
    1926     tt_int_op(fascist_firewall_choose_address_rs(&(fake_rs), \
    1927                                                  (fw_connection), \
    1928                                                  (pref_only), \
    1929                                                  &chosen_rs_ap), \
    1930               OP_EQ, (expect_rv)); \
     1926    fascist_firewall_choose_address_rs(&(fake_rs), (fw_connection), \
     1927                                       (pref_only), &chosen_rs_ap); \
    19311928    tt_assert(tor_addr_eq(&(expect_ap).addr, &chosen_rs_ap.addr)); \
    19321929    tt_int_op((expect_ap).port, OP_EQ, chosen_rs_ap.port); \
    19331930  STMT_END
    test_policies_fascist_firewall_allows_address(void *arg) 
    19401937    tor_addr_port_t chosen_node_ap; \
    19411938    tor_addr_make_null(&chosen_node_ap.addr, AF_INET); \
    19421939    chosen_node_ap.port = 0; \
    1943     tt_int_op(fascist_firewall_choose_address_node(&(fake_node), \
    1944                                                    (fw_connection), \
    1945                                                    (pref_only), \
    1946                                                    &chosen_node_ap), \
    1947               OP_EQ, (expect_rv)); \
     1940    fascist_firewall_choose_address_node(&(fake_node), (fw_connection), \
     1941                                         (pref_only), &chosen_node_ap); \
    19481942    tt_assert(tor_addr_eq(&(expect_ap).addr, &chosen_node_ap.addr)); \
    19491943    tt_int_op((expect_ap).port, OP_EQ, chosen_node_ap.port); \
    19501944  STMT_END