[PATCH] Fix remote_get so it will return NULL when no remote is found.

[PATCH] Fix remote_get so it will return NULL when no remote is found.

[Victor Bogado da Silva Lins]

>From 0cf45f264cf7f1b3aa3a8875109fbf4c03d56126 Mon Sep 17 00:00:00 2001
From: Victor Bogado <victor@bogado.net>
Date: Thu, 5 Jun 2008 09:36:41 -0300
Subject: [PATCH] Fix remote_get so it will return NULL when no remote is
found.

remote_get should return NULL when there is no remote with that name, at
least this is what remote.c's rm() function seems to think. As this is a
reasonable assumption, and it seems that the function remote_get is
acutally trying to do this, I fixed the test so it will test if the URL
is equal to the name of the remote.
---
 remote.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/remote.c b/remote.c
index 91e3b11..62b3611 100644
--- a/remote.c
+++ b/remote.c
@@ -598,7 +598,7 @@ struct remote *remote_get(const char *name)
 	}
 	if (!ret->url)
 		add_url_alias(ret, name);
-	if (!ret->url)
+	if (!strcmp(*ret->url,ret->name))
 		return NULL;
 	ret->fetch = parse_fetch_refspec(ret->fetch_refspec_nr,
ret->fetch_refspec);
 	ret->push = parse_push_refspec(ret->push_refspec_nr,
ret->push_refspec);
-- 
1.5.5

Re: [PATCH] Fix remote_get so it will return NULL when no remote is found.

[Johannes Schindelin]

Hi,

[nice to go to sleep and see a patch in the morning]

On Thu, 5 Jun 2008, Victor Bogado da Silva Lins wrote:

> >>From 0cf45f264cf7f1b3aa3a8875109fbf4c03d56126 Mon Sep 17 00:00:00 2001
> From: Victor Bogado <victor@bogado.net>
> Date: Thu, 5 Jun 2008 09:36:41 -0300
> Subject: [PATCH] Fix remote_get so it will return NULL when no remote is
> found.

Please imitate what you see on this list, especially by the Git regulars.  
They never include these headers.

> remote_get should return NULL when there is no remote with that name, at 
> least this is what remote.c's rm() function seems to think. As this is a 
> reasonable assumption, and it seems that the function remote_get is 
> acutally trying to do this, I fixed the test so it will test if the URL 
> is equal to the name of the remote.

This is a bit vague, a bit wrong, and it has the subjective "I" in it.  
remote_get() _is_ assumed to return NULL when no remote with that name was 
found.

And the function was fixed (3rd person, passive).  At least that is how I 
remember most of the rest of git.git's commit messages.

> diff --git a/remote.c b/remote.c
> index 91e3b11..62b3611 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -598,7 +598,7 @@ struct remote *remote_get(const char *name)
>  	}
>  	if (!ret->url)
>  		add_url_alias(ret, name);
> -	if (!ret->url)
> +	if (!strcmp(*ret->url,ret->name))
>  		return NULL;

Hmm.  That sounds very dangerous, using a pointer without checking that it 
is NULL.

Besides, I think that the add_url_alias(ret, name) above is at 
fault.  Not the alias part about it, though.  This used to be an add_url() 
from the conception of remote.c, but then it was only used for fetching.

I think the proper solution is to add a parameter, or a function, to force 
NULL if the remote did not exist.

Of course, this involves a careful review of the existing callers, to find 
out which ones rely on getting a newly created remote when none was found.

In any case, thank you very much for finding that bug (the if() I pointed 
out was in remote.c from the very beginning, even then never being 
triggered).

Ciao,
Dscho