Refactor and unit-test all the code that looks at network interfaces to get current IP

Right now Tor has large hairy functions such as get_interface_address6() and get_interface_addresses_raw() that attempt to use system-specific API to try and get an IP address of network interface Tor instance is currently using. They should be refactored to improve code readability and maintainability. Also, we need to make sure there are unit tests for this kind of code.

changed milestone to %Tor: 0.2.6.x-final

added 026-defferrable 026-triaged-1 component::core tor/tor milestone::Tor: 0.2.6.x-final owner::rl1987 priority::medium refactoring resolution::implemented status::closed type::enhancement version::tor unspecified labels

Trac:
Milestone: N/A to Tor: 0.2.6.x-final

Trac:
Status: new to assigned
Owner: N/A to rl1987

I have covered most of resolve_my_address() function with tests:

https://github.com/rl1987/tor/tree/resolvemyaddr

Also, I have done some refactoring and unit testing regarding the rather messy get_interface_addresses_raw() function:

https://github.com/rl1987/tor/tree/if_addr_refactoring

What should I do next? What should be in the checklist of things to get done to get the above changes merged into Tor master?

Sorry about the delays!

reviewing tesolvemyaddr branch:

• tor_gethostname,(char *name, size_t namelen)) needs documentation.
• mock functions in test_config.c should documentation (eg, "This is a mock function to replace X; this replacement does Y.")
• Otherwise, this looks fine, assuming test coverage is okay

reviewing if_addr_refactoring branch:

• The tests look good, assuming coverage is okay
• Instead of duplicating the "+#ifdef HAVE_GETIFADDRS ...#elif define(_WIN32)) ...#elif defined(SIOCGIFCONF) && defined(HAVE_IOCTL) " checks in a few places, it's probably better to have address.h do "#define HAVE_IFADDRS_TO_SMARTLIST" in the first case, and #define HAVE_IP_ADAPTER_ADDRESSES_TO_SMARTLIST in the second case, and so on. Then the other code can check those more readable macros.
• All the new functions in address.c need documentation.
• Otherwise, looks good!

(For more info on our documentation style, and on getting test coverage, see doc/HACKING. )

Trac:
Status: assigned to needs_revision

Trac:
Keywords: refactoring deleted, refactoring 026-triaged-1 026-defferrable added

I believe I now have a 100 percent unit test coverage of resolve_my_address(). See resolvemyaddr branch.

Trac:
Status: needs_revision to needs_review

I made a bunch of fixes in my if_addr_refactoring branch in my public repository. Still looks okay to you?

Looking at resolvemyaddr now.

resolvemyaddr looks fairly solid. I made a few tweaks and merged. Thanks!

(Leaving this ticket open for the other branch.)

(waiting on windows unit tests)

I have covered the Windows specific code with unit tests. New unit-tests are passing both on Wine and on Windows 7. See my if_addr_refactoring branch.

The if_addr_refactoring branch is ready for review.

Looks good; let's try it out!

(squashed and merging to master.)

Trac:
Status: needs_review to closed
Resolution: N/A to implemented

closed

mentioned in issue #12377 (moved)

mentioned in issue #14875 (moved)

moved to tpo/core/tor#12376 (closed)

mentioned in issue tpo/core/tor#14875 (closed)