Re: [PATCH 4/5] tcp: unify ipv4 and ipv6 code paths
- From
Erik Faye-Lund <kusmabite@gmail.com>
- Date
- Mar 8, 2012, 15:39 UTC
- Message-ID
- <CABPQNSYfv19cVQoAoUyXVaF1TpLXTYDRFnHE4vr=X42W771tbA@mail.gmail.com>
- In-Reply-To
- <20120308130913.GD9426@burratino>
On Thu, Mar 8, 2012 at 2:09 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
Show 43 quoted lines
> 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?