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

> 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);