Re: [PATCH v2] Fix remote.<remote>.vcs

From: Daniel Barkalow <barkalow@iabervon.org>
Date: Jan 27, 2010, 23:54 UTC
Message-ID: <alpine.LNX.2.00.1001271853130.14365@iabervon.org>
In-Reply-To: <7vk4v3pabr.fsf@alter.siamese.dyndns.org>

On Wed, 27 Jan 2010, Junio C Hamano wrote:

Show 26 quoted lines

> Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:
> 
> > On Wed, Jan 27, 2010 at 01:39:00PM -0500, Daniel Barkalow wrote:
> >> On Wed, 27 Jan 2010, Ilari Liusvaara wrote:
> >> >  
> >> >  	if (!remote)
> >> >  		die("No remote provided to transport_get()");
> >> >  
> >> >  	ret->remote = remote;
> >> > +	helper = remote->foreign_vcs;
> >> 
> >> Needs to be "helper = remote ? remote->foreign_vcs : NULL", for the same 
> >> reason that the test below had been "remote && remote->foreign_vcs".
> >
> > Few lines above that:
> >
> >      if (!remote)
> >              die("No remote provided to transport_get()");
> 
> Perhaps we would want this micro-clean-up on top then.
> 
> -- >8 --
> Subject: transport_get(): drop unnecessary check for !remote
> 
> At the beginning of the function we make sure remote is not NULL, and
> the remainder of the funciton already depends on it.

I agree with both of these; there used to be code that used a NULL remote and just a URL, but that's gone now.

Show 16 quoted lines

> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> diff --git a/transport.c b/transport.c
> index 87581b8..3846aac 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -921,7 +921,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
>  	ret->remote = remote;
>  	helper = remote->foreign_vcs;
>  
> -	if (!url && remote && remote->url)
> +	if (!url && remote->url)
>  		url = remote->url[0];
>  	ret->url = url;
>  
>