On Wed, 22 Feb 2006, Ignatios Souvatzis wrote:

> On Tue, Feb 21, 2006 at 10:20:53PM +0100, Ignatios Souvatzis wrote:
> > On Tue, Feb 21, 2006 at 01:11:52PM -0800, jonathan@dsg.stanford.edu wrote:
> > Yes, and that error message is bogus as you noted: [...]
>
> I've looked at the code - IMHO there is a clean one-line patch:

I've not looked at the code (ok just a sneak peek), but it strikes me that
there could be other errors returned here (ENOMEM for instance), so I
propose a different one liner, which is more accurate if you just consider
that specific result to be not an error at this time.

-         if (sock < 0)
+         if (sock < 0 && errno != EPROTONOSUPPORT)

> RCS file: /cvsroot/src/crypto/dist/ssh/sshconnect.c,v
> retrieving revision 1.31
> diff -u -r1.31 sshconnect.c
> --- sshconnect.c        23 Apr 2005 16:53:29 -0000      1.31
> +++ sshconnect.c        22 Feb 2006 10:29:20 -0000
> @@ -188,7 +188,7 @@
>         }
>         sock = socket(ai->ai_family, ai->ai_socktype, ai->ai_protocol);
>         if (sock < 0)
> -               error("socket: %.100s", strerror(errno));
> +               debug("socket: %.100s", strerror(errno));
>
>         /* Bind the socket to an alternative local IP address */
>         if (options.bind_address == NULL)
>
> Note that the last setting of errno is printed after the for loop in the
> function ssh_connect(), anyway - see line 395, so if _no_ address has a
> supported address family, the user would be notified.

I wish I hadnt taken that sneak peek now. Maybe it can't happen for other
reasons, but as error() is not an exit function, close(-1) and bind(-1)
both seem to be possible when (options.bind_address != NULL) and the
socket() failed

better to do:

	if (sock < 0) {
		if (errno != EPROTONOSUPPORT)
			error(...);

		return -1;
	}

Also, if you did change the error() to debug() then you should really
change the comment on line 391 "Any error is already output" to something
more appropriate. I would suggest "Not necessarily an error"

iain