From: Torsten Bögershausen Date: Mon, 23 May 2016 04:31:53 GMT Subject: Re: [PATCH v7 1/9] connect: document why we sometimes call get_port after get_host_and_port Message-ID: <574287B9.4090307@web.de> In-Reply-To: <20160522080316.GA19790@glandium.org> On 05/22/2016 10:03 AM, Mike Hommey wrote: > On Sun, May 22, 2016 at 08:07:05AM +0200, Torsten Bögershausen wrote: >> On 22.05.16 01:17, Mike Hommey wrote: >>> Signed-off-by: Mike Hommey >>> --- >>> connect.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/connect.c b/connect.c >>> index c53f3f1..caa2a3c 100644 >>> --- a/connect.c >>> +++ b/connect.c >>> @@ -742,6 +742,12 @@ struct child_process *git_connect(int fd[2], const char *url, >>> transport_check_allowed("ssh"); >>> get_host_and_port(&ssh_host, &port); >>> >>> + /* get_host_and_port may not return a port even when >>> + * there is one: In the [host:port]:path case, >>> + * get_host_and_port is called with "[host:port]" and >>> + * returns "host:port" and NULL. >>> + * In that specific case, we still need to split the >>> + * port. */ >> Is it worth to mention that this case is "still supported legacy" ? > If it's worth mentioning anywhere, it seems to me it would start with > urls.txt? > > Mike > I don't know. urls.txt is for Git users, and connect.c is for Git developers. urls.txt does not mention that Git follows any RFC when parsing the URLS', it doesn't claim to be 100% compliant. Even if it makes sense to do so, as many user simply expect Git to accept RFC compliant URL's, and it makes the development easier, if there is an already written specification, that describes all the details. The parser is not 100% RFC compliant, one example: - old-style usgage like "git clone [host:222]:~/path/to/repo are supported To easy the live for developers, it could make sense why the code is as it is, simply to avoid that people around the world suddenly run into problems, when they upgrade the Git version. So if there is a comment in the code, it could help new developers to understand things easier.