Re: [PATCH] imap-send: add support for IPv6

From: Benjamin Kramer <benny.kra@googlemail.com>
Date: May 27, 2009, 10:50 UTC
Message-ID: <4A1D1AE3.9030900@googlemail.com>
In-Reply-To: <7v7i03kqjq.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

> We already use %h length specifier to explicitly say the parameter is
> a short in the IPV4 part of this program, so I am sure this won't regress
> anything for people, but I wonder what the point of it is...  (I am not
> asking nor even suggesting to change this, by the way).

getaddrinfo(3) takes the port as a string so we have to convert it. I tried to match the style of the existing code with the format string. The portstr buffer is 6 chars long so the highest possible unsigned short 65535 fits in exactly.

> It is Ok for now (as existing codepath liberally uses fprintf() and
> fputs() to report errors), but ideally we should start converting these to
> error() calls, I think, in a follow-up patch.

Looking at the number of fprintfs in imap-send.c a simple search/replace probably won't do the job here. I tried to contact the original author but his mail address seems to be dead ...

> Is forcing to NUMERICHOST done to match IPV4 codepath that does
> inet_ntoa()?  I guess that makes sense.

We need to get the IP string, otherwise the output of imap-send would make no sense. Here's what imap-send outputs when I try to connect to localhost.

Without the patch:
    Resolving localhost... ok
    Connecting to 127.0.0.1:993... connect: Connection refused

With the patch:
    Resolving localhost... ok
    Connecting to [::1]:993... connect: Connection refused
    Connecting to [fe80::1%lo0]:993... connect: Connection refused
    Connecting to [127.0.0.1]:993... connect: Connection refused
    Error: unable to connect to server.

Using the hostname instead of the IP address here wouldn't be very useful.