Re: [PATCH 4/5] tcp: unify ipv4 and ipv6 code paths

[Erik Faye-Lund <kusmabite@gmail.com>]

On Thu, Mar 8, 2012 at 2:09 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Date: Mon, 6 Jun 2011 04:41:28 -0500
>
> The new DNS API abstracts away differences between the gethostbyname-
> and getaddrinfo-centric interfaces for looking up a host, making the
> code to use them in connect.c a little easier to read.
>
> To make a lookup:
>
>        resolver_result ai;
>        dns_resolve(host, port, 0, &ai);
>        ...
>        dns_free(ai);
>
> To iterate over responses:
>
>        resolved_address i;
>        for_each_address(i, ai) {
>                ...
>        }
>
> In the !NO_IPV6 codepath, the git_locate_host function that is used to
> find the canonical IP and hostname for a git server's public address
> (for virtual hosting) tells getaddrinfo to restrict attention to TCP
> services after this patch.  That should make no difference because the
> service parameter is NULL.
>
> No functional change intended.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> This is the title feature, corresponding to
> http://thread.gmane.org/gmane.comp.version-control.git/175106/focus=175111
>
> It needed changes to adjust to released changes in the code it touches,
> but nothing is fundamentally different from v1.
>
>  Makefile   |    5 ++
>  dns-ipv4.c |   33 +++++++++++
>  dns-ipv4.h |   69 +++++++++++++++++++++++
>  dns-ipv6.c |   49 ++++++++++++++++
>  dns-ipv6.h |   31 +++++++++++
>  tcp.c      |  182 +++++++++++-------------------------------------------------
>  6 files changed, 218 insertions(+), 151 deletions(-)

I'm not entirely sure I understand the motivation here. We already had
well-tested, implementations of IPv4 and IPv6 tcp-socket setup. Here
you unify the code by adding abstraction, but it ends up amounting to
more lines of code, with the details scattered around in different
source files.

For me, this means that I have to learn a new API, and to see what
really happens when something goes wrong, I have to jump between
multiple source files.

And I'm not entirely sure what this patch actually improves. If it was
likely that we'd get support for yet another IP-stack version, then
this would probably be a win. But that's not likely, is it?