rsync - Feb 2006 - Improved diagnostics patch

Hi, all --

Here's a small patch that gives better diagnostics on the daemon side if it
fails to start up due to inability to create or bind the socket.
Presently, it gives a log entry indicating that no sockets could be bound,
but crucially does *not* have the system error message (i.e. errno) from
the failed call, making it difficult or impossible to determine the
problem.

I do have a couple of misgivings about the patch, however, in that it seems
that it might emit log messages that look like errors even though the
daemon did successfully bind and start, when there are multiple protocols
available and it fails on one but succeeds on another (quoting from the
code's comments: "if for example the machine knows about IPv6 in the C
library, but not in the kernel").  That said, I think that too much
diagnostic information is usually better than not enough, and I think it
would be rare that spurious messages are emitted.

Also, one can't be sure that the address-family and protocol can always be
printed with a printf-code of "%d", although it compiles without
warning on
my machine.  <opinion>This is where choosing a programming language that
supports data encapsulation really helps.</opinion>  The family and
protocol
could just be removed from the message, although it is possible to imagine
that, on a multi-protocol box, there could be multiple failures due to
multiple causes, and it would be necessary to match up the diagnostic error
messages.

-- David Favro
lists.samba.org at meta-dynamic dot com
http://meta-dynamic.com

The patch:
--- socket.c.~1.116.~   2006-02-03 13:46:38.000000000 -0500
+++ socket.c    2006-02-20 07:18:11.000000000 -0500
@@ -364,6 +364,7 @@
                           resp->ai_protocol);

                if (s == -1) {
+                       rsyserr(FERROR, errno, "socket(%d,%d)",
resp->ai_family, resp->ai_protocol );
                        /* See if there's another address that will work...
*/
                        continue;
                }
@@ -385,6 +386,7 @@
                /* Now we've got a socket - we need to bind it. */
                if (bind(s, resp->ai_addr, resp->ai_addrlen) < 0) {
                        /* Nope, try another */
+                       rsyserr(FERROR, errno, "bind()" );
                        close(s);
                        continue;
                }

On Mon, Feb 20, 2006 at 08:03:30PM +0700, David Favro
wrote:
> Here's a small patch that gives better diagnostics on the daemon side
> if it fails to start up due to inability to create or bind the socket.
Thanks for the patch.  I've considered it and your misgivings, and have
come up with the attached revised version that only outputs the extra
socket()/bind() failure messages in the case of a total failure, or when
the daemon was run with at least -vv.  See what you think.

..wayne..
-------------- next part --------------
--- socket.c	7 Mar 2006 08:46:29 -0000	1.118
+++ socket.c	22 Mar 2006 17:47:03 -0000
@@ -333,9 +333,9 @@ static int *open_socket_in(int type, int
 			   int af_hint)
 {
 	int one = 1;
-	int s, *socks, maxs, i;
+	int s, *socks, maxs, i, ecnt;
 	struct addrinfo hints, *all_ai, *resp;
-	char portbuf[10];
+	char portbuf[10], **errmsgs;
 	int error;
 
 	memset(&hints, 0, sizeof hints);
@@ -353,17 +353,25 @@ static int *open_socket_in(int type, int
 	/* Count max number of sockets we might open. */
 	for (maxs = 0, resp = all_ai; resp; resp = resp->ai_next, maxs++) {}
 
-	if (!(socks = new_array(int, maxs + 1)))
+	socks = new_array(int, maxs + 1);
+	errmsgs = new_array(char *, maxs);
+	if (!socks || !errmsgs)
 		out_of_memory("open_socket_in");
 
 	/* We may not be able to create the socket, if for example the
 	 * machine knows about IPv6 in the C library, but not in the
 	 * kernel. */
-	for (resp = all_ai, i = 0; resp; resp = resp->ai_next) {
+	for (resp = all_ai, i = ecnt = 0; resp; resp = resp->ai_next) {
 		s = socket(resp->ai_family, resp->ai_socktype,
 			   resp->ai_protocol);
 
 		if (s == -1) {
+			int r = asprintf(&errmsgs[ecnt++],
+				"socket(%d,%d,%d) failed: %s\n",
+				(int)resp->ai_family, (int)resp->ai_socktype,
+				(int)resp->ai_protocol, strerror(errno));
+			if (r < 0)
+				out_of_memory("open_socket_in");
 			/* See if there's another address that will work... */
 			continue;
 		}
@@ -385,6 +393,10 @@ static int *open_socket_in(int type, int
 		/* Now we've got a socket - we need to bind it. */
 		if (bind(s, resp->ai_addr, resp->ai_addrlen) < 0) {
 			/* Nope, try another */
+			int r = asprintf(&errmsgs[ecnt++],
+				"bind() failed: %s\n", strerror(errno));
+			if (r < 0)
+				out_of_memory("open_socket_in");
 			close(s);
 			continue;
 		}
@@ -396,6 +408,15 @@ static int *open_socket_in(int type, int
 	if (all_ai)
 		freeaddrinfo(all_ai);
 
+	/* Only output the socket()/bind() messages if we were totally
+	 * unsuccessful, or if the daemon is being run with -vv. */
+	for (s = 0; s < ecnt; s++) {
+		if (!i || verbose > 1)
+			rwrite(FLOG, errmsgs[s], strlen(errmsgs[s]));
+		free(errmsgs[s]);
+	}
+	free(errmsgs);
+
 	if (!i) {
 		rprintf(FERROR,
 			"unable to bind any inbound sockets on port %d\n",