{"thread":{"id":"27807","subject":"With errno fix: [PATCH] Do not log unless all connect() attempts fail","startedAt":"2011-07-13T16:28:18Z","lastAt":"2011-07-13T22:38:20Z","messageCount":4,"participants":["Dave Zarzycki","Erik Faye-Lund","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"171263","messageId":"7276ACEE-EF52-49DF-83EA-642DE504B3EA@apple.com","threadId":"27807","inReplyTo":null,"subject":"With errno fix: [PATCH] Do not log unless all connect() attempts fail","fromName":"Dave Zarzycki","fromEmail":"zarzycki@apple.com","sentAt":"2011-07-13T16:28:18Z","receivedAt":"2011-07-13T16:28:18Z","isPatch":true,"sender":{"key":"zarzycki@apple.com","avatar":"https://avatars.githubusercontent.com/u/1071982?v=4"},"body":"IPv6 hosts are often unreachable on the primarily IPv4 Internet and\ntherefore we shouldn't print an error if there are still other hosts we\ncan try to connect() to. This helps \"git fetch --quiet\" stay quiet.\n\nSigned-off-by: Dave Zarzycki <zarzycki@apple.com>\n---\n connect.c |   15 +++++++++------\n 1 files changed, 9 insertions(+), 6 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex 2119c3f..87b2e3f 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -192,6 +192,7 @@ static const char *ai_name(const struct addrinfo *ai)\n  */\n static int git_tcp_connect_sock(char *host, int flags)\n {\n+\tstruct strbuf error_message = STRBUF_INIT;\n \tint sockfd = -1, saved_errno = 0;\n \tconst char *port = STR(DEFAULT_GIT_PORT);\n \tstruct addrinfo hints, *ai0, *ai;\n@@ -217,6 +218,11 @@ static int git_tcp_connect_sock(char *host, int flags)\n \t\tfprintf(stderr, \"done.\\nConnecting to %s (port %s) ... \", host, port);\n \n \tfor (ai0 = ai; ai; ai = ai->ai_next) {\n+\t\tif (saved_errno) {\n+\t\t\tstrbuf_addf(&error_message, \"%s[%d: %s]: errno=%s\\n\",\n+\t\t\t\thost, cnt, ai_name(ai), strerror(saved_errno));\n+\t\t\tsaved_errno = 0;\n+\t\t}\n \t\tsockfd = socket(ai->ai_family,\n \t\t\t\tai->ai_socktype, ai->ai_protocol);\n \t\tif (sockfd < 0) {\n@@ -225,11 +231,6 @@ static int git_tcp_connect_sock(char *host, int flags)\n \t\t}\n \t\tif (connect(sockfd, ai->ai_addr, ai->ai_addrlen) < 0) {\n \t\t\tsaved_errno = errno;\n-\t\t\tfprintf(stderr, \"%s[%d: %s]: errno=%s\\n\",\n-\t\t\t\thost,\n-\t\t\t\tcnt,\n-\t\t\t\tai_name(ai),\n-\t\t\t\tstrerror(saved_errno));\n \t\t\tclose(sockfd);\n \t\t\tsockfd = -1;\n \t\t\tcontinue;\n@@ -242,11 +243,13 @@ static int git_tcp_connect_sock(char *host, int flags)\n \tfreeaddrinfo(ai0);\n \n \tif (sockfd < 0)\n-\t\tdie(\"unable to connect a socket (%s)\", strerror(saved_errno));\n+\t\tdie(\"unable to connect to %s:\\n%s\", host, error_message.buf);\n \n \tif (flags & CONNECT_VERBOSE)\n \t\tfprintf(stderr, \"done.\\n\");\n \n+\tstrbuf_release(&error_message);\n+\n \treturn sockfd;\n }\n \n-- \n1.7.6.135.g8cdba\n"},{"id":"171276","messageId":"CABPQNSYWKVngfoj4AqTCbWixENSmgnbdDBGBt+EpWnBgbqpofg@mail.gmail.com","threadId":"27807","inReplyTo":"7276ACEE-EF52-49DF-83EA-642DE504B3EA@apple.com","subject":"Re: With errno fix: [PATCH] Do not log unless all connect() attempts fail","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-07-13T18:47:24Z","receivedAt":"2011-07-13T18:47:24Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Wed, Jul 13, 2011 at 6:28 PM, Dave Zarzycki <zarzycki@apple.com> wrote:\n> IPv6 hosts are often unreachable on the primarily IPv4 Internet and\n> therefore we shouldn't print an error if there are still other hosts we\n> can try to connect() to. This helps \"git fetch --quiet\" stay quiet.\n>\n> Signed-off-by: Dave Zarzycki <zarzycki@apple.com>\n> ---\n>  connect.c |   15 +++++++++------\n>  1 files changed, 9 insertions(+), 6 deletions(-)\n>\n> diff --git a/connect.c b/connect.c\n> index 2119c3f..87b2e3f 100644\n> --- a/connect.c\n> +++ b/connect.c\n> @@ -192,6 +192,7 @@ static const char *ai_name(const struct addrinfo *ai)\n>  */\n>  static int git_tcp_connect_sock(char *host, int flags)\n>  {\n> +       struct strbuf error_message = STRBUF_INIT;\n>        int sockfd = -1, saved_errno = 0;\n>        const char *port = STR(DEFAULT_GIT_PORT);\n>        struct addrinfo hints, *ai0, *ai;\n> @@ -217,6 +218,11 @@ static int git_tcp_connect_sock(char *host, int flags)\n>                fprintf(stderr, \"done.\\nConnecting to %s (port %s) ... \", host, port);\n>\n>        for (ai0 = ai; ai; ai = ai->ai_next) {\n> +               if (saved_errno) {\n> +                       strbuf_addf(&error_message, \"%s[%d: %s]: errno=%s\\n\",\n> +                               host, cnt, ai_name(ai), strerror(saved_errno));\n> +                       saved_errno = 0;\n> +               }\n\nUhm, this will still fail to report errors for the very last entry,\nno? When socket returns -1 in the last iteration (and errno gets\nsaved), there's no code that reports it...\n"},{"id":"171287","messageId":"7vtyap6gox.fsf@alter.siamese.dyndns.org","threadId":"27807","inReplyTo":"CABPQNSYWKVngfoj4AqTCbWixENSmgnbdDBGBt+EpWnBgbqpofg@mail.gmail.com","subject":"Re: With errno fix: [PATCH] Do not log unless all connect() attempts fail","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-07-13T20:58:38Z","receivedAt":"2011-07-13T20:58:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Erik Faye-Lund <kusmabite@gmail.com> writes:\n\n> Uhm, this will still fail to report errors for the very last entry,\n> no? When socket returns -1 in the last iteration (and errno gets\n> saved), there's no code that reports it...\n\nI guess the fix should look something like this.\n\nBy the way, is anybody interested in fixing the other side of the ifdef\nthat is compiled on IPv4-only installations? \n\n connect.c |   15 ++++++---------\n 1 files changed, 6 insertions(+), 9 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex 8eb9f44..844107e 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -193,7 +193,7 @@ static const char *ai_name(const struct addrinfo *ai)\n static int git_tcp_connect_sock(char *host, int flags)\n {\n \tstruct strbuf error_message = STRBUF_INIT;\n-\tint sockfd = -1, saved_errno = 0;\n+\tint sockfd = -1;\n \tconst char *port = STR(DEFAULT_GIT_PORT);\n \tstruct addrinfo hints, *ai0, *ai;\n \tint gai;\n@@ -220,15 +220,12 @@ static int git_tcp_connect_sock(char *host, int flags)\n \tfor (ai0 = ai; ai; ai = ai->ai_next) {\n \t\tsockfd = socket(ai->ai_family,\n \t\t\t\tai->ai_socktype, ai->ai_protocol);\n-\t\tif (sockfd < 0) {\n-\t\t\tsaved_errno = errno;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (connect(sockfd, ai->ai_addr, ai->ai_addrlen) < 0) {\n-\t\t\tsaved_errno = errno;\n+\t\tif ((sockfd < 0) ||\n+\t\t    (connect(sockfd, ai->ai_addr, ai->ai_addrlen) < 0)) {\n \t\t\tstrbuf_addf(&error_message, \"%s[%d: %s]: errno=%s\\n\",\n-\t\t\t\thost, cnt, ai_name(ai), strerror(saved_errno));\n-\t\t\tclose(sockfd);\n+\t\t\t\t    host, cnt, ai_name(ai), strerror(errno));\n+\t\t\tif (0 <= sockfd)\n+\t\t\t\tclose(sockfd);\n \t\t\tsockfd = -1;\n \t\t\tcontinue;\n \t\t}\n"},{"id":"171297","messageId":"CABPQNSZj7P6XUMgChgZ6XYEZVtVNmXmVM27_Ms=mXV1c1an6KQ@mail.gmail.com","threadId":"27807","inReplyTo":"7vtyap6gox.fsf@alter.siamese.dyndns.org","subject":"Re: With errno fix: [PATCH] Do not log unless all connect() attempts fail","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-07-13T22:38:20Z","receivedAt":"2011-07-13T22:38:20Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Wed, Jul 13, 2011 at 10:58 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Erik Faye-Lund <kusmabite@gmail.com> writes:\n>\n>> Uhm, this will still fail to report errors for the very last entry,\n>> no? When socket returns -1 in the last iteration (and errno gets\n>> saved), there's no code that reports it...\n>\n> I guess the fix should look something like this.\n>\n> By the way, is anybody interested in fixing the other side of the ifdef\n> that is compiled on IPv4-only installations?\n>\n>  connect.c |   15 ++++++---------\n>  1 files changed, 6 insertions(+), 9 deletions(-)\n>\n> diff --git a/connect.c b/connect.c\n> index 8eb9f44..844107e 100644\n> --- a/connect.c\n> +++ b/connect.c\n> @@ -193,7 +193,7 @@ static const char *ai_name(const struct addrinfo *ai)\n>  static int git_tcp_connect_sock(char *host, int flags)\n>  {\n>        struct strbuf error_message = STRBUF_INIT;\n> -       int sockfd = -1, saved_errno = 0;\n> +       int sockfd = -1;\n>        const char *port = STR(DEFAULT_GIT_PORT);\n>        struct addrinfo hints, *ai0, *ai;\n>        int gai;\n> @@ -220,15 +220,12 @@ static int git_tcp_connect_sock(char *host, int flags)\n>        for (ai0 = ai; ai; ai = ai->ai_next) {\n>                sockfd = socket(ai->ai_family,\n>                                ai->ai_socktype, ai->ai_protocol);\n> -               if (sockfd < 0) {\n> -                       saved_errno = errno;\n> -                       continue;\n> -               }\n> -               if (connect(sockfd, ai->ai_addr, ai->ai_addrlen) < 0) {\n> -                       saved_errno = errno;\n> +               if ((sockfd < 0) ||\n> +                   (connect(sockfd, ai->ai_addr, ai->ai_addrlen) < 0)) {\n>                        strbuf_addf(&error_message, \"%s[%d: %s]: errno=%s\\n\",\n> -                               host, cnt, ai_name(ai), strerror(saved_errno));\n> -                       close(sockfd);\n> +                                   host, cnt, ai_name(ai), strerror(errno));\n> +                       if (0 <= sockfd)\n> +                               close(sockfd);\n>                        sockfd = -1;\n>                        continue;\n>                }\n>\n\nYeah, this looks like sensible to me.\n"}]}