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

Victor Bogado da Silva Lins· Jun 5, 2008, 12:48 UTC · lore

>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(-)

Show changes to remote.c +1 −1

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

Johannes Schindelin· Jun 5, 2008, 13:26 UTC · re: Victor Bogado da Silva Lins · lore

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

Hi,

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

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

Show 5 quoted lines

> >>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.

Show 5 quoted lines

> 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.

Show 11 quoted lines

> 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

patchFix remote_get so it will return NULL when no remote is found.