From: Johannes Schindelin Date: Thu, 05 Jun 2008 13:26:30 GMT Subject: Re: [PATCH] Fix remote_get so it will return NULL when no remote is found. Message-ID: In-Reply-To: <1212670091.30293.3.camel@omicron.ep.petrobras.com.br> 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 > 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