Re: [PATCH] ident.c: add support for IPv6

[Jeff King <peff@peff.net>]

On Fri, Oct 30, 2015 at 03:48:07PM +0100, Elia Pinto wrote:

> Add IPv6 support by implementing name resolution with the
> protocol agnostic getaddrinfo(3) API. The old gethostbyname(3)
> code is still available when git is compiled with NO_IPV6.

Makes sense. I'm not excited by the duplication in the early part of the
function, though:

> +#ifndef NO_IPV6
> +
> +static void add_domainname(struct strbuf *out)
> +{
> +	char buf[1024];
> +	struct addrinfo hints, *ai;
> +	int gai;
> +
> +	if (gethostname(buf, sizeof(buf))) {
> +		warning("cannot get host name: %s", strerror(errno));
> +		strbuf_addstr(out, "(none)");
> +		return;
> +	}
> +	if (strchr(buf, '.'))
> +		strbuf_addstr(out, buf);
> +	else	{
> +		memset (&hints, '\0', sizeof (hints));
> +		hints.ai_flags = AI_CANONNAME;
> +		if (!(gai = getaddrinfo(buf, NULL, &hints, &ai)) && ai && strchr(ai->ai_canonname, '.')) {
> +			strbuf_addstr(out, ai->ai_canonname);
> +			freeaddrinfo(ai);
> +		}
> +		else
> +			strbuf_addf(out, "%s.(none)", buf);
> +	}
> +}

Especially the "(none)" stuff is ugly enough as it is, without being
duplicated in two spots. Can we factor out the else clause that calls
gethostbyname(), and just override that part with the #ifdef?

For that matter, we have a few other spots that use getaddrinfo and
#ifdef. I wonder if it would be possible to simply use getaddrinfo
everywhere, and make a compatibility wrapper that uses gethostbyname for
older systems. The cut-and-paste duplication in connect.c, for example,
is pretty egregious.

-Peff