[tor-bugs] #18105 [Core Tor/Tor]: Replace getsockname with tor_getsockname

[Tor Bug Tracker & Wiki]

#18105: Replace getsockname with tor_getsockname
-------------------------------------------------+-------------------------
 Reporter:  teor                                 |          Owner:
                                                 |  eewayhsu
     Type:  enhancement                          |         Status:
                                                 |  needs_revision
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.3.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  easy, intro, api, code-              |  Actual Points:
  simplification                                 |
Parent ID:                                       |         Points:  small
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by teor):

 * status:  needs_review => needs_revision


Comment:

 Hi,

 Thanks for this revision!

 Replying to [comment:21 callumw]:
 > Thanks for the advice regarding the header files. I moved the function
 to address.c as you recommended and that resolved the issue.
 >
 > I have attached a new patch file with the new function implementation
 in. The first test under 'make check' fails but this behaviour can be
 replicated in a newly checkout-out repo so I assume it is unrelated to my
 changes here.

 `make check-spaces` will fail due to this line:
 {{{
 if(getsockname(sock, (struct sockaddr *)&sockaddr, &addr_len) < 0){ return
 -1; }
 }}}

 We have a coding style that puts spaces between `if` and `(`, and `)` and
 `{` and we typically have `if` blocks on multiple lines. Try to match the
 existing coding style, and run `make check-spaces` to confirm.

 Code review:

 I think we'd be ok with adding .vimrc to .gitignore, but we would want
 that in a separate commit. Alternately, you can add .vimrc to your own
 ~/.gitignore_global (there's a git config option for a global ignore
 file).

 We don't need to do the MOCK_IMPL/MOCK_DECL on these functions, because
 they're not being mocked in the unit tests.

 We should use tor_getsockname() in tor_addr_from_getsockname(), so we can
 mock tor_getsockname() in the unit tests if we need to.

 tor_addr_from_sockaddr() returns a port in port_out. So `uint16_t
 port_num` needs to be a pointer, and should probably be called something
 like `port_out`. Similarly, `tor_addr_t *tor_addr` should probably be
 called `addr_out`. And our convention is to put `out` parameters at the
 end of the argument list.

 The function needs a comment that describes what each argument does, and
 what the return value is. You can use the one on tor_addr_from_sockaddr()
 as an example.

 Next Steps:

 Once we introduce this function, we can use it to replace code that calls
 [tor_]getsockname() then tor_addr_from_sockaddr().

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/18105#comment:23>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online