git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* With errno fix: [PATCH] Do not log unless all connect() attempts fail
@ 2011-07-13 16:28 Dave Zarzycki
  2011-07-13 18:47 ` Erik Faye-Lund
  0 siblings, 1 reply; 4+ messages in thread
From: Dave Zarzycki @ 2011-07-13 16:28 UTC (permalink / raw)
  To: git

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.

Signed-off-by: Dave Zarzycki <zarzycki@apple.com>
---
 connect.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/connect.c b/connect.c
index 2119c3f..87b2e3f 100644
--- a/connect.c
+++ b/connect.c
@@ -192,6 +192,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;
 	const char *port = STR(DEFAULT_GIT_PORT);
 	struct addrinfo hints, *ai0, *ai;
@@ -217,6 +218,11 @@ static int git_tcp_connect_sock(char *host, int flags)
 		fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port);
 
 	for (ai0 = ai; ai; ai = ai->ai_next) {
+		if (saved_errno) {
+			strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n",
+				host, cnt, ai_name(ai), strerror(saved_errno));
+			saved_errno = 0;
+		}
 		sockfd = socket(ai->ai_family,
 				ai->ai_socktype, ai->ai_protocol);
 		if (sockfd < 0) {
@@ -225,11 +231,6 @@ 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));
 			close(sockfd);
 			sockfd = -1;
 			continue;
@@ -242,11 +243,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);
 
 	if (flags & CONNECT_VERBOSE)
 		fprintf(stderr, "done.\n");
 
+	strbuf_release(&error_message);
+
 	return sockfd;
 }
 
-- 
1.7.6.135.g8cdba

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: With errno fix: [PATCH] Do not log unless all connect() attempts fail
  2011-07-13 16:28 With errno fix: [PATCH] Do not log unless all connect() attempts fail Dave Zarzycki
@ 2011-07-13 18:47 ` Erik Faye-Lund
  2011-07-13 20:58   ` Junio C Hamano
  0 siblings, 1 reply; 4+ messages in thread
From: Erik Faye-Lund @ 2011-07-13 18:47 UTC (permalink / raw)
  To: Dave Zarzycki; +Cc: git

On Wed, Jul 13, 2011 at 6:28 PM, Dave Zarzycki <zarzycki@apple.com> 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.
>
> Signed-off-by: Dave Zarzycki <zarzycki@apple.com>
> ---
>  connect.c |   15 +++++++++------
>  1 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/connect.c b/connect.c
> index 2119c3f..87b2e3f 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -192,6 +192,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;
>        const char *port = STR(DEFAULT_GIT_PORT);
>        struct addrinfo hints, *ai0, *ai;
> @@ -217,6 +218,11 @@ static int git_tcp_connect_sock(char *host, int flags)
>                fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port);
>
>        for (ai0 = ai; ai; ai = ai->ai_next) {
> +               if (saved_errno) {
> +                       strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n",
> +                               host, cnt, ai_name(ai), strerror(saved_errno));
> +                       saved_errno = 0;
> +               }

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: With errno fix: [PATCH] Do not log unless all connect() attempts fail
  2011-07-13 18:47 ` Erik Faye-Lund
@ 2011-07-13 20:58   ` Junio C Hamano
  2011-07-13 22:38     ` Erik Faye-Lund
  0 siblings, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2011-07-13 20:58 UTC (permalink / raw)
  To: kusmabite; +Cc: Dave Zarzycki, git

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: With errno fix: [PATCH] Do not log unless all connect() attempts fail
  2011-07-13 20:58   ` Junio C Hamano
@ 2011-07-13 22:38     ` Erik Faye-Lund
  0 siblings, 0 replies; 4+ messages in thread
From: Erik Faye-Lund @ 2011-07-13 22:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Dave Zarzycki, git

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2011-07-13 22:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-13 16:28 With errno fix: [PATCH] Do not log unless all connect() attempts fail Dave Zarzycki
2011-07-13 18:47 ` Erik Faye-Lund
2011-07-13 20:58   ` Junio C Hamano
2011-07-13 22:38     ` Erik Faye-Lund

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).