[PATCH 0/5] transport: unify ipv4 and ipv6 code paths

[PATCH 0/5] transport: unify ipv4 and ipv6 code paths

[Jonathan Nieder]

Hi,

These patches eliminate some ifdef-ery concerning NO_IPV6.  I used
them when writing the SRV patch, which applies on top, but it's
probably best to think of it as an independent topic.

Patch 4 is the heart of the series.  It provides an interface similar
to getaddrinfo that can be implemented on top of either gethostbyname
or getaddrinfo and puts each implementation in a separate file.  This
way, callers can just use the common API and they do not need to have
two copies of their code, one for each host resolution API.

Patches 1-3 move code around until all the code that patch 4 touches
is in one place.

Patches 5 is a potential error handling improvement noticed while
writing patches 1-4.  It's probably not actually needed but it was a
comfort to me.

These patches have been in use in Debian since June of last year.  I'd
like to see this in mainline early in the 1.7.11 cycle to make coding
that touches this area during that cycle more pleasant.  Thoughts of
all kinds welcome.

Jonathan Nieder (5):
  transport: expose git_tcp_connect and friends in new tcp.h
  daemon: make host resolution into a separate function
  daemon: move locate_host() to tcp.c
  tcp: unify ipv4 and ipv6 code paths
  daemon: check for errors retrieving IP address

 Makefile   |    7 ++
 connect.c  |  277 +-----------------------------------------------------------
 daemon.c   |   58 ++-----------
 dns-ipv4.c |   33 ++++++++
 dns-ipv4.h |   68 +++++++++++++++
 dns-ipv6.c |   49 +++++++++++
 dns-ipv6.h |   31 +++++++
 tcp.c      |  217 +++++++++++++++++++++++++++++++++++++++++++++++
 tcp.h      |   11 +++
 9 files changed, 422 insertions(+), 329 deletions(-)
 create mode 100644 dns-ipv4.c
 create mode 100644 dns-ipv4.h
 create mode 100644 dns-ipv6.c
 create mode 100644 dns-ipv6.h
 create mode 100644 tcp.c
 create mode 100644 tcp.h

[PATCH 1/5] transport: expose git_tcp_connect() and friends in new tcp.h

[Jonathan Nieder]

Date: Mon, 6 Jun 2011 04:37:14 -0500

Split off a new tcp.c with the functions git_tcp_connect() and
git_proxy_connect() that resolve and connect to a host.

Part of a series to teach git to respect DNS SRV records when making
new connections.  This is a preliminary step to make the connection
library easier to understand before changing it.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
From http://thread.gmane.org/gmane.comp.version-control.git/175106/focus=175107

Reviewing mass code migration is a little tricky.  My current favorite
strategy is

	git show connect.c Makefile tcp.h
	git diff HEAD^:connect.c HEAD:tcp.c

to make sure nothing important was added or dropped.

 Makefile  |    2 +
 connect.c |  277 +-----------------------------------------------------------
 tcp.c     |  278 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tcp.h     |    8 ++
 4 files changed, 289 insertions(+), 276 deletions(-)
 create mode 100644 tcp.c
 create mode 100644 tcp.h

diff --git a/Makefile b/Makefile
index e4f8e0ef..0d0ac31d 100644
--- a/Makefile
+++ b/Makefile
@@ -736,6 +736,7 @@ LIB_OBJS += string-list.o
 LIB_OBJS += submodule.o
 LIB_OBJS += symlinks.o
 LIB_OBJS += tag.o
+LIB_OBJS += tcp.o
 LIB_OBJS += trace.o
 LIB_OBJS += transport.o
 LIB_OBJS += transport-helper.o
@@ -2148,6 +2149,7 @@ builtin/prune.o builtin/reflog.o reachable.o: reachable.h
 builtin/commit.o builtin/revert.o wt-status.o: wt-status.h
 builtin/tar-tree.o archive-tar.o: tar.h
 connect.o transport.o url.o http-backend.o: url.h
+connect.o tcp.o: tcp.h
 http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h
 http.o http-walker.o http-push.o http-fetch.o remote-curl.o: http.h url.h
 
diff --git a/connect.c b/connect.c
index 912cddee..962dc030 100644
--- a/connect.c
+++ b/connect.c
@@ -5,6 +5,7 @@
 #include "refs.h"
 #include "run-command.h"
 #include "remote.h"
+#include "tcp.h"
 #include "url.h"
 
 static char *server_capabilities;
@@ -145,282 +146,6 @@ static enum protocol get_protocol(const char *name)
 	die("I don't handle protocol '%s'", name);
 }
 
-#define STR_(s)	# s
-#define STR(s)	STR_(s)
-
-static void get_host_and_port(char **host, const char **port)
-{
-	char *colon, *end;
-
-	if (*host[0] == '[') {
-		end = strchr(*host + 1, ']');
-		if (end) {
-			*end = 0;
-			end++;
-			(*host)++;
-		} else
-			end = *host;
-	} else
-		end = *host;
-	colon = strchr(end, ':');
-
-	if (colon) {
-		*colon = 0;
-		*port = colon + 1;
-	}
-}
-
-static void enable_keepalive(int sockfd)
-{
-	int ka = 1;
-
-	if (setsockopt(sockfd, SOL_SOCKET, SO_KEEPALIVE, &ka, sizeof(ka)) < 0)
-		fprintf(stderr, "unable to set SO_KEEPALIVE on socket: %s\n",
-			strerror(errno));
-}
-
-#ifndef NO_IPV6
-
-static const char *ai_name(const struct addrinfo *ai)
-{
-	static char addr[NI_MAXHOST];
-	if (getnameinfo(ai->ai_addr, ai->ai_addrlen, addr, sizeof(addr), NULL, 0,
-			NI_NUMERICHOST) != 0)
-		strcpy(addr, "(unknown)");
-
-	return addr;
-}
-
-/*
- * Returns a connected socket() fd, or else die()s.
- */
-static int git_tcp_connect_sock(char *host, int flags)
-{
-	struct strbuf error_message = STRBUF_INIT;
-	int sockfd = -1;
-	const char *port = STR(DEFAULT_GIT_PORT);
-	struct addrinfo hints, *ai0, *ai;
-	int gai;
-	int cnt = 0;
-
-	get_host_and_port(&host, &port);
-	if (!*port)
-		port = "<none>";
-
-	memset(&hints, 0, sizeof(hints));
-	hints.ai_socktype = SOCK_STREAM;
-	hints.ai_protocol = IPPROTO_TCP;
-
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "Looking up %s ... ", host);
-
-	gai = getaddrinfo(host, port, &hints, &ai);
-	if (gai)
-		die("Unable to look up %s (port %s) (%s)", host, port, gai_strerror(gai));
-
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port);
-
-	for (ai0 = ai; ai; ai = ai->ai_next, cnt++) {
-		sockfd = socket(ai->ai_family,
-				ai->ai_socktype, ai->ai_protocol);
-		if ((sockfd < 0) ||
-		    (connect(sockfd, ai->ai_addr, ai->ai_addrlen) < 0)) {
-			strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n",
-				    host, cnt, ai_name(ai), strerror(errno));
-			if (0 <= sockfd)
-				close(sockfd);
-			sockfd = -1;
-			continue;
-		}
-		if (flags & CONNECT_VERBOSE)
-			fprintf(stderr, "%s ", ai_name(ai));
-		break;
-	}
-
-	freeaddrinfo(ai0);
-
-	if (sockfd < 0)
-		die("unable to connect to %s:\n%s", host, error_message.buf);
-
-	enable_keepalive(sockfd);
-
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "done.\n");
-
-	strbuf_release(&error_message);
-
-	return sockfd;
-}
-
-#else /* NO_IPV6 */
-
-/*
- * Returns a connected socket() fd, or else die()s.
- */
-static int git_tcp_connect_sock(char *host, int flags)
-{
-	struct strbuf error_message = STRBUF_INIT;
-	int sockfd = -1;
-	const char *port = STR(DEFAULT_GIT_PORT);
-	char *ep;
-	struct hostent *he;
-	struct sockaddr_in sa;
-	char **ap;
-	unsigned int nport;
-	int cnt;
-
-	get_host_and_port(&host, &port);
-
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "Looking up %s ... ", host);
-
-	he = gethostbyname(host);
-	if (!he)
-		die("Unable to look up %s (%s)", host, hstrerror(h_errno));
-	nport = strtoul(port, &ep, 10);
-	if ( ep == port || *ep ) {
-		/* Not numeric */
-		struct servent *se = getservbyname(port,"tcp");
-		if ( !se )
-			die("Unknown port %s", port);
-		nport = se->s_port;
-	}
-
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port);
-
-	for (cnt = 0, ap = he->h_addr_list; *ap; ap++, cnt++) {
-		memset(&sa, 0, sizeof sa);
-		sa.sin_family = he->h_addrtype;
-		sa.sin_port = htons(nport);
-		memcpy(&sa.sin_addr, *ap, he->h_length);
-
-		sockfd = socket(he->h_addrtype, SOCK_STREAM, 0);
-		if ((sockfd < 0) ||
-		    connect(sockfd, (struct sockaddr *)&sa, sizeof sa) < 0) {
-			strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n",
-				host,
-				cnt,
-				inet_ntoa(*(struct in_addr *)&sa.sin_addr),
-				strerror(errno));
-			if (0 <= sockfd)
-				close(sockfd);
-			sockfd = -1;
-			continue;
-		}
-		if (flags & CONNECT_VERBOSE)
-			fprintf(stderr, "%s ",
-				inet_ntoa(*(struct in_addr *)&sa.sin_addr));
-		break;
-	}
-
-	if (sockfd < 0)
-		die("unable to connect to %s:\n%s", host, error_message.buf);
-
-	enable_keepalive(sockfd);
-
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "done.\n");
-
-	return sockfd;
-}
-
-#endif /* NO_IPV6 */
-
-
-static void git_tcp_connect(int fd[2], char *host, int flags)
-{
-	int sockfd = git_tcp_connect_sock(host, flags);
-
-	fd[0] = sockfd;
-	fd[1] = dup(sockfd);
-}
-
-
-static char *git_proxy_command;
-
-static int git_proxy_command_options(const char *var, const char *value,
-		void *cb)
-{
-	if (!strcmp(var, "core.gitproxy")) {
-		const char *for_pos;
-		int matchlen = -1;
-		int hostlen;
-		const char *rhost_name = cb;
-		int rhost_len = strlen(rhost_name);
-
-		if (git_proxy_command)
-			return 0;
-		if (!value)
-			return config_error_nonbool(var);
-		/* [core]
-		 * ;# matches www.kernel.org as well
-		 * gitproxy = netcatter-1 for kernel.org
-		 * gitproxy = netcatter-2 for sample.xz
-		 * gitproxy = netcatter-default
-		 */
-		for_pos = strstr(value, " for ");
-		if (!for_pos)
-			/* matches everybody */
-			matchlen = strlen(value);
-		else {
-			hostlen = strlen(for_pos + 5);
-			if (rhost_len < hostlen)
-				matchlen = -1;
-			else if (!strncmp(for_pos + 5,
-					  rhost_name + rhost_len - hostlen,
-					  hostlen) &&
-				 ((rhost_len == hostlen) ||
-				  rhost_name[rhost_len - hostlen -1] == '.'))
-				matchlen = for_pos - value;
-			else
-				matchlen = -1;
-		}
-		if (0 <= matchlen) {
-			/* core.gitproxy = none for kernel.org */
-			if (matchlen == 4 &&
-			    !memcmp(value, "none", 4))
-				matchlen = 0;
-			git_proxy_command = xmemdupz(value, matchlen);
-		}
-		return 0;
-	}
-
-	return git_default_config(var, value, cb);
-}
-
-static int git_use_proxy(const char *host)
-{
-	git_proxy_command = getenv("GIT_PROXY_COMMAND");
-	git_config(git_proxy_command_options, (void*)host);
-	return (git_proxy_command && *git_proxy_command);
-}
-
-static struct child_process *git_proxy_connect(int fd[2], char *host)
-{
-	const char *port = STR(DEFAULT_GIT_PORT);
-	const char **argv;
-	struct child_process *proxy;
-
-	get_host_and_port(&host, &port);
-
-	argv = xmalloc(sizeof(*argv) * 4);
-	argv[0] = git_proxy_command;
-	argv[1] = host;
-	argv[2] = port;
-	argv[3] = NULL;
-	proxy = xcalloc(1, sizeof(*proxy));
-	proxy->argv = argv;
-	proxy->in = -1;
-	proxy->out = -1;
-	if (start_command(proxy))
-		die("cannot start proxy %s", argv[0]);
-	fd[0] = proxy->out; /* read from proxy stdout */
-	fd[1] = proxy->in;  /* write to proxy stdin */
-	return proxy;
-}
-
 #define MAX_CMD_LEN 1024
 
 static char *get_port(char *host)
diff --git a/tcp.c b/tcp.c
new file mode 100644
index 00000000..f5e1ab37
--- /dev/null
+++ b/tcp.c
@@ -0,0 +1,278 @@
+#include "cache.h"
+#include "run-command.h"
+
+#define STR_(s)	# s
+#define STR(s)	STR_(s)
+
+static void get_host_and_port(char **host, const char **port)
+{
+	char *colon, *end;
+
+	if (*host[0] == '[') {
+		end = strchr(*host + 1, ']');
+		if (end) {
+			*end = 0;
+			end++;
+			(*host)++;
+		} else
+			end = *host;
+	} else
+		end = *host;
+	colon = strchr(end, ':');
+
+	if (colon) {
+		*colon = 0;
+		*port = colon + 1;
+	}
+}
+
+static void enable_keepalive(int sockfd)
+{
+	int ka = 1;
+
+	if (setsockopt(sockfd, SOL_SOCKET, SO_KEEPALIVE, &ka, sizeof(ka)) < 0)
+		fprintf(stderr, "unable to set SO_KEEPALIVE on socket: %s\n",
+			strerror(errno));
+}
+
+#ifndef NO_IPV6
+
+static const char *ai_name(const struct addrinfo *ai)
+{
+	static char addr[NI_MAXHOST];
+	if (getnameinfo(ai->ai_addr, ai->ai_addrlen, addr, sizeof(addr), NULL, 0,
+			NI_NUMERICHOST) != 0)
+		strcpy(addr, "(unknown)");
+
+	return addr;
+}
+
+/*
+ * Returns a connected socket() fd, or else die()s.
+ */
+static int git_tcp_connect_sock(char *host, int flags)
+{
+	struct strbuf error_message = STRBUF_INIT;
+	int sockfd = -1;
+	const char *port = STR(DEFAULT_GIT_PORT);
+	struct addrinfo hints, *ai0, *ai;
+	int gai;
+	int cnt = 0;
+
+	get_host_and_port(&host, &port);
+	if (!*port)
+		port = "<none>";
+
+	memset(&hints, 0, sizeof(hints));
+	hints.ai_socktype = SOCK_STREAM;
+	hints.ai_protocol = IPPROTO_TCP;
+
+	if (flags & CONNECT_VERBOSE)
+		fprintf(stderr, "Looking up %s ... ", host);
+
+	gai = getaddrinfo(host, port, &hints, &ai);
+	if (gai)
+		die("Unable to look up %s (port %s) (%s)", host, port, gai_strerror(gai));
+
+	if (flags & CONNECT_VERBOSE)
+		fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port);
+
+	for (ai0 = ai; ai; ai = ai->ai_next, cnt++) {
+		sockfd = socket(ai->ai_family,
+				ai->ai_socktype, ai->ai_protocol);
+		if ((sockfd < 0) ||
+		    (connect(sockfd, ai->ai_addr, ai->ai_addrlen) < 0)) {
+			strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n",
+				    host, cnt, ai_name(ai), strerror(errno));
+			if (0 <= sockfd)
+				close(sockfd);
+			sockfd = -1;
+			continue;
+		}
+		if (flags & CONNECT_VERBOSE)
+			fprintf(stderr, "%s ", ai_name(ai));
+		break;
+	}
+
+	freeaddrinfo(ai0);
+
+	if (sockfd < 0)
+		die("unable to connect to %s:\n%s", host, error_message.buf);
+
+	enable_keepalive(sockfd);
+
+	if (flags & CONNECT_VERBOSE)
+		fprintf(stderr, "done.\n");
+
+	strbuf_release(&error_message);
+
+	return sockfd;
+}
+
+#else /* NO_IPV6 */
+
+/*
+ * Returns a connected socket() fd, or else die()s.
+ */
+static int git_tcp_connect_sock(char *host, int flags)
+{
+	struct strbuf error_message = STRBUF_INIT;
+	int sockfd = -1;
+	const char *port = STR(DEFAULT_GIT_PORT);
+	char *ep;
+	struct hostent *he;
+	struct sockaddr_in sa;
+	char **ap;
+	unsigned int nport;
+	int cnt;
+
+	get_host_and_port(&host, &port);
+
+	if (flags & CONNECT_VERBOSE)
+		fprintf(stderr, "Looking up %s ... ", host);
+
+	he = gethostbyname(host);
+	if (!he)
+		die("Unable to look up %s (%s)", host, hstrerror(h_errno));
+	nport = strtoul(port, &ep, 10);
+	if ( ep == port || *ep ) {
+		/* Not numeric */
+		struct servent *se = getservbyname(port,"tcp");
+		if ( !se )
+			die("Unknown port %s", port);
+		nport = se->s_port;
+	}
+
+	if (flags & CONNECT_VERBOSE)
+		fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port);
+
+	for (cnt = 0, ap = he->h_addr_list; *ap; ap++, cnt++) {
+		memset(&sa, 0, sizeof sa);
+		sa.sin_family = he->h_addrtype;
+		sa.sin_port = htons(nport);
+		memcpy(&sa.sin_addr, *ap, he->h_length);
+
+		sockfd = socket(he->h_addrtype, SOCK_STREAM, 0);
+		if ((sockfd < 0) ||
+		    connect(sockfd, (struct sockaddr *)&sa, sizeof sa) < 0) {
+			strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n",
+				host,
+				cnt,
+				inet_ntoa(*(struct in_addr *)&sa.sin_addr),
+				strerror(errno));
+			if (0 <= sockfd)
+				close(sockfd);
+			sockfd = -1;
+			continue;
+		}
+		if (flags & CONNECT_VERBOSE)
+			fprintf(stderr, "%s ",
+				inet_ntoa(*(struct in_addr *)&sa.sin_addr));
+		break;
+	}
+
+	if (sockfd < 0)
+		die("unable to connect to %s:\n%s", host, error_message.buf);
+
+	enable_keepalive(sockfd);
+
+	if (flags & CONNECT_VERBOSE)
+		fprintf(stderr, "done.\n");
+
+	return sockfd;
+}
+
+#endif /* NO_IPV6 */
+
+
+void git_tcp_connect(int fd[2], char *host, int flags)
+{
+	int sockfd = git_tcp_connect_sock(host, flags);
+
+	fd[0] = sockfd;
+	fd[1] = dup(sockfd);
+}
+
+
+static char *git_proxy_command;
+
+static int git_proxy_command_options(const char *var, const char *value,
+		void *cb)
+{
+	if (!strcmp(var, "core.gitproxy")) {
+		const char *for_pos;
+		int matchlen = -1;
+		int hostlen;
+		const char *rhost_name = cb;
+		int rhost_len = strlen(rhost_name);
+
+		if (git_proxy_command)
+			return 0;
+		if (!value)
+			return config_error_nonbool(var);
+		/* [core]
+		 * ;# matches www.kernel.org as well
+		 * gitproxy = netcatter-1 for kernel.org
+		 * gitproxy = netcatter-2 for sample.xz
+		 * gitproxy = netcatter-default
+		 */
+		for_pos = strstr(value, " for ");
+		if (!for_pos)
+			/* matches everybody */
+			matchlen = strlen(value);
+		else {
+			hostlen = strlen(for_pos + 5);
+			if (rhost_len < hostlen)
+				matchlen = -1;
+			else if (!strncmp(for_pos + 5,
+					  rhost_name + rhost_len - hostlen,
+					  hostlen) &&
+				 ((rhost_len == hostlen) ||
+				  rhost_name[rhost_len - hostlen -1] == '.'))
+				matchlen = for_pos - value;
+			else
+				matchlen = -1;
+		}
+		if (0 <= matchlen) {
+			/* core.gitproxy = none for kernel.org */
+			if (matchlen == 4 &&
+			    !memcmp(value, "none", 4))
+				matchlen = 0;
+			git_proxy_command = xmemdupz(value, matchlen);
+		}
+		return 0;
+	}
+
+	return git_default_config(var, value, cb);
+}
+
+int git_use_proxy(const char *host)
+{
+	git_proxy_command = getenv("GIT_PROXY_COMMAND");
+	git_config(git_proxy_command_options, (void*)host);
+	return (git_proxy_command && *git_proxy_command);
+}
+
+struct child_process *git_proxy_connect(int fd[2], char *host)
+{
+	const char *port = STR(DEFAULT_GIT_PORT);
+	const char **argv;
+	struct child_process *proxy;
+
+	get_host_and_port(&host, &port);
+
+	argv = xmalloc(sizeof(*argv) * 4);
+	argv[0] = git_proxy_command;
+	argv[1] = host;
+	argv[2] = port;
+	argv[3] = NULL;
+	proxy = xcalloc(1, sizeof(*proxy));
+	proxy->argv = argv;
+	proxy->in = -1;
+	proxy->out = -1;
+	if (start_command(proxy))
+		die("cannot start proxy %s", argv[0]);
+	fd[0] = proxy->out; /* read from proxy stdout */
+	fd[1] = proxy->in;  /* write to proxy stdin */
+	return proxy;
+}
diff --git a/tcp.h b/tcp.h
new file mode 100644
index 00000000..4de5f712
--- /dev/null
+++ b/tcp.h
@@ -0,0 +1,8 @@
+#ifndef TCP_H
+#define TCP_H
+
+extern int git_use_proxy(const char *host);
+extern void git_tcp_connect(int fd[2], char *host, int flags);
+extern struct child_process *git_proxy_connect(int fd[2], char *host);
+
+#endif
-- 
1.7.9.2

[PATCH 2/5] daemon: make host resolution a separate function

[Jonathan Nieder]

Date: Mon, 6 Jun 2011 04:38:45 -0500

The locate_host() function looks up the IP address and canonical
hostname of the host named by its argument.  If it succeeds,
*ip_address and *canon_hostname are freed and replaced by the hosts'
IP address and canonical hostname, respectively, as strings.  If it
fails, *ip_address and *canon_hostname are left alone.

The git daemon uses this functionality to support the %IP and %CH
placeholders for its --interpolated-path feature.  Splitting it out as
a separate function would make it easier to tweak, for example to
unify the ipv6 and ipv4 code paths or to share code with other parts
of git that make DNS queries.

Signed-off-by; Jonathan Nieder <jrnieder@gmail.com>
---
From http://thread.gmane.org/gmane.comp.version-control.git/175106/focus=175108

The commit message was tweaked, but the patch is the same as before.

 daemon.c |  112 +++++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 63 insertions(+), 49 deletions(-)

diff --git a/daemon.c b/daemon.c
index 15ce918a..2a9dfea0 100644
--- a/daemon.c
+++ b/daemon.c
@@ -441,6 +441,65 @@ static void parse_host_and_port(char *hostport, char **host,
 	}
 }
 
+#ifndef NO_IPV6
+
+static void locate_host(const char *hostname, char **ip_address,
+						char **canon_hostname)
+{
+	struct addrinfo hints;
+	struct addrinfo *ai;
+	int gai;
+	static char addrbuf[HOST_NAME_MAX + 1];
+	struct sockaddr_in *sin_addr;
+
+	memset(&hints, 0, sizeof(hints));
+	hints.ai_flags = AI_CANONNAME;
+
+	gai = getaddrinfo(hostname, NULL, &hints, &ai);
+	if (gai)
+		return;
+
+	sin_addr = (void *)ai->ai_addr;
+	inet_ntop(AF_INET, &sin_addr->sin_addr, addrbuf, sizeof(addrbuf));
+	free(*ip_address);
+	*ip_address = xstrdup(addrbuf);
+
+	free(*canon_hostname);
+	*canon_hostname = xstrdup(ai->ai_canonname ?
+				  ai->ai_canonname : *ip_address);
+
+	freeaddrinfo(ai);
+}
+
+#else
+
+static void locate_host(const char *hostname, char **ip_address,
+						char **canon_hostname)
+{
+	struct hostent *hent;
+	struct sockaddr_in sa;
+	char **ap;
+	static char addrbuf[HOST_NAME_MAX + 1];
+
+	hent = gethostbyname(hostname);
+
+	ap = hent->h_addr_list;
+	memset(&sa, 0, sizeof sa);
+	sa.sin_family = hent->h_addrtype;
+	sa.sin_port = htons(0);
+	memcpy(&sa.sin_addr, *ap, hent->h_length);
+
+	inet_ntop(hent->h_addrtype, &sa.sin_addr,
+		  addrbuf, sizeof(addrbuf));
+
+	free(*canon_hostname);
+	*canon_hostname = xstrdup(hent->h_name);
+	free(*ip_address);
+	*ip_address = xstrdup(addrbuf);
+}
+
+#endif
+
 /*
  * Read the host as supplied by the client connection.
  */
@@ -476,56 +535,11 @@ static void parse_host_arg(char *extra_args, int buflen)
 	}
 
 	/*
-	 * Locate canonical hostname and its IP address.
+	 * Locate canonical hostname and its IP address,
+	 * if possible.
 	 */
-	if (hostname) {
-#ifndef NO_IPV6
-		struct addrinfo hints;
-		struct addrinfo *ai;
-		int gai;
-		static char addrbuf[HOST_NAME_MAX + 1];
-
-		memset(&hints, 0, sizeof(hints));
-		hints.ai_flags = AI_CANONNAME;
-
-		gai = getaddrinfo(hostname, NULL, &hints, &ai);
-		if (!gai) {
-			struct sockaddr_in *sin_addr = (void *)ai->ai_addr;
-
-			inet_ntop(AF_INET, &sin_addr->sin_addr,
-				  addrbuf, sizeof(addrbuf));
-			free(ip_address);
-			ip_address = xstrdup(addrbuf);
-
-			free(canon_hostname);
-			canon_hostname = xstrdup(ai->ai_canonname ?
-						 ai->ai_canonname : ip_address);
-
-			freeaddrinfo(ai);
-		}
-#else
-		struct hostent *hent;
-		struct sockaddr_in sa;
-		char **ap;
-		static char addrbuf[HOST_NAME_MAX + 1];
-
-		hent = gethostbyname(hostname);
-
-		ap = hent->h_addr_list;
-		memset(&sa, 0, sizeof sa);
-		sa.sin_family = hent->h_addrtype;
-		sa.sin_port = htons(0);
-		memcpy(&sa.sin_addr, *ap, hent->h_length);
-
-		inet_ntop(hent->h_addrtype, &sa.sin_addr,
-			  addrbuf, sizeof(addrbuf));
-
-		free(canon_hostname);
-		canon_hostname = xstrdup(hent->h_name);
-		free(ip_address);
-		ip_address = xstrdup(addrbuf);
-#endif
-	}
+	if (hostname)
+		locate_host(hostname, &ip_address, &canon_hostname);
 }
 
 
-- 
1.7.9.2

[PATCH 3/5] daemon: move locate_host() to tcp lib

[Jonathan Nieder]

Date: Mon, 6 Jun 2011 04:39:29 -0500

Keep the different name resolution functions close together so they
can learn from each other and perhaps share code in the future.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
From http://thread.gmane.org/gmane.comp.version-control.git/175106/focus=175108

The original forgot to move HOST_NAME_MAX, causing build failures on systems
that lack it.

 Makefile |    2 +-
 daemon.c |   66 ++------------------------------------------------------------
 tcp.c    |   57 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tcp.h    |    3 +++
 4 files changed, 63 insertions(+), 65 deletions(-)

diff --git a/Makefile b/Makefile
index 0d0ac31d..927a079a 100644
--- a/Makefile
+++ b/Makefile
@@ -2149,7 +2149,7 @@ builtin/prune.o builtin/reflog.o reachable.o: reachable.h
 builtin/commit.o builtin/revert.o wt-status.o: wt-status.h
 builtin/tar-tree.o archive-tar.o: tar.h
 connect.o transport.o url.o http-backend.o: url.h
-connect.o tcp.o: tcp.h
+connect.o daemon.o tcp.o: tcp.h
 http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h
 http.o http-walker.o http-push.o http-fetch.o remote-curl.o: http.h url.h
 
diff --git a/daemon.c b/daemon.c
index 2a9dfea0..3736fe53 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1,14 +1,11 @@
 #include "cache.h"
 #include "pkt-line.h"
+#include "tcp.h"
 #include "exec_cmd.h"
 #include "run-command.h"
 #include "strbuf.h"
 #include "string-list.h"
 
-#ifndef HOST_NAME_MAX
-#define HOST_NAME_MAX 256
-#endif
-
 #ifndef NI_MAXSERV
 #define NI_MAXSERV 32
 #endif
@@ -441,65 +438,6 @@ static void parse_host_and_port(char *hostport, char **host,
 	}
 }
 
-#ifndef NO_IPV6
-
-static void locate_host(const char *hostname, char **ip_address,
-						char **canon_hostname)
-{
-	struct addrinfo hints;
-	struct addrinfo *ai;
-	int gai;
-	static char addrbuf[HOST_NAME_MAX + 1];
-	struct sockaddr_in *sin_addr;
-
-	memset(&hints, 0, sizeof(hints));
-	hints.ai_flags = AI_CANONNAME;
-
-	gai = getaddrinfo(hostname, NULL, &hints, &ai);
-	if (gai)
-		return;
-
-	sin_addr = (void *)ai->ai_addr;
-	inet_ntop(AF_INET, &sin_addr->sin_addr, addrbuf, sizeof(addrbuf));
-	free(*ip_address);
-	*ip_address = xstrdup(addrbuf);
-
-	free(*canon_hostname);
-	*canon_hostname = xstrdup(ai->ai_canonname ?
-				  ai->ai_canonname : *ip_address);
-
-	freeaddrinfo(ai);
-}
-
-#else
-
-static void locate_host(const char *hostname, char **ip_address,
-						char **canon_hostname)
-{
-	struct hostent *hent;
-	struct sockaddr_in sa;
-	char **ap;
-	static char addrbuf[HOST_NAME_MAX + 1];
-
-	hent = gethostbyname(hostname);
-
-	ap = hent->h_addr_list;
-	memset(&sa, 0, sizeof sa);
-	sa.sin_family = hent->h_addrtype;
-	sa.sin_port = htons(0);
-	memcpy(&sa.sin_addr, *ap, hent->h_length);
-
-	inet_ntop(hent->h_addrtype, &sa.sin_addr,
-		  addrbuf, sizeof(addrbuf));
-
-	free(*canon_hostname);
-	*canon_hostname = xstrdup(hent->h_name);
-	free(*ip_address);
-	*ip_address = xstrdup(addrbuf);
-}
-
-#endif
-
 /*
  * Read the host as supplied by the client connection.
  */
@@ -539,7 +477,7 @@ static void parse_host_arg(char *extra_args, int buflen)
 	 * if possible.
 	 */
 	if (hostname)
-		locate_host(hostname, &ip_address, &canon_hostname);
+		git_locate_host(hostname, &ip_address, &canon_hostname);
 }
 
 
diff --git a/tcp.c b/tcp.c
index f5e1ab37..9263e0d2 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1,6 +1,10 @@
 #include "cache.h"
 #include "run-command.h"
 
+#ifndef HOST_NAME_MAX
+#define HOST_NAME_MAX 256
+#endif
+
 #define STR_(s)	# s
 #define STR(s)	STR_(s)
 
@@ -47,6 +51,34 @@ static const char *ai_name(const struct addrinfo *ai)
 	return addr;
 }
 
+void git_locate_host(const char *hostname, char **ip_address,
+					char **canon_hostname)
+{
+	struct addrinfo hints;
+	struct addrinfo *ai;
+	int gai;
+	static char addrbuf[HOST_NAME_MAX + 1];
+	struct sockaddr_in *sin_addr;
+
+	memset(&hints, 0, sizeof(hints));
+	hints.ai_flags = AI_CANONNAME;
+
+	gai = getaddrinfo(hostname, NULL, &hints, &ai);
+	if (gai)
+		return;
+
+	sin_addr = (void *)ai->ai_addr;
+	inet_ntop(AF_INET, &sin_addr->sin_addr, addrbuf, sizeof(addrbuf));
+	free(*ip_address);
+	*ip_address = xstrdup(addrbuf);
+
+	free(*canon_hostname);
+	*canon_hostname = xstrdup(ai->ai_canonname ?
+				  ai->ai_canonname : *ip_address);
+
+	freeaddrinfo(ai);
+}
+
 /*
  * Returns a connected socket() fd, or else die()s.
  */
@@ -111,6 +143,31 @@ static int git_tcp_connect_sock(char *host, int flags)
 
 #else /* NO_IPV6 */
 
+void git_locate_host(const char *hostname, char **ip_address,
+					char **canon_hostname)
+{
+	struct hostent *hent;
+	struct sockaddr_in sa;
+	char **ap;
+	static char addrbuf[HOST_NAME_MAX + 1];
+
+	hent = gethostbyname(hostname);
+
+	ap = hent->h_addr_list;
+	memset(&sa, 0, sizeof sa);
+	sa.sin_family = hent->h_addrtype;
+	sa.sin_port = htons(0);
+	memcpy(&sa.sin_addr, *ap, hent->h_length);
+
+	inet_ntop(hent->h_addrtype, &sa.sin_addr,
+		  addrbuf, sizeof(addrbuf));
+
+	free(*canon_hostname);
+	*canon_hostname = xstrdup(hent->h_name);
+	free(*ip_address);
+	*ip_address = xstrdup(addrbuf);
+}
+
 /*
  * Returns a connected socket() fd, or else die()s.
  */
diff --git a/tcp.h b/tcp.h
index 4de5f712..bed3cdca 100644
--- a/tcp.h
+++ b/tcp.h
@@ -1,6 +1,9 @@
 #ifndef TCP_H
 #define TCP_H
 
+extern void git_locate_host(const char *hostname,
+			char **ip_address, char **canon_hostname);
+
 extern int git_use_proxy(const char *host);
 extern void git_tcp_connect(int fd[2], char *host, int flags);
 extern struct child_process *git_proxy_connect(int fd[2], char *host);
-- 
1.7.9.2

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

[Jonathan Nieder]

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(-)
 create mode 100644 dns-ipv4.c
 create mode 100644 dns-ipv4.h
 create mode 100644 dns-ipv6.c
 create mode 100644 dns-ipv6.h

diff --git a/Makefile b/Makefile
index 927a079a..8b603895 100644
--- a/Makefile
+++ b/Makefile
@@ -1612,6 +1612,11 @@ ifdef NO_TRUSTABLE_FILEMODE
 endif
 ifdef NO_IPV6
 	BASIC_CFLAGS += -DNO_IPV6
+	LIB_OBJS += dns-ipv4.o
+	LIB_H += dns-ipv4.h
+else
+	LIB_OBJS += dns-ipv6.o
+	LIB_H += dns-ipv6.h
 endif
 ifdef NO_UINTMAX_T
 	BASIC_CFLAGS += -Duintmax_t=uint32_t
diff --git a/dns-ipv4.c b/dns-ipv4.c
new file mode 100644
index 00000000..911a8569
--- /dev/null
+++ b/dns-ipv4.c
@@ -0,0 +1,33 @@
+#include "cache.h"
+#include "dns-ipv4.h"
+
+int dns_resolve(const char *host, const char *port, int flags,
+		resolver_result *res)
+{
+	char *ep;
+	struct hostent *he;
+	unsigned int nport;
+
+	he = gethostbyname(host);
+	if (!he && (flags & RESOLVE_FAIL_QUIETLY))
+		return -1;
+	if (!he)
+		die("Unable to look up %s (%s)", host, hstrerror(h_errno));
+
+	if (!port) {
+		nport = 0;
+		goto done;
+	}
+	nport = strtoul(port, &ep, 10);
+	if ( ep == port || *ep ) {
+		/* Not numeric */
+		struct servent *se = getservbyname(port,"tcp");
+		if ( !se )
+			die("Unknown port %s", port);
+		nport = se->s_port;
+	}
+done:
+	res->he = he;
+	res->port = nport;
+	return 0;
+}
diff --git a/dns-ipv4.h b/dns-ipv4.h
new file mode 100644
index 00000000..6803bcba
--- /dev/null
+++ b/dns-ipv4.h
@@ -0,0 +1,69 @@
+#ifndef DNS_IPV4_H
+#define DNS_IPV4_H
+
+#ifndef HOST_NAME_MAX
+#define HOST_NAME_MAX 256
+#endif
+
+struct ipv4_address {
+	char **ap;
+	struct sockaddr_in sa;
+};
+
+struct ipv4_addrinfo {
+	struct hostent *he;
+	unsigned int port;
+};
+
+typedef struct ipv4_addrinfo resolver_result;
+typedef struct ipv4_address resolved_address;
+
+enum {
+	RESOLVE_CANONNAME = 1,
+	RESOLVE_FAIL_QUIETLY = 2
+};
+extern int dns_resolve(const char *host, const char *port, int flags,
+			resolver_result *res);
+
+static inline const char *dns_name(const resolved_address *addr)
+{
+	return inet_ntoa(*(struct in_addr *)&addr->sa.sin_addr);
+}
+
+static inline char *dns_ip_address(const resolved_address *addr,
+					const resolver_result *ai)
+{
+	char addrbuf[HOST_NAME_MAX + 1];
+	inet_ntop(ai->he->h_addrtype, &addr->sa.sin_addr,
+		  addrbuf, sizeof(addrbuf));
+	return xstrdup(addrbuf);
+}
+
+static inline int dns_fill_sockaddr_(char *ap,
+		const struct ipv4_addrinfo *ai, struct sockaddr_in *sa)
+{
+	if (!ap)	/* done. */
+		return -1;
+
+	memset(sa, 0, sizeof(*sa));
+	sa->sin_family = ai->he->h_addrtype;
+	sa->sin_port = htons(ai->port);
+	memcpy(&sa->sin_addr, ap, ai->he->h_length);
+	return 0;
+}
+
+#define for_each_address(addr, ai) \
+	for ((addr).ap = (ai).he->h_addr_list; \
+	     !dns_fill_sockaddr_(*(addr).ap, &(ai), &(addr).sa); \
+	     (addr).ap++)
+
+#define dns_family(addr, ai) ((ai).he->h_addrtype)
+#define dns_socktype(addr, ai) SOCK_STREAM
+#define dns_protocol(addr, ai) 0
+#define dns_addr(addr, ai) ((struct sockaddr *) &(addr).sa)
+#define dns_addrlen(addr, ai) sizeof((addr).sa)
+#define dns_canonname(addr, ai) ((ai).he->h_name)
+
+#define dns_free(ai) do { /* nothing */ } while (0)
+
+#endif
diff --git a/dns-ipv6.c b/dns-ipv6.c
new file mode 100644
index 00000000..ca59ff91
--- /dev/null
+++ b/dns-ipv6.c
@@ -0,0 +1,49 @@
+#include "cache.h"
+#include "dns-ipv6.h"
+
+#ifndef HOST_NAME_MAX
+#define HOST_NAME_MAX 256
+#endif
+
+const char *dns_name(const resolved_address *i)
+{
+	const struct addrinfo *ai = *i;
+	static char addr[NI_MAXHOST];
+	if (getnameinfo(ai->ai_addr, ai->ai_addrlen, addr, sizeof(addr), NULL, 0,
+			NI_NUMERICHOST) != 0)
+		strcpy(addr, "(unknown)");
+
+	return addr;
+}
+
+char *dns_ip_address(const resolved_address *i, const resolver_result *ai0)
+{
+	const struct addrinfo *ai = *i;
+	char addrbuf[HOST_NAME_MAX + 1];
+	struct sockaddr_in *sin_addr;
+
+	sin_addr = (void *)ai->ai_addr;
+	inet_ntop(AF_INET, &sin_addr->sin_addr, addrbuf, sizeof(addrbuf));
+	return xstrdup(addrbuf);
+}
+
+int dns_resolve(const char *host, const char *port, int flags,
+		resolver_result *res)
+{
+	struct addrinfo hints;
+	int gai;
+
+	memset(&hints, 0, sizeof(hints));
+	if (flags & RESOLVE_CANONNAME)
+		hints.ai_flags = AI_CANONNAME;
+	hints.ai_socktype = SOCK_STREAM;
+	hints.ai_protocol = IPPROTO_TCP;
+
+	gai = getaddrinfo(host, port, &hints, res);
+	if (gai && (flags & RESOLVE_FAIL_QUIETLY))
+		return -1;
+	if (gai)
+		die("Unable to look up %s (port %s) (%s)", host, port, gai_strerror(gai));
+
+	return 0;
+}
diff --git a/dns-ipv6.h b/dns-ipv6.h
new file mode 100644
index 00000000..16bf84b5
--- /dev/null
+++ b/dns-ipv6.h
@@ -0,0 +1,31 @@
+#ifndef DNS_IPV6_H
+#define DNS_IPV6_H
+
+typedef struct addrinfo *resolver_result;
+typedef const struct addrinfo *resolved_address;
+
+enum {
+	RESOLVE_CANONNAME = 1,
+	RESOLVE_FAIL_QUIETLY = 2
+};
+extern int dns_resolve(const char *host, const char *port, int flags,
+			resolver_result *res);
+/* result is in static buffer */
+extern const char *dns_name(const resolved_address *i);
+/* result is in malloc'ed buffer */
+extern char *dns_ip_address(const resolved_address *i,
+				const resolver_result *ai);
+
+#define for_each_address(i, ai) \
+	for (i = ai; i; i = (i)->ai_next)
+
+#define dns_family(i, ai) ((i)->ai_family)
+#define dns_socktype(i, ai) ((i)->ai_socktype)
+#define dns_protocol(i, ai) ((i)->ai_protocol)
+#define dns_addr(i, ai) ((i)->ai_addr)
+#define dns_addrlen(i, ai) ((i)->ai_addrlen)
+#define dns_canonname(i, ai) ((i)->ai_canonname)
+
+#define dns_free(ai) freeaddrinfo(ai)
+
+#endif
diff --git a/tcp.c b/tcp.c
index 9263e0d2..4239daf3 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1,8 +1,10 @@
 #include "cache.h"
 #include "run-command.h"
 
-#ifndef HOST_NAME_MAX
-#define HOST_NAME_MAX 256
+#ifndef NO_IPV6
+#include "dns-ipv6.h"
+#else
+#include "dns-ipv4.h"
 #endif
 
 #define STR_(s)	# s
@@ -39,44 +41,27 @@ static void enable_keepalive(int sockfd)
 			strerror(errno));
 }
 
-#ifndef NO_IPV6
-
-static const char *ai_name(const struct addrinfo *ai)
-{
-	static char addr[NI_MAXHOST];
-	if (getnameinfo(ai->ai_addr, ai->ai_addrlen, addr, sizeof(addr), NULL, 0,
-			NI_NUMERICHOST) != 0)
-		strcpy(addr, "(unknown)");
-
-	return addr;
-}
-
 void git_locate_host(const char *hostname, char **ip_address,
 					char **canon_hostname)
 {
-	struct addrinfo hints;
-	struct addrinfo *ai;
-	int gai;
-	static char addrbuf[HOST_NAME_MAX + 1];
-	struct sockaddr_in *sin_addr;
+	resolver_result ai;
+	resolved_address i;
 
-	memset(&hints, 0, sizeof(hints));
-	hints.ai_flags = AI_CANONNAME;
-
-	gai = getaddrinfo(hostname, NULL, &hints, &ai);
-	if (gai)
+	if (dns_resolve(hostname, NULL,
+			RESOLVE_CANONNAME | RESOLVE_FAIL_QUIETLY, &ai))
 		return;
 
-	sin_addr = (void *)ai->ai_addr;
-	inet_ntop(AF_INET, &sin_addr->sin_addr, addrbuf, sizeof(addrbuf));
-	free(*ip_address);
-	*ip_address = xstrdup(addrbuf);
+	for_each_address(i, ai) {
+		free(*ip_address);
+		*ip_address = dns_ip_address(&i, &ai);
 
-	free(*canon_hostname);
-	*canon_hostname = xstrdup(ai->ai_canonname ?
-				  ai->ai_canonname : *ip_address);
+		free(*canon_hostname);
+		*canon_hostname = xstrdup(dns_canonname(i, ai) ?
+					dns_canonname(i, ai) : *ip_address);
+		break;
+	}
 
-	freeaddrinfo(ai);
+	dns_free(ai);
 }
 
 /*
@@ -87,46 +72,42 @@ static int git_tcp_connect_sock(char *host, int flags)
 	struct strbuf error_message = STRBUF_INIT;
 	int sockfd = -1;
 	const char *port = STR(DEFAULT_GIT_PORT);
-	struct addrinfo hints, *ai0, *ai;
-	int gai;
-	int cnt = 0;
+	resolver_result ai;
+	resolved_address i;
+	int cnt = -1;
 
 	get_host_and_port(&host, &port);
 	if (!*port)
 		port = "<none>";
 
-	memset(&hints, 0, sizeof(hints));
-	hints.ai_socktype = SOCK_STREAM;
-	hints.ai_protocol = IPPROTO_TCP;
-
 	if (flags & CONNECT_VERBOSE)
 		fprintf(stderr, "Looking up %s ... ", host);
 
-	gai = getaddrinfo(host, port, &hints, &ai);
-	if (gai)
-		die("Unable to look up %s (port %s) (%s)", host, port, gai_strerror(gai));
+	if (dns_resolve(host, port, 0, &ai))
+		die("BUG: dns_resolve returned error?");
 
 	if (flags & CONNECT_VERBOSE)
 		fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port);
 
-	for (ai0 = ai; ai; ai = ai->ai_next, cnt++) {
-		sockfd = socket(ai->ai_family,
-				ai->ai_socktype, ai->ai_protocol);
-		if ((sockfd < 0) ||
-		    (connect(sockfd, ai->ai_addr, ai->ai_addrlen) < 0)) {
+	for_each_address(i, ai) {
+		cnt++;
+		sockfd = socket(dns_family(i, ai),
+				dns_socktype(i, ai), dns_protocol(i, ai));
+		if (sockfd < 0 ||
+		    connect(sockfd, dns_addr(i, ai), dns_addrlen(i, ai)) < 0) {
 			strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n",
-				    host, cnt, ai_name(ai), strerror(errno));
+				    host, cnt, dns_name(&i), strerror(errno));
 			if (0 <= sockfd)
 				close(sockfd);
 			sockfd = -1;
 			continue;
 		}
 		if (flags & CONNECT_VERBOSE)
-			fprintf(stderr, "%s ", ai_name(ai));
+			fprintf(stderr, "%s ", dns_name(&i));
 		break;
 	}
 
-	freeaddrinfo(ai0);
+	dns_free(ai);
 
 	if (sockfd < 0)
 		die("unable to connect to %s:\n%s", host, error_message.buf);
@@ -141,107 +122,6 @@ static int git_tcp_connect_sock(char *host, int flags)
 	return sockfd;
 }
 
-#else /* NO_IPV6 */
-
-void git_locate_host(const char *hostname, char **ip_address,
-					char **canon_hostname)
-{
-	struct hostent *hent;
-	struct sockaddr_in sa;
-	char **ap;
-	static char addrbuf[HOST_NAME_MAX + 1];
-
-	hent = gethostbyname(hostname);
-
-	ap = hent->h_addr_list;
-	memset(&sa, 0, sizeof sa);
-	sa.sin_family = hent->h_addrtype;
-	sa.sin_port = htons(0);
-	memcpy(&sa.sin_addr, *ap, hent->h_length);
-
-	inet_ntop(hent->h_addrtype, &sa.sin_addr,
-		  addrbuf, sizeof(addrbuf));
-
-	free(*canon_hostname);
-	*canon_hostname = xstrdup(hent->h_name);
-	free(*ip_address);
-	*ip_address = xstrdup(addrbuf);
-}
-
-/*
- * Returns a connected socket() fd, or else die()s.
- */
-static int git_tcp_connect_sock(char *host, int flags)
-{
-	struct strbuf error_message = STRBUF_INIT;
-	int sockfd = -1;
-	const char *port = STR(DEFAULT_GIT_PORT);
-	char *ep;
-	struct hostent *he;
-	struct sockaddr_in sa;
-	char **ap;
-	unsigned int nport;
-	int cnt;
-
-	get_host_and_port(&host, &port);
-
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "Looking up %s ... ", host);
-
-	he = gethostbyname(host);
-	if (!he)
-		die("Unable to look up %s (%s)", host, hstrerror(h_errno));
-	nport = strtoul(port, &ep, 10);
-	if ( ep == port || *ep ) {
-		/* Not numeric */
-		struct servent *se = getservbyname(port,"tcp");
-		if ( !se )
-			die("Unknown port %s", port);
-		nport = se->s_port;
-	}
-
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port);
-
-	for (cnt = 0, ap = he->h_addr_list; *ap; ap++, cnt++) {
-		memset(&sa, 0, sizeof sa);
-		sa.sin_family = he->h_addrtype;
-		sa.sin_port = htons(nport);
-		memcpy(&sa.sin_addr, *ap, he->h_length);
-
-		sockfd = socket(he->h_addrtype, SOCK_STREAM, 0);
-		if ((sockfd < 0) ||
-		    connect(sockfd, (struct sockaddr *)&sa, sizeof sa) < 0) {
-			strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n",
-				host,
-				cnt,
-				inet_ntoa(*(struct in_addr *)&sa.sin_addr),
-				strerror(errno));
-			if (0 <= sockfd)
-				close(sockfd);
-			sockfd = -1;
-			continue;
-		}
-		if (flags & CONNECT_VERBOSE)
-			fprintf(stderr, "%s ",
-				inet_ntoa(*(struct in_addr *)&sa.sin_addr));
-		break;
-	}
-
-	if (sockfd < 0)
-		die("unable to connect to %s:\n%s", host, error_message.buf);
-
-	enable_keepalive(sockfd);
-
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "done.\n");
-
-	return sockfd;
-}
-
-#endif /* NO_IPV6 */
-
-
 void git_tcp_connect(int fd[2], char *host, int flags)
 {
 	int sockfd = git_tcp_connect_sock(host, flags);
-- 
1.7.9.2

[PATCH 5/5] daemon: check for errors retrieving IP address

[Jonathan Nieder]

Date: Fri, 17 Jun 2011 05:26:05 -0500

To retrieve a canonical IP address for possible use in a
--interpolated-path=%IP string, the git daemon calls inet_ntop to fill
a buffer of size HOST_NAME_MAX+1, ignoring errors, and copies the
result.  If the address has length > HOST_NAME_MAX (which was as low
as 8 in ancient times) then inet_ntop could error out, leading the git
daemon to copy uninitialized data.

This probably never happens in practice because modern systems allow
such long hostnames as 64 chars (POSIX requires at least 255) or have
no compile-time maximum at all and let git fall back to a buffer size
of 256.  But it is more comforting not to rely on that.  So:

 - in "ipv4" code, which uses inet_ntop, use a buffer size of 64 (long
   enough to hold an ipv6 address if needed).

 - in ipv6 code, use the "getnameinfo" function that is already being
   used to convert addresses to text in error messages.  It uses a
   buffer of length NI_MAXHOST.

 - check for errors and make dns_ip_address() and git_locate_host()
   return NULL when they occur.  strbuf_expand_dict_cb treats NULL as
   an empty substitution string.

As a nice side effect, the fallback definition of HOST_NAME_MAX for
platforms that don't define it is no longer needed.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
This patch allows getnameinfo and inet_ntop to use a larger buffer or
fail.  (I was mostly nervous because HOST_NAME_MAX is not guaranteed
to be large enough to hold an IP address.)

That's the end of the series.  Thanks for reading.

 dns-ipv4.h |   11 +++++------
 dns-ipv6.c |   16 ++++++++--------
 tcp.c      |    6 ++++--
 3 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/dns-ipv4.h b/dns-ipv4.h
index 6803bcba..c5f1778f 100644
--- a/dns-ipv4.h
+++ b/dns-ipv4.h
@@ -1,9 +1,7 @@
 #ifndef DNS_IPV4_H
 #define DNS_IPV4_H
 
-#ifndef HOST_NAME_MAX
-#define HOST_NAME_MAX 256
-#endif
+#define ADDRBUFLEN 64	/* 46 for an ipv6 address, plus a little extra */
 
 struct ipv4_address {
 	char **ap;
@@ -33,9 +31,10 @@ static inline const char *dns_name(const resolved_address *addr)
 static inline char *dns_ip_address(const resolved_address *addr,
 					const resolver_result *ai)
 {
-	char addrbuf[HOST_NAME_MAX + 1];
-	inet_ntop(ai->he->h_addrtype, &addr->sa.sin_addr,
-		  addrbuf, sizeof(addrbuf));
+	char addrbuf[ADDRBUFLEN];
+	if (!inet_ntop(ai->he->h_addrtype, &addr->sa.sin_addr,
+		  addrbuf, sizeof(addrbuf)))
+		return NULL;
 	return xstrdup(addrbuf);
 }
 
diff --git a/dns-ipv6.c b/dns-ipv6.c
index ca59ff91..f2325681 100644
--- a/dns-ipv6.c
+++ b/dns-ipv6.c
@@ -1,8 +1,9 @@
 #include "cache.h"
 #include "dns-ipv6.h"
 
-#ifndef HOST_NAME_MAX
-#define HOST_NAME_MAX 256
+/* from RFC 2553 */
+#ifndef NI_MAXHOST
+#define NI_MAXHOST 1025
 #endif
 
 const char *dns_name(const resolved_address *i)
@@ -19,12 +20,11 @@ const char *dns_name(const resolved_address *i)
 char *dns_ip_address(const resolved_address *i, const resolver_result *ai0)
 {
 	const struct addrinfo *ai = *i;
-	char addrbuf[HOST_NAME_MAX + 1];
-	struct sockaddr_in *sin_addr;
-
-	sin_addr = (void *)ai->ai_addr;
-	inet_ntop(AF_INET, &sin_addr->sin_addr, addrbuf, sizeof(addrbuf));
-	return xstrdup(addrbuf);
+	char addr[NI_MAXHOST];
+	if (getnameinfo(ai->ai_addr, ai->ai_addrlen, addr, sizeof(addr),
+			NULL, 0, NI_NUMERICHOST))
+		return NULL;
+	return xstrdup(addr);
 }
 
 int dns_resolve(const char *host, const char *port, int flags,
diff --git a/tcp.c b/tcp.c
index 4239daf3..83f0313a 100644
--- a/tcp.c
+++ b/tcp.c
@@ -56,8 +56,10 @@ void git_locate_host(const char *hostname, char **ip_address,
 		*ip_address = dns_ip_address(&i, &ai);
 
 		free(*canon_hostname);
-		*canon_hostname = xstrdup(dns_canonname(i, ai) ?
-					dns_canonname(i, ai) : *ip_address);
+		*canon_hostname =
+			dns_canonname(i, ai) ? xstrdup(dns_canonname(i, ai)) :
+			*ip_address ? xstrdup(*ip_address) :
+			NULL;
 		break;
 	}
 
-- 
1.7.9.2

[PATCH 6/5] tcp: make dns_resolve() return an error code

[Jonathan Nieder]

Date: Thu, 23 Jun 2011 05:23:57 -0500

getaddrinfo() and gethostbyname() use error codes to describe their
errors.  Pass that error code back to the caller when dns_resolve()
fails and the RESOLVE_FAIL_QUIETLY flag is set, so callers can save
the diagnosis and print it later:

	int saved_rerrno = dns_resolve(...);
	...
	if (saved_rerrno)
		die("resolver failed: %s", dns_strerror(saved_rerrno));

In the ipv4 codepath, we assume that h_errno is never 0 on error.
POSIX.1-2004 does not specify whether 0 is a valid value for h_errno,
but luckily common practice is for h_errno to be a strictly positive
integer (HOST_NOT_FOUND = 1, NO_DATA = 2, NO_RECOVERY = 3, or
TRY_AGAIN = 4).  If gethostbyname errors out with h_errno == 0 on some
platform, just let git die with a message indicating a BUG so the bad
assumption can be corrected.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
This makes saving up errors from a host resolution failure as in
v1.7.7-rc0~40^2 (connect: only log if all attempts failed, 2011-08-01)
possible.  By the way, this series is currently against "maint" for no
particular reason and presumably it would conflict with that patch. ;-)

I'll be happy to rebase against "master" some time soon.

 dns-ipv4.c |    7 +++++--
 dns-ipv4.h |    5 +++++
 dns-ipv6.c |    2 +-
 dns-ipv6.h |    1 +
 4 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/dns-ipv4.c b/dns-ipv4.c
index 911a8569..8820c9e8 100644
--- a/dns-ipv4.c
+++ b/dns-ipv4.c
@@ -9,8 +9,11 @@ int dns_resolve(const char *host, const char *port, int flags,
 	unsigned int nport;
 
 	he = gethostbyname(host);
-	if (!he && (flags & RESOLVE_FAIL_QUIETLY))
-		return -1;
+	if (!he && (flags & RESOLVE_FAIL_QUIETLY)) {
+		if (!h_errno)
+			die("BUG: gethostbyname failed but h_errno == 0");
+		return h_errno;
+	}
 	if (!he)
 		die("Unable to look up %s (%s)", host, hstrerror(h_errno));
 
diff --git a/dns-ipv4.h b/dns-ipv4.h
index c5f1778f..6f70d639 100644
--- a/dns-ipv4.h
+++ b/dns-ipv4.h
@@ -18,6 +18,10 @@ typedef struct ipv4_address resolved_address;
 
 enum {
 	RESOLVE_CANONNAME = 1,
+	/*
+	 * Quietly return an error code instead of exiting on error.
+	 * Callers can use dns_strerror() to get an error string.
+	 */
 	RESOLVE_FAIL_QUIETLY = 2
 };
 extern int dns_resolve(const char *host, const char *port, int flags,
@@ -63,6 +67,7 @@ static inline int dns_fill_sockaddr_(char *ap,
 #define dns_addrlen(addr, ai) sizeof((addr).sa)
 #define dns_canonname(addr, ai) ((ai).he->h_name)
 
+#define dns_strerror(n) hstrerror(n)
 #define dns_free(ai) do { /* nothing */ } while (0)
 
 #endif
diff --git a/dns-ipv6.c b/dns-ipv6.c
index f2325681..0b0e0602 100644
--- a/dns-ipv6.c
+++ b/dns-ipv6.c
@@ -41,7 +41,7 @@ int dns_resolve(const char *host, const char *port, int flags,
 
 	gai = getaddrinfo(host, port, &hints, res);
 	if (gai && (flags & RESOLVE_FAIL_QUIETLY))
-		return -1;
+		return gai;
 	if (gai)
 		die("Unable to look up %s (port %s) (%s)", host, port, gai_strerror(gai));
 
diff --git a/dns-ipv6.h b/dns-ipv6.h
index 16bf84b5..4211c9e2 100644
--- a/dns-ipv6.h
+++ b/dns-ipv6.h
@@ -26,6 +26,7 @@ extern char *dns_ip_address(const resolved_address *i,
 #define dns_addrlen(i, ai) ((i)->ai_addrlen)
 #define dns_canonname(i, ai) ((i)->ai_canonname)
 
+#define dns_strerror(gai) gai_strerror(gai)
 #define dns_free(ai) freeaddrinfo(ai)
 
 #endif
-- 
1.7.9.2

[PATCH 7/5] transport: optionally honor DNS SRV records

[Jonathan Nieder]

Date: Mon, 6 Jun 2011 04:46:20 -0500

SRV records are an extra layer of indirection on top of A/AAAA records
mapping from domain names and service types to the hostnames of
machines that offer that service.  That has a couple of nice effects:

 - a single domain can use different hosts for different services
 - a single domain can use multiple hosts for one service

Teach git to perform a SRV lookup whenever resolving a git:// URL.
This means:

 - if your git server was previously the same machine as your wesnoth
   server, you can move it to a separate machine without forcing
   everyone to update their links to the old URLs

 - if you have a primary git server and a backup machine that should
   be used when the primary server goes down, the client can
   automatically take care of it

 - if you have multiple git servers and would like to spread load
   between them, the client can automatically take care of it

That is, SRV records let us ask the client to carry out various tasks
that would require a proxy on the server side with traditional DNS.
The client performs a SRV query to _git._tcp.<domain name> to receive
its instructions.  RFC 2782 has details.

Ideally reaping these benefits would just involve passing a special
flag to getaddrinfo().  Since we don't live in such a world, this
patch uses the BIND 8 API provided by libresolv to parse the response
for ourselves.

RFC 2782 requires some non-determinism in the order of hosts
contacted; this patch uses drand48() for that.  To avoid causing
trouble for platforms that lack the libbind ns_* functions or
drand48(), the SRV support is only provided when requested by setting
the USE_SRV_RR compile-time option.

git servers must ensure that they can also be reached by a plain
A/AAAA lookup to support git clients without SRV support, for example
by proxying connections to an appropriate server:

	# in inetd.conf
	git stream tcp nowait.400 nobody /usr/sbin/tcpd \
		/bin/nc -q0 gitserver.example.com git

Regression: this uglifies error messages for connection errors a
little.  It would probably be better to leave out the connection count
when we are not trying more than one server.

Based on a patch by Julien Cristau <jcristau@debian.org>.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
As I mentioned at the gittogether, I think this would be a valuable
feature in git.  You can test it out (though this is only the simple
case, no load balancing or failover) by doing

	git clone git://git.debian.org/~jrnieder-guest/git.git

and watching what happens with wireshark.

The patch needs documentation.  Maybe a howto and an addendum to the
protocol docs would do.  Anyway, I hope it's at least entertaining in
the current state.

 Makefile |   10 ++
 srv.c    |  321 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 srv.h    |   15 +++
 tcp.c    |  100 ++++++++++++++------
 4 files changed, 418 insertions(+), 28 deletions(-)
 create mode 100644 srv.c
 create mode 100644 srv.h

diff --git a/Makefile b/Makefile
index 8b603895..56198f11 100644
--- a/Makefile
+++ b/Makefile
@@ -148,6 +148,10 @@ all::
 # Notably on Solaris hstrerror resides in libresolv and on Solaris 7
 # inet_ntop and inet_pton additionally reside there.
 #
+# Define USE_SRV_RR if you want git to pay attention to SRV resource records
+# when looking up servers to contact over git protocol.  This implies
+# NEEDS_RESOLV.
+#
 # Define NO_MMAP if you want to avoid mmap.
 #
 # Define NO_PTHREADS if you do not have or do not want to use Pthreads.
@@ -1490,6 +1494,11 @@ endif
 ifdef NEEDS_NSL
 	EXTLIBS += -lnsl
 endif
+ifdef USE_SRV_RR
+	BASIC_CFLAGS += -DUSE_SRV_RR
+	LIB_OBJS += srv.o
+	NEEDS_RESOLV = YesPlease
+endif
 ifdef NEEDS_RESOLV
 	EXTLIBS += -lresolv
 endif
@@ -2155,6 +2164,7 @@ builtin/commit.o builtin/revert.o wt-status.o: wt-status.h
 builtin/tar-tree.o archive-tar.o: tar.h
 connect.o transport.o url.o http-backend.o: url.h
 connect.o daemon.o tcp.o: tcp.h
+tcp.o srv.o: srv.h
 http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h
 http.o http-walker.o http-push.o http-fetch.o remote-curl.o: http.h url.h
 
diff --git a/srv.c b/srv.c
new file mode 100644
index 00000000..2716206e
--- /dev/null
+++ b/srv.c
@@ -0,0 +1,321 @@
+#include "git-compat-util.h"
+#include "strbuf.h"
+#include "srv.h"
+
+#include <arpa/nameser.h>
+#include <resolv.h>
+
+struct parsed_srv_rr {
+	int priority;
+	int weight;
+	int port;
+	char *target;
+};
+
+static void srv_swap(struct parsed_srv_rr *p1, struct parsed_srv_rr *p2)
+{
+	char *a, *b;
+	int size = sizeof(struct parsed_srv_rr);
+
+	for (a = (char *) p1, b = (char *) p2; size; size--) {
+		char t = *a;
+		*a++ = *b;
+		*b++ = t;
+	}
+}
+
+static int priority_compare(const void *p1, const void *p2)
+{
+	const struct parsed_srv_rr *a = p1, *b = p2;
+
+	/* can't overflow because priorities are 16 bits wide */
+	return b->priority - a->priority;
+}
+
+static int get_qname_for_srv(struct strbuf *sb, const char *host)
+{
+	const char prefix[] = "_git._tcp.";
+	size_t hostlen;
+
+	hostlen = strlen(host);
+	if (unsigned_add_overflows(strlen(prefix) + 1, hostlen))
+		return error("absurdly long hostname");
+
+	strbuf_reset(sb);
+	strbuf_grow(sb, strlen(prefix) + hostlen);
+	strbuf_add(sb, prefix, strlen(prefix));
+	strbuf_add(sb, host, hostlen);
+	return 0;
+}
+
+static int srv_parse_rr(const ns_msg *msg,
+			const ns_rr *rr, struct parsed_srv_rr *res)
+{
+	const unsigned char *p;
+	char buf[1024];
+
+	if (ns_rr_rdlen(*rr) < 2+2+2 /* priority, weight, port */)
+		return error("SRV RR is too short");
+
+	p = ns_rr_rdata(*rr);
+	res->priority = *p++ << CHAR_BIT;
+	res->priority += *p++;
+
+	res->weight = *p++ << CHAR_BIT;
+	res->weight += *p++;
+
+	res->port = *p++ << CHAR_BIT;
+	res->port += *p++;
+
+	/*
+	 * RFC2782 doesn't allow compressed target domain names but we
+	 * might as well accept them anyway.
+	 */
+	if (dn_expand(ns_msg_base(*msg), ns_msg_end(*msg), p,
+			buf, sizeof(buf)) < 0)
+		return error("cannot expand target domain name in SRV RR");
+
+	res->target = xstrdup(buf);
+	return 0;
+}
+
+static int srv_parse(ns_msg *msg, struct parsed_srv_rr **res)
+{
+	struct parsed_srv_rr *rrs = NULL;
+	int nr_parsed = 0;
+	int cnames = 0;
+	int i, n;
+
+	n = ns_msg_count(*msg, ns_s_an);
+
+	/* skip CNAME records */
+	for (i = 0; i < n; i++) {
+		ns_rr rr;
+		if (ns_parserr(msg, ns_s_an, i, &rr)) {
+			error("cannot parse DNS RR: %s", strerror(errno));
+			goto fail;
+		}
+		if (ns_rr_type(rr) != ns_t_cname)
+			break;
+	}
+	cnames = i;
+	n -= cnames;
+
+	rrs = xmalloc(n * sizeof(*rrs));
+	for (i = 0; i < n; i++) {
+		ns_rr rr;
+
+		if (ns_parserr(msg, ns_s_an, cnames + i, &rr)) {
+			error("cannot parse DNS RR: %s", strerror(errno));
+			goto fail;
+		}
+		if (ns_rr_type(rr) != ns_t_srv) {
+			error("expected SRV RR, found RR type %d",
+						(int) ns_rr_type(rr));
+			goto fail;
+		}
+		if (srv_parse_rr(msg, &rr, rrs + i))
+			/* srv_parse_rr writes a message */
+			goto fail;
+		nr_parsed++;
+	}
+
+	*res = rrs;
+	return n;
+fail:
+	for (i = 0; i < nr_parsed; i++)
+		free(rrs[i].target);
+	free(rrs);
+	return -1;
+}
+
+static int weighted_item(struct parsed_srv_rr *rrs, int n, unsigned int pos)
+{
+	int i;
+
+	for (i = 0; i < n; i++) {
+		unsigned int wt = rrs[i].weight;
+
+		if (pos <= wt)
+			break;
+		pos -= wt;
+	}
+	return i;
+}
+
+static void shuffle_one(struct parsed_srv_rr *rrs, int n,
+				int i, int *j, unsigned int *wt_remaining)
+{
+	unsigned int pos;
+	int k;
+
+	pos = (unsigned int) ((*wt_remaining + 1) * drand48());
+
+	if (!pos) {
+		*wt_remaining -= rrs[i].weight;
+		return;
+	}
+
+	/* Which item will take the place of rrs[i]? */
+	if (*j < i)
+		*j = i;
+	k = *j + weighted_item(rrs + *j, n - *j, pos);
+
+	assert(k < n);
+	*wt_remaining -= rrs[k].weight;
+
+	if (k == i)
+		return;
+
+	srv_swap(rrs + i, rrs + k);
+
+	/*
+	 * If rrs[i] had weight zero, move it to stay in the clump
+	 * of weight-zero records.  rrs[k] cannot have had weight
+	 * zero because pos > 0.
+	 */
+	assert(*j <= k);
+	if (i < *j) {
+		srv_swap(rrs + k, rrs + *j);
+		(*j)++;
+	}
+}
+
+static void weighted_shuffle(struct parsed_srv_rr *rrs, int n)
+{
+	int i, j;
+	unsigned int total = 0;
+	static int seeded;
+
+	/*
+	 * Calculate total weight and move weight-zero
+	 * records to the beginning of the array.
+	 */
+	assert(n < (1 << 16));
+	for (i = j = 0; i < n; i++) {
+		unsigned int wt = rrs[i].weight;
+		assert(wt < (1 << 16));
+
+		if (!wt) {
+			srv_swap(rrs + i, rrs + j);
+			j++;
+		}
+
+		/*
+		 * In the worst case, n is 2^16 - 1 and
+		 * each weight is 2^16 - 1, making the total
+		 * a little less than 2^32.
+		 */
+		assert(!unsigned_add_overflows(total, wt + 1));
+		total += wt;
+	}
+	/* Usual case: all weights are zero. */
+	if (!total)
+		return;
+
+	if (!seeded) {
+		seeded = 1;
+		srand48(time(NULL));
+	}
+
+	for (i = 0; i < n; i++)
+		/*
+		 * Now the records starting at rrs[i] could be in any order,
+		 * except those of weight 0 are at the start of the list
+		 * (ending with rrs[j-1]).
+		 *
+		 * Pick an item from rrs[i]..rrs[n] at random, taking weights
+		 * into account, and reorder to make it rrs[i], preserving
+		 * that invariant.
+		 */
+		shuffle_one(rrs, n, i, &j, &total);
+}
+
+static void sort_rrs(struct parsed_srv_rr *rrs, int n)
+{
+	int i, j, prio;
+
+	qsort(rrs, n, sizeof(*rrs), priority_compare);
+
+	/*
+	 * Within each priority level, order servers randomly,
+	 * respecting weight.
+	 */
+	j = 0;
+	prio = rrs[j].priority;
+	for (i = 0; i < n; i++) {
+		if (rrs[i].priority == prio)
+			continue;
+
+		weighted_shuffle(rrs + j, i - j);
+		j = i;
+		prio = rrs[j].priority;
+	}
+	weighted_shuffle(rrs + j, n - j);
+}
+
+/* Reference: RFC2782. */
+int get_srv(const char *host, struct host **hosts)
+{
+	struct strbuf sb = STRBUF_INIT;
+	unsigned char buf[1024];
+	ns_msg msg;
+	int len, n, i, ret;
+	struct parsed_srv_rr *rrs = NULL;
+	struct host *reply = NULL;
+
+	/* if no SRV record is found, fall back to plain address lookup */
+	ret = 0;
+
+	/* _git._tcp.<host> */
+	if (get_qname_for_srv(&sb, host))
+		goto out;
+	len = res_query(sb.buf, ns_c_in, ns_t_srv, buf, sizeof(buf));
+	if (len < 0)
+		goto out;
+
+	/* If a SRV RR cannot be parsed, give up. */
+	ret = -1;
+
+	if (ns_initparse(buf, len, &msg)) {
+		error("cannot initialize DNS parser: %s", strerror(errno));
+		goto out;
+	}
+	n = srv_parse(&msg, &rrs);
+	if (n < 0)
+		/* srv_parse writes a message */
+		goto out;
+	if (!n) {
+		ret = 0;
+		goto out;
+	}
+	assert(n < (1 << 16));
+
+	/* A single RR with target "." means "go away". */
+	if (n == 1 &&
+	    (!*rrs[0].target || !strcmp(rrs[0].target, ".")))
+		goto out2;
+
+	sort_rrs(rrs, n);
+
+	/* Success! */
+	ret = n;
+	reply = xmalloc(n * sizeof(*reply));
+	for (i = 0; i < n; i++) {
+		char buf[32];
+		snprintf(buf, sizeof(buf), "%d", rrs[i].port);
+
+		reply[i].hostname = rrs[i].target;
+		reply[i].port = xstrdup(buf);
+	}
+	*hosts = reply;
+	goto out;
+
+out2:
+	for (i = 0; i < n; i++)
+		free(rrs[i].target);
+out:
+	free(rrs);
+	strbuf_release(&sb);
+	return ret;
+}
diff --git a/srv.h b/srv.h
new file mode 100644
index 00000000..7cea4f4c
--- /dev/null
+++ b/srv.h
@@ -0,0 +1,15 @@
+#ifndef SRV_H
+#define SRV_H
+
+struct host {
+	char *hostname;
+	char *port;
+};
+
+#ifndef USE_SRV_RR
+#define get_srv(host, hosts) 0
+#else
+extern int get_srv(const char *host, struct host **hosts);
+#endif
+
+#endif
diff --git a/tcp.c b/tcp.c
index 83f0313a..c27a0d7f 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "run-command.h"
+#include "srv.h"
 
 #ifndef NO_IPV6
 #include "dns-ipv6.h"
@@ -72,46 +73,85 @@ void git_locate_host(const char *hostname, char **ip_address,
 static int git_tcp_connect_sock(char *host, int flags)
 {
 	struct strbuf error_message = STRBUF_INIT;
-	int sockfd = -1;
-	const char *port = STR(DEFAULT_GIT_PORT);
-	resolver_result ai;
-	resolved_address i;
-	int cnt = -1;
+	int sockfd = -1, gai = 0;
+	const char *port = NULL;
+	struct host *hosts = NULL;
+	int j, n = 0;
 
 	get_host_and_port(&host, &port);
+	if (!port) {
+		port = STR(DEFAULT_GIT_PORT);
+		n = get_srv(host, &hosts);
+	}
+	if (n < 0)
+		die("Unable to look up %s", host);
 	if (!*port)
 		port = "<none>";
+	if (!n) {
+		hosts = xmalloc(sizeof(*hosts));
+		hosts[0].hostname = xstrdup(host);
+		hosts[0].port = xstrdup(port);
+		n = 1;
+	}
 
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "Looking up %s ... ", host);
+	for (j = 0; j < n; j++) {
+		resolver_result ai;
+		resolved_address i;
+		int cnt;
 
-	if (dns_resolve(host, port, 0, &ai))
-		die("BUG: dns_resolve returned error?");
+		if (flags & CONNECT_VERBOSE)
+			fprintf(stderr, "Looking up %s ... ", hosts[j].hostname);
 
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port);
+		gai = dns_resolve(hosts[j].hostname,
+				hosts[j].port, RESOLVE_FAIL_QUIETLY, &ai);
+		if (gai) {
+			if (flags & CONNECT_VERBOSE)
+				fprintf(stderr, "failed.\n");
 
-	for_each_address(i, ai) {
-		cnt++;
-		sockfd = socket(dns_family(i, ai),
-				dns_socktype(i, ai), dns_protocol(i, ai));
-		if (sockfd < 0 ||
-		    connect(sockfd, dns_addr(i, ai), dns_addrlen(i, ai)) < 0) {
-			strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n",
-				    host, cnt, dns_name(&i), strerror(errno));
-			if (0 <= sockfd)
-				close(sockfd);
-			sockfd = -1;
+			if (n == 1 && !strcmp(host, hosts[j].hostname))
+				strbuf_addf(&error_message, "%s: %s\n",
+					host, dns_strerror(gai));
+			else
+				strbuf_addf(&error_message,
+					"%s[%d: %s:%s]: %s\n", host, j,
+					hosts[j].hostname, hosts[j].port,
+					dns_strerror(gai));
 			continue;
 		}
+
 		if (flags & CONNECT_VERBOSE)
-			fprintf(stderr, "%s ", dns_name(&i));
-		break;
+			fprintf(stderr, "done.\nConnecting to %s (port %s) ... ",
+					hosts[j].hostname, hosts[j].port);
+
+		cnt = -1;
+		for_each_address(i, ai) {
+			cnt++;
+			sockfd = socket(dns_family(i, ai),
+					dns_socktype(i, ai), dns_protocol(i, ai));
+			if (sockfd < 0 ||
+			    connect(sockfd, dns_addr(i, ai), dns_addrlen(i, ai)) < 0) {
+				strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n",
+						hosts[j].hostname,
+						cnt,
+						dns_name(&i),
+						strerror(errno));
+				if (0 <= sockfd)
+					close(sockfd);
+				sockfd = -1;
+				continue;
+			}
+			if (flags & CONNECT_VERBOSE)
+				fprintf(stderr, "%s ", dns_name(&i));
+			break;
+		}
+
+		dns_free(ai);
+
+		if (sockfd >= 0)
+			break;
 	}
 
-	dns_free(ai);
-
-	if (sockfd < 0)
+	if (gai || sockfd < 0)
 		die("unable to connect to %s:\n%s", host, error_message.buf);
 
 	enable_keepalive(sockfd);
@@ -119,8 +159,12 @@ static int git_tcp_connect_sock(char *host, int flags)
 	if (flags & CONNECT_VERBOSE)
 		fprintf(stderr, "done.\n");
 
+	for (j = 0; j < n; j++) {
+		free(hosts[j].hostname);
+		free(hosts[j].port);
+	}
+	free(hosts);
 	strbuf_release(&error_message);
-
 	return sockfd;
 }
 
-- 
1.7.9.2

[PATCH 8/5] srv: tolerate broken DNS replies

[Jonathan Nieder]

At a hotel with a very broken Wi-Fi setup, Richard found his copy
of git unable to cope:

	% git clone git://git.kitenet.net/mr
	Cloning into 'mr'...
	error: cannot initialize DNS parser: Message too long
	fatal: Unable to look up git.kitenet.net

Other programs gave some warnings but otherwise worked fine.  From a
packet capture, it seems that the response to a SRV query for
_git._tcp.git.kitenet.net in this setup was a single A resource record
pointing to the link-local address 169.254.1.1, followed by two
trailing bytes: c0 1a.  The trailing bytes cause the underlying parser
to fail.

It would not be good to silently tolerate this and similar kinds of
brokenness, but working around it would help people on affected
systems to recover.  Luckily RFC2782 gives us enough leeway to act as
we please for this particular kind of error, so give a warning and
fall back to an A/AAAA query (which should work).

Similarly, if we receive non-SRV RRs in response to a SRV query,
RFC2782 does not say to error out, so in the spirit of graceful
degradation let's warn and skip those records.

Reported-by: Richard Hartmann <richih.mailinglist@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Thanks for reading.  That's the end of the series.

Good night,
Jonathan

 srv.c |   40 ++++++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/srv.c b/srv.c
index 2716206e..829ef762 100644
--- a/srv.c
+++ b/srv.c
@@ -83,7 +83,6 @@ static int srv_parse(ns_msg *msg, struct parsed_srv_rr **res)
 {
 	struct parsed_srv_rr *rrs = NULL;
 	int nr_parsed = 0;
-	int cnames = 0;
 	int i, n;
 
 	n = ns_msg_count(*msg, ns_s_an);
@@ -98,30 +97,33 @@ static int srv_parse(ns_msg *msg, struct parsed_srv_rr **res)
 		if (ns_rr_type(rr) != ns_t_cname)
 			break;
 	}
-	cnames = i;
-	n -= cnames;
 
-	rrs = xmalloc(n * sizeof(*rrs));
-	for (i = 0; i < n; i++) {
+	rrs = xmalloc((n - i) * sizeof(*rrs));
+	for (; i < n; i++) {
 		ns_rr rr;
 
-		if (ns_parserr(msg, ns_s_an, cnames + i, &rr)) {
+		if (ns_parserr(msg, ns_s_an, i, &rr)) {
 			error("cannot parse DNS RR: %s", strerror(errno));
 			goto fail;
 		}
 		if (ns_rr_type(rr) != ns_t_srv) {
-			error("expected SRV RR, found RR type %d",
+			/*
+			 * Maybe the server is playing tricks and returned
+			 * an A record.  Let it pass and if we don't get
+			 * any SRV RRs, we can fall back to an A lookup.
+			 */
+			warning("expected SRV RR, found RR type %d",
 						(int) ns_rr_type(rr));
-			goto fail;
+			continue;
 		}
-		if (srv_parse_rr(msg, &rr, rrs + i))
+		if (srv_parse_rr(msg, &rr, rrs + nr_parsed))
 			/* srv_parse_rr writes a message */
 			goto fail;
 		nr_parsed++;
 	}
 
 	*res = rrs;
-	return n;
+	return nr_parsed;
 fail:
 	for (i = 0; i < nr_parsed; i++)
 		free(rrs[i].target);
@@ -274,13 +276,23 @@ int get_srv(const char *host, struct host **hosts)
 	if (len < 0)
 		goto out;
 
+	/*
+	 * If the reply to a SRV query is malformed, fall back to an
+	 * A query.
+	 *
+	 * The RFC2782 usage rules don't say anything about this, but
+	 * in practice, it seems that some firewalls or DNS servers
+	 * (think: captive portal) handle A queries sensibly and
+	 * provide malformed replies in response to SRV queries.
+	 */
+	if (ns_initparse(buf, len, &msg)) {
+		warning("cannot parse SRV response: %s", strerror(errno));
+		goto out;
+	}
+
 	/* If a SRV RR cannot be parsed, give up. */
 	ret = -1;
 
-	if (ns_initparse(buf, len, &msg)) {
-		error("cannot initialize DNS parser: %s", strerror(errno));
-		goto out;
-	}
 	n = srv_parse(&msg, &rrs);
 	if (n < 0)
 		/* srv_parse writes a message */
-- 
1.7.9.2

Re: [PATCH 1/5] transport: expose git_tcp_connect() and friends in new tcp.h

[Erik Faye-Lund]

On Thu, Mar 8, 2012 at 2:03 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Date: Mon, 6 Jun 2011 04:37:14 -0500
>
> Split off a new tcp.c with the functions git_tcp_connect() and
> git_proxy_connect() that resolve and connect to a host.
>
> Part of a series to teach git to respect DNS SRV records when making
> new connections.  This is a preliminary step to make the connection
> library easier to understand before changing it.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

Nice. Now that this has been libified, someone can probably get rid of
some code in imap-send.c (specifically in the imap_open_store
function) on top of your topic...

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

[Erik Faye-Lund]

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?

Re: [PATCH 7/5] transport: optionally honor DNS SRV records

[Erik Faye-Lund]

On Thu, Mar 8, 2012 at 2:21 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Date: Mon, 6 Jun 2011 04:46:20 -0500
>
> SRV records are an extra layer of indirection on top of A/AAAA records
> mapping from domain names and service types to the hostnames of
> machines that offer that service.  That has a couple of nice effects:
>
>  - a single domain can use different hosts for different services
>  - a single domain can use multiple hosts for one service
>
> Teach git to perform a SRV lookup whenever resolving a git:// URL.
> This means:
>
>  - if your git server was previously the same machine as your wesnoth
>   server, you can move it to a separate machine without forcing
>   everyone to update their links to the old URLs
>
>  - if you have a primary git server and a backup machine that should
>   be used when the primary server goes down, the client can
>   automatically take care of it
>
>  - if you have multiple git servers and would like to spread load
>   between them, the client can automatically take care of it
>
> That is, SRV records let us ask the client to carry out various tasks
> that would require a proxy on the server side with traditional DNS.
> The client performs a SRV query to _git._tcp.<domain name> to receive
> its instructions.  RFC 2782 has details.
>
> Ideally reaping these benefits would just involve passing a special
> flag to getaddrinfo().  Since we don't live in such a world, this
> patch uses the BIND 8 API provided by libresolv to parse the response
> for ourselves.
>
> RFC 2782 requires some non-determinism in the order of hosts
> contacted; this patch uses drand48() for that.  To avoid causing
> trouble for platforms that lack the libbind ns_* functions or
> drand48(), the SRV support is only provided when requested by setting
> the USE_SRV_RR compile-time option.
>
> git servers must ensure that they can also be reached by a plain
> A/AAAA lookup to support git clients without SRV support, for example
> by proxying connections to an appropriate server:
>
>        # in inetd.conf
>        git stream tcp nowait.400 nobody /usr/sbin/tcpd \
>                /bin/nc -q0 gitserver.example.com git
>
> Regression: this uglifies error messages for connection errors a
> little.  It would probably be better to leave out the connection count
> when we are not trying more than one server.
>
> Based on a patch by Julien Cristau <jcristau@debian.org>.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> As I mentioned at the gittogether, I think this would be a valuable
> feature in git.  You can test it out (though this is only the simple
> case, no load balancing or failover) by doing
>
>        git clone git://git.debian.org/~jrnieder-guest/git.git
>
> and watching what happens with wireshark.
>
> The patch needs documentation.  Maybe a howto and an addendum to the
> protocol docs would do.  Anyway, I hope it's at least entertaining in
> the current state.

It's an interesting feature, but I'm a little bit worried if this
promotes non-portable setups; won't these repos be unreachable (at
least without manually redirecting or also keeping a copy on the
advertised URL) on machines where libresolv is unavailable? I'm mainly
thinking about the "a single domain can use different hosts for
different services"-benefit you mentioned. Multiple hosts for one
service would probably be done by simply advertising one of the URLs,
and get some load-balancing from the clients that DOES have
libresolv...

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

[Jonathan Nieder]

Erik Faye-Lund wrote:

> 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.

Thanks.  It's true: I didn't convey the motivation well at all.

Before this patch, git_tcp_connect_sock() looks roughly like this:

	#ifndef NO_IPV6

	static int git_tcp_connect_sock(char *host, int flags)
	{
		get_host_and_port(&host, &port);
		memcpy(&hints, tcp_stream_hints, sizeof(hints));
		gai = getaddrinfo(host, port, &hints, &ai);
		for (ai0 = ai; ai; ai = ai->ai_next, cnt++) {
			... socket(), connect(), etc ...
		}
		freeaddrinfo(ai0);
		enable_keepalive(sockfd);
		return sockfd;
	}

	#else

	static int git_tcp_connect_sock(char *host, int flags)
	{
		get_host_and_port(&host, &port);
		he = gethostbyname(host);
		for (ap = he->h_addr_list; *ap; ap++, cnt++) {
			... socket(), connect(), etc ...
		}
		enable_keepalive(sockfd);
		return sockfd;
	}

	#endif

Any change to this procedure has to be made in both places, and in the
past, that has caused some minor bugs that were unnoticed for a while,
but no big deal --- it's not that complicated, so the code duplication
is tolerable.

After this patch and the next two, it looks like so:

	static int git_tcp_connect_sock(char *host, int flags)
	{
		get_host_and_port(&host, &port);
		gai = dns_resolve(host, port, 0, &ai);
		for_each_address(i, ai) {
			... socket(), connect(), etc ...
		}
		dns_free(ai0);
		enable_keepalive(sockfd);
		return sockfd;
	}

That is, it is almost identical to the getaddrinfo version.

So it is not about making the code shorter, but about not repeating
ourselves.

I did this for my sanity while implementing the SRV code: if my
changes work with the gethostbyname API, this way it is much more
likely this way that they will still work with getaddrinfo, too.  So
basically, the point is that I did not want to have to think about the
!NO_IPV6 codepath while writing patches (and others might not want to
have to think about the NO_IPV6 codepath at all --- the same
advantages apply for them, too).

	API dictionary in the !NO_IPV6 case
	(see dns-ipv6.h and dns-ipv6.c for details):

	resolver_result = struct addrinfo *
	resolved_address = const struct addrinfo *
	dns_resolve = getaddrinfo
	dns_name = getnameinfo, use in error messages
	dns_ip_address = getnameinfo, git-daemon uses for %IP substitution
	for_each_address = for (ai0 = ai; ai; ai = ai->ai_next)

	dns_family = ->ai_family
	dns_socktype = ->ai_socktype
	etc

	dns_strerror = gai_strerror
	dns_free = freeaddrinfo

The dns_name()/dns_ip_address() pair is a little silly.  The former
returns its result in a static buffer and the latter uses malloc.
Improvements welcome.

Of course, git_tcp_connect_sock() is not the only function that uses
host resolution facilities.  The benefit scales as more functions get
converted.

Hope that helps,
Jonathan

Re: [PATCH 7/5] transport: optionally honor DNS SRV records

[Jonathan Nieder]

Erik Faye-Lund wrote:

> It's an interesting feature, but I'm a little bit worried if this
> promotes non-portable setups; won't these repos be unreachable (at
> least without manually redirecting or also keeping a copy on the
> advertised URL) on machines where libresolv is unavailable? I'm mainly
> thinking about the "a single domain can use different hosts for
> different services"-benefit you mentioned.

Yes.  I think of redirecting with SRV records without a fallback as a
misconfiguration ("don't do that, then").  Note that it would not only
affect people without drand48 but people with broken DNS servers.

In the case of Alioth, what they do is let inetd listen on
git.debian.org = vasks.debian.org, using netcat to forward connections
to wagner.debian.org.  So clients with and without SRV support end up
finding their bits shuttled to the same place, with the only
difference being a little wasted load on vasks in the no-SRV case.

This means the Alioth admins don't get the benefit of not having to
set up a proxy, but they do get the benefit of clients taking on more
of the work and the service continuing to be available for some
clients if vasks goes down.

Maybe adding an envvar to disable the SRV handling would make it
easier for server admins to check the fallback.  Can we do more?

(By the way, what platforms don't support BIND and a random number
generator?)

Thanks,
Jonathan

Re: [PATCH 8/5] srv: tolerate broken DNS replies

[Richard Hartmann]

On Thu, Mar 8, 2012 at 14:23, Jonathan Nieder <jrnieder@gmail.com> wrote:

> Other programs gave some warnings but otherwise worked fine.

Just ftr, Chrome/Chromium ignores this quickly and loads pages while
Konqueror takes ages to get around this. Not related to git, but maybe
an interesting data point in the future. If not, disregard.


Thanks for hunting and killing this one.


Richard

Re: [PATCH 7/5] transport: optionally honor DNS SRV records

[Johannes Sixt]

Am 3/8/2012 22:35, schrieb Jonathan Nieder:
> (By the way, what platforms don't support BIND and a random number
> generator?)

MinGW, for example:

D:\Src\mingw-git>cat foo.c && gcc -c foo.c
#include <arpa/nameser.h>
#include <resolv.h>
#include <stdlib.h>

void (*x)(long int) = srand48;
foo.c:1:26: error: arpa/nameser.h: No such file or directory
foo.c:2:20: error: resolv.h: No such file or directory
foo.c:5: error: 'srand48' undeclared here (not in a function)

-- Hannes

Re: [PATCH 7/5] transport: optionally honor DNS SRV records

[Jonathan Nieder]

Hi,

Johannes Sixt wrote:
> Am 3/8/2012 22:35, schrieb Jonathan Nieder:

>> (By the way, what platforms don't support BIND and a random number
>> generator?)
>
> MinGW, for example:

The ISC seems to provide BIND source code[*] and binaries for Windows,
though I'm not sure how well they could fit into a typical development
environment for git's MinGW port.  A good approach for drand48 is
harder to imagine, mostly because there are too many choices for a
free PRNG to use to replace it.  Not a bad problem to have. ;-)

Though all of that would only come up once someone wants to use
features requiring these facilities on Windows.  Thanks for the
hints and sorry for the distraction.

Ciao,
Jonathan

[*] http://www.isc.org/software/bind

Re: [PATCH 0/5] transport: unify ipv4 and ipv6 code paths

[Erik Faye-Lund]

On Thu, Mar 8, 2012 at 1:48 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hi,
>
> These patches eliminate some ifdef-ery concerning NO_IPV6.  I used
> them when writing the SRV patch, which applies on top, but it's
> probably best to think of it as an independent topic.
>
> Patch 4 is the heart of the series.  It provides an interface similar
> to getaddrinfo that can be implemented on top of either gethostbyname
> or getaddrinfo and puts each implementation in a separate file.  This
> way, callers can just use the common API and they do not need to have
> two copies of their code, one for each host resolution API.
>
> Patches 1-3 move code around until all the code that patch 4 touches
> is in one place.
>
> Patches 5 is a potential error handling improvement noticed while
> writing patches 1-4.  It's probably not actually needed but it was a
> comfort to me.
>
> These patches have been in use in Debian since June of last year.  I'd
> like to see this in mainline early in the 1.7.11 cycle to make coding
> that touches this area during that cycle more pleasant.  Thoughts of
> all kinds welcome.

What happened to this series?

Re: [PATCH 0/5] transport: unify ipv4 and ipv6 code paths

[Junio C Hamano]

Erik Faye-Lund <kusmabite@gmail.com> writes:

> On Thu, Mar 8, 2012 at 1:48 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Hi,
>>
>> These patches eliminate some ifdef-ery concerning NO_IPV6.  I used
>> them when writing the SRV patch, which applies on top, but it's
>> probably best to think of it as an independent topic.
>>
>> Patch 4 is the heart of the series.  It provides an interface similar
>> to getaddrinfo that can be implemented on top of either gethostbyname
>> or getaddrinfo and puts each implementation in a separate file.  This
>> way, callers can just use the common API and they do not need to have
>> two copies of their code, one for each host resolution API.
>>
>> Patches 1-3 move code around until all the code that patch 4 touches
>> is in one place.
>>
>> Patches 5 is a potential error handling improvement noticed while
>> writing patches 1-4.  It's probably not actually needed but it was a
>> comfort to me.
>>
>> These patches have been in use in Debian since June of last year.  I'd
>> like to see this in mainline early in the 1.7.11 cycle to make coding
>> that touches this area during that cycle more pleasant.  Thoughts of
>> all kinds welcome.
>
> What happened to this series?

Yeah, what happened to it?  I was looking at the diff files Ubuntu
applies when generating its recent .deb files, and noticed these
patches today, and was wondering what was going on.

Re: [PATCH 0/5] transport: unify ipv4 and ipv6 code paths

[Jonathan Nieder]

Erik Faye-Lund wrote:
> On Thu, Mar 8, 2012 at 1:48 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

>> Patch 4 is the heart of the series.  It provides an interface similar
>> to getaddrinfo that can be implemented on top of either gethostbyname
>> or getaddrinfo and puts each implementation in a separate file.  This
>> way, callers can just use the common API and they do not need to have
>> two copies of their code, one for each host resolution API.
[...]
> What happened to this series?

It seemed nice in principle, but if I understood correctly then the
API was too complicated for anyone but me to like it.  Instead of an
interface similar to getaddrinfo, it would probably be better to
implement an interface _identical_ to getaddrinfo for the the subset
of functionality that git uses.

See [1] for example for proof that that's possible.

I would make some different choices from Russ if doing it myself ---
e.g., I would want to unconditionally use

	#define getaddrinfo git_getaddrinfo

when NO_IPV6 is set so testing these code paths would be as simple as
enabling NO_IPV6, even on systems that also provide the modern API in
libc.

Hoping that clarifies,
Jonathan

[1] http://git.eyrie.org/?p=devel/rra-c-util.git;a=tree;f=portable