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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]