From: Erik Faye-Lund <kusmabite@gmail.com>
Date: Wed, 13 Jul 2011 22:38:20 GMT
Subject: Re: With errno fix: [PATCH] Do not log unless all connect() attempts fail
Message-ID: <CABPQNSZj7P6XUMgChgZ6XYEZVtVNmXmVM27_Ms=mXV1c1an6KQ@mail.gmail.com>
In-Reply-To: <7vtyap6gox.fsf@alter.siamese.dyndns.org>

On Wed, Jul 13, 2011 at 10:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Erik Faye-Lund <kusmabite@gmail.com> writes:
>
>> Uhm, this will still fail to report errors for the very last entry,
>> no? When socket returns -1 in the last iteration (and errno gets
>> saved), there's no code that reports it...
>
> I guess the fix should look something like this.
>
> By the way, is anybody interested in fixing the other side of the ifdef
> that is compiled on IPv4-only installations?
>
>  connect.c |   15 ++++++---------
>  1 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/connect.c b/connect.c
> index 8eb9f44..844107e 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -193,7 +193,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;
> +       int sockfd = -1;
>        const char *port = STR(DEFAULT_GIT_PORT);
>        struct addrinfo hints, *ai0, *ai;
>        int gai;
> @@ -220,15 +220,12 @@ static int git_tcp_connect_sock(char *host, int flags)
>        for (ai0 = ai; ai; ai = ai->ai_next) {
>                sockfd = socket(ai->ai_family,
>                                ai->ai_socktype, ai->ai_protocol);
> -               if (sockfd < 0) {
> -                       saved_errno = errno;
> -                       continue;
> -               }
> -               if (connect(sockfd, ai->ai_addr, ai->ai_addrlen) < 0) {
> -                       saved_errno = errno;
> +               if ((sockfd < 0) ||
> +                   (connect(sockfd, ai->ai_addr, ai->ai_addrlen) < 0)) {
>                        strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n",
> -                               host, cnt, ai_name(ai), strerror(saved_errno));
> -                       close(sockfd);
> +                                   host, cnt, ai_name(ai), strerror(errno));
> +                       if (0 <= sockfd)
> +                               close(sockfd);
>                        sockfd = -1;
>                        continue;
>                }
>

Yeah, this looks like sensible to me.