From: Mike Hommey <mh@glandium.org>
Date: Mon, 16 May 2016 22:53:41 GMT
Subject: Re: [PATCH v5 4/9] connect: make parse_connect_url() return separated host and port
Message-ID: <20160516225341.GA19595@glandium.org>
In-Reply-To: <xmqqr3d1zkz7.fsf@gitster.mtv.corp.google.com>

On Mon, May 16, 2016 at 03:39:08PM -0700, Junio C Hamano wrote:
> Mike Hommey <mh@glandium.org> writes:
> 
> > +	get_host_and_port(&host, &port);
> > +
> > +	if (*host && !port) {
> > +		/* The host might contain a user:password string, ignore it
> > +		 * when searching for the port again */
> > +		char *end_user = strrchr(host, '@');
> > +		port = get_port(end_user ? end_user : host);
> 
> Scanning from the right because host part would never have '@', but
> there could be an invalid URL with an unquoted '@' in userinfo part?
> Then this makes sense.

Indeed, I forgot to update the comment after the discussion on this
list.

> > diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
> > index 91a69fc..9acba2b 100755
> > --- a/t/t5500-fetch-pack.sh
> > +++ b/t/t5500-fetch-pack.sh
> > @@ -553,7 +553,7 @@ check_prot_path () {
> >  	Diag: protocol=$2
> >  	Diag: path=$3
> >  	EOF
> > -	git fetch-pack --diag-url "$1" | grep -v hostandport= >actual &&
> > +	git fetch-pack --diag-url "$1" | grep -v host= | grep -v port= >actual &&
> 
> A single process:
> 
> 	... | grep -v -e '^host=' -e '^port='

heh, I'm actually replacing it with a single egrep in a subsequent
patch, following feedback from previous round, but missed that there was
this intermediate step with multiple greps still. Do you want an update
anyways, or does only the final result really matter? I guess I can
update it in any case, since I'll have to update the patch for the
comment above anyways.

Mike