From: Erik Faye-Lund <kusmabite@gmail.com>
Date: Wed, 13 Jul 2011 09:23:33 GMT
Subject: Re: [PATCH] Do not log unless all connect() attempts fail
Message-ID: <CABPQNSaPXmHE1qECUbG9oWU43HbAXxAY42T1P=MNHgkkWM936w@mail.gmail.com>
In-Reply-To: <1EC2718A-A993-443C-8D7C-DEBD7C424EB9@apple.com>

On Tue, Jul 12, 2011 at 6:28 PM, Dave Zarzycki <zarzycki@apple.com> wrote:
> IPv6 hosts are often unreachable on the primarily IPv4 Internet and
> therefore we shouldn't print an error if there are still other hosts we
> can try to connect() to. This helps "git fetch --quiet" stay quiet.
>
> Signed-off-by: Dave Zarzycki <zarzycki@apple.com>
> ---
>  connect.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/connect.c b/connect.c
> index 2119c3f..8eb9f44 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -192,6 +192,7 @@ static const char *ai_name(const struct addrinfo *ai)
>  */
>  static int git_tcp_connect_sock(char *host, int flags)
>  {
> +       struct strbuf error_message = STRBUF_INIT;
>        int sockfd = -1, saved_errno = 0;
>        const char *port = STR(DEFAULT_GIT_PORT);
>        struct addrinfo hints, *ai0, *ai;
> @@ -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;
> -                       fprintf(stderr, "%s[%d: %s]: errno=%s\n",
> -                               host,
> -                               cnt,
> -                               ai_name(ai),
> -                               strerror(saved_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.