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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]