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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 13, 2011, 20:39 UTC
Message-ID: <7v1uxt7w5l.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CABPQNSaPXmHE1qECUbG9oWU43HbAXxAY42T1P=MNHgkkWM936w@mail.gmail.com>

Erik Faye-Lund <kusmabite@gmail.com> writes:

Show 29 quoted lines

>> 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...