Re: [PATCH] Only print an error for the last connect() failure

[Jeff King <peff@peff.net>]

On Thu, Jun 09, 2011 at 09:18:10AM -0700, Dave Zarzycki 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.
> ---
>  connect.c |   12 +++++++-----
>  1 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/connect.c b/connect.c
> index 2119c3f..7f70ce7 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -225,11 +225,13 @@ 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));
> +			if (ai->ai_next == NULL) {
> +				fprintf(stderr, "%s[%d: %s]: errno=%s\n",
> +					host,
> +					cnt,
> +					ai_name(ai),
> +					strerror(saved_errno));
> +			}

I agree being noisy about early failures when we succeed later is a bad
thing. But when we fail completely, doesn't your code now mask early
failures and print only the final one? The early failures might be the
important ones for the user.

So perhaps we should do something instead like:

  struct strbuf error_message = STRBUF_INIT;
  ...
  if (connect(...) < 0) {
          strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n",
                      host, cnt, ai_name(ai), strerror(errno));
          ...
  }

  if (sockfd < 0)
          die("unable to connect to %s:\n%s", host, error_message.buf);
  strbuf_release(&error_message);

-Peff