[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #24546 [Core Tor/Tor]: Use tor_addr_is_v4() rather than family, or reject all v6-mapped IPv4 addresses

#24546: Use tor_addr_is_v4() rather than family, or reject all v6-mapped IPv4
addresses
-------------------------------------------------+-------------------------
 Reporter:  teor                                 |          Owner:  neel
     Type:  defect                               |         Status:
                                                 |  needs_revision
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  unspecified
Component:  Core Tor/Tor                         |        Version:  Tor:
                                                 |  unspecified
 Severity:  Normal                               |     Resolution:
 Keywords:  tor-dirauth, ipv6,                   |  Actual Points:
  033-triage-20180320, 033-removed-20180320,     |
  035-triaged-in-20180711,                       |
  040-deferred-20190220                          |
Parent ID:                                       |         Points:  1
 Reviewer:  ahf, teor                            |        Sponsor:
                                                 |  SponsorV-can
-------------------------------------------------+-------------------------
Changes (by teor):

 * status:  needs_review => needs_revision
 * reviewer:  ahf => ahf, teor


Comment:

 Thanks for this pull request. I did a review on the code.

 I am concerned about the size of the diff in commit 53ff26b. It changes
 over 100 lines. It contains at least one logic error that passed review.
 There could be more.

 Here are our options for moving forward:
 1. Use an automated tool like coccinelle to do the changes in 53ff26b.
 Then we can verify the output using the tool. We should also turn
 tor_addr_is_*() into inline header functions, so they are more efficient.
 2. Delete the function tor_addr_is_v4(), and replace all its uses with
 `tor_addr_family(addr) == AF_INET`. There are only 13 uses of
 tor_addr_is_v4() in master, so the diff should be much smaller and easier
 to review.

 I prefer option 2, because we will end up with fewer functions, and less
 code complexity. And the overall diff will be smaller.

 What do you think?

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/24546#comment:23>
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