Re: [PATCH] Do not log unless all connect() attempts fail

Erik Faye-Lund [off-list ref] writes:

quoted

diff --git a/connect.c b/connect.c
index 2119c3f..8eb9f44 100644
--- a/connect.c
+++ b/connect.c

@@ -225,11 +226,8 @@ static int git_tcp_connect_sock(char *host, int flags)
               }
               if (connect(sockfd, ai->ai_addr, ai->ai_addrlen) < 0) {
                       saved_errno = errno;
+                       strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n",
+                               host, cnt, ai_name(ai), strerror(saved_errno));
                       close(sockfd);
                       sockfd = -1;
                       continue;

@@ -242,11 +240,13 @@ static int git_tcp_connect_sock(char *host, int flags)
       freeaddrinfo(ai0);

       if (sockfd < 0)
-               die("unable to connect a socket (%s)", strerror(saved_errno));
+               die("unable to connect to %s:\n%s", host, error_message.buf);

This kills the output from the case where "sockfd < 0" evaluates to
true for the last entry in ai, no (just above your second hunk), no?
In that case errno gets copied to saved_errno, and the old output
would do strerror(old_errno), but now you just print the log you've
gathered, and don't even look at saved_errno.

If this is intentional then you should probably kill the saved_errno
variable also, it's rendered pointless by this patch.

As error_message.buf contains strerror([saved_]errno), I think it is
intentional to remove strerror() from the final die().  Without looking at
the code outside the context of the patch I cannot tell if saved_errno has
become unneeded, but I tend to trust your analysis so...