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

Torsten Bögershausen [off-list ref] writes:

On 2015-10-30 15.48, Elia Pinto wrote:

quoted

Add IPv6 support by implementing name resolution with the

Minor question: How is this related to IPV6?
Could the header line be written something like

"ident.c: Use getaddrinfo() instead of gethostbyname() if available"

On which systems has the patch been tested ?
Linux ?
Mac OS X ?
Windows ?
BSD ?

The motivation on which platforms the usage of getaddrinfo() is preferred
over gethostbyname() could be helpful to motivate this patch:
System XYZ behaves bad when gethostbyname() is used.
Fix it by using getaddrinfo() instead.

gethostbyname() fills a hostent that gives us the official name,
list of aliases, _one_ addrtype (either AF_INET or AF_INET6), and
list of addresses, so if we were asking for the physical addresses,
we may not be able to obtain all addresses for a host with both IPv4
and IPv6 addresses, which may be an issue.

But this function is about learning the official name of the host,
given the result of gethostname().  In that context, does that
"limited to single address family" issue of gethostbyname() still
matter?  I am guessing it doesn't, and I somehow doubt that the
value of this patch is about working around any platform bug.

I think the real reason to favour getaddrinfo() over gethostbyname()
is that the family of functions the latter belongs to is obsolete.
In other codepaths where we need to learn the inet address, we
already use getaddrinfo() if available (i.e. NO_IPV6 is not set),
and gethostbyname() is used only when compiling with NO_IPV6.

Converting this codepath to match that pattern incidentally allows
you to work around a platform bugs like "gethostbyname() is broken
on sysmte XYZ" (which you work around by not saying NO_IPV6), but I
view it as a side effect.

quoted

+static void add_domainname(struct strbuf *out)
+{
+	char buf[1024];
+	struct addrinfo hints, *ai;
+	int gai;

The scope of these variables can be narrowed, by moving them into the "{" block,
where they are needed. (Before the memset())

quoted

+
+	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	{

Many ' ' between else and '{', one should be enough

quoted

+		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

Colud be written in one line as "} else"

quoted

+			strbuf_addf(out, "%s.(none)", buf);
+	}
+}
+#else /* NO_IPV6 */