Re: [tor-bugs] #9729 [Tor]: Make bridges publish additional ORPort addresses in their descriptor

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#9729: Make bridges publish additional ORPort addresses in their descriptor
----------------------------+----------------------------------------------
     Reporter:  sqrt2       |      Owner:
         Type:              |     Status:  needs_revision
  enhancement               |  Milestone:
     Priority:  normal      |    Version:  Tor: 0.2.5.1-alpha
    Component:  Tor         |   Keywords:  ORPort bridge multiple addresses
   Resolution:              |  Parent ID:
Actual Points:              |
       Points:              |
----------------------------+----------------------------------------------

Comment (by sqrt2):

 Thanks for taking a look :) I'm going to attach a patch with changes as
 follows:

 > are we leaking memory in get_interface_address6() in "smart way" when
 return_addrs is empty after the loop (returning NULL)?

 Yes. Fixed that.

 > the construct with r = return_addrs in get_interface_address6() is a bit
 odd IMO; the test !r will never be true f.ex.

 The test !r will be true whenever we got there through goto err. (I've
 added a comment to make this clearer.)

 > are we leaking memory (the list in r) in get_interface_address()?

 Fixed.

 > slight code duplication in dirserv_orconn_tls_done() when calling
 node_set_last_reachability()

 Fixed.

 > is the check for the ipv6 address in dirserv_single_reachability_test()
 backwards? i.e. should check for tor_addr_compare() == 0

 Fixed.

 > did router_get_active_listener_port_by_addr_type_af() get the
 tor_addr_compare() test backwards too?

 Damnit, fixed.

 > the call to tor_free() in router_get_advertised_or_port_by_af() (at
 router.c:1508) is never reached and we're leaking that memory

 Fixed.

 > needs a ChangeLog entry (i.e. a file in changes/)

 I will create one when I know that I haven't missed something major or
 done something otherwise stupid.

 > documentation for last_discovered_ipv*_address refers to a function that
 does not exist (change to get_stable_interface_address6?)

 Fixed.

 > remove trailing whitespace

 Fixed.

 > no need to break a line like "port_cfg_t *
 router_get_main_ipv6_listener_address(const smartlist_t *ports);"

 I was under the impression that there is an 80-character by line limit,
 also in header files.

 > i'd add curly braces to constructs like [...]

 OK.

 > long lines here and there (see 'Wide' when doing "make check-spaces")

 Fixed.

 > indentation is wrong in a couple places (like address.c:1479)

 What do you mean? It looks fine to me.

 > no need for '\' to continue a line (like dirserv.c:2966)

 I wish I had an ISO keyboard with a large enter key :( (The backspace is
 right above Enter on mine.)

 > i'd break lines like this in two: [...]

 That's not strictly my code and appears like this also in circuitbuild.c,
 so I'll leave it alone.

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/9729#comment:18>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs