[PATCH] Require a struct remote in transport_get()

[PATCH] Require a struct remote in transport_get()

[Daniel Barkalow]

cmd_ls_remote() was calling transport_get() with a NULL remote and a
non-NULL url in the case where it was run outside a git
repository. This involved a bunch of ill-tested special
cases. Instead, simply get the struct remote for the URL with
remote_get(), which works fine outside a git repository, and can also
take global options into account.

This fixes a tiny and obscure bug where "git ls-remote" without a repo
didn't support global url.*.insteadOf, even though "git clone" and
"git ls-remote" in any repo did.

Also, enforce that all callers provide a struct remote to transport_get().

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
This is sufficient to stop the segfault when tring "git ls-remote 
http://..." outside of a repo, but not to make it work, which requires 
either something simple but not ideal or something complex.

 builtin-ls-remote.c |    6 +++---
 transport.c         |    7 +++++--
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/builtin-ls-remote.c b/builtin-ls-remote.c
index 78a88f7..b5bad0c 100644
--- a/builtin-ls-remote.c
+++ b/builtin-ls-remote.c
@@ -86,10 +86,10 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 			pattern[j - i] = p;
 		}
 	}
-	remote = nongit ? NULL : remote_get(dest);
-	if (remote && !remote->url_nr)
+	remote = remote_get(dest);
+	if (!remote->url_nr)
 		die("remote %s has no configured URL", dest);
-	transport = transport_get(remote, remote ? remote->url[0] : dest);
+	transport = transport_get(remote, remote->url[0]);
 	if (uploadpack != NULL)
 		transport_set_option(transport, TRANS_OPT_UPLOADPACK, uploadpack);
 
diff --git a/transport.c b/transport.c
index 644a30a..298dc46 100644
--- a/transport.c
+++ b/transport.c
@@ -812,6 +812,9 @@ struct transport *transport_get(struct remote *remote, const char *url)
 {
 	struct transport *ret = xcalloc(1, sizeof(*ret));
 
+	if (!remote)
+		die("No remote provided to transport_get()");
+
 	ret->remote = remote;
 	ret->url = url;
 
@@ -849,10 +852,10 @@ struct transport *transport_get(struct remote *remote, const char *url)
 		data->thin = 1;
 		data->conn = NULL;
 		data->uploadpack = "git-upload-pack";
-		if (remote && remote->uploadpack)
+		if (remote->uploadpack)
 			data->uploadpack = remote->uploadpack;
 		data->receivepack = "git-receive-pack";
-		if (remote && remote->receivepack)
+		if (remote->receivepack)
 			data->receivepack = remote->receivepack;
 	}
 
-- 
1.6.5.2.142.g063c5.dirty

Re: [PATCH] Require a struct remote in transport_get()

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> cmd_ls_remote() was calling transport_get() with a NULL remote and a
> non-NULL url in the case where it was run outside a git
> repository. This involved a bunch of ill-tested special
> cases. Instead, simply get the struct remote for the URL with
> remote_get(), which works fine outside a git repository, and can also
> take global options into account.
>
> This fixes a tiny and obscure bug where "git ls-remote" without a repo
> didn't support global url.*.insteadOf, even though "git clone" and
> "git ls-remote" in any repo did.
>
> Also, enforce that all callers provide a struct remote to transport_get().
>
> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> ---
> This is sufficient to stop the segfault when tring "git ls-remote 
> http://..." outside of a repo, but not to make it work, which requires 
> either something simple but not ideal or something complex.

Thanks; I think this and your other patch are important fixes, and should
go directly on 'maint'.  Do you prefer to queue them on 'next' to cook for
a week instead?

Re: [PATCH] Require a struct remote in transport_get()

[Daniel Barkalow]

On Tue, 3 Nov 2009, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > cmd_ls_remote() was calling transport_get() with a NULL remote and a
> > non-NULL url in the case where it was run outside a git
> > repository. This involved a bunch of ill-tested special
> > cases. Instead, simply get the struct remote for the URL with
> > remote_get(), which works fine outside a git repository, and can also
> > take global options into account.
> >
> > This fixes a tiny and obscure bug where "git ls-remote" without a repo
> > didn't support global url.*.insteadOf, even though "git clone" and
> > "git ls-remote" in any repo did.
> >
> > Also, enforce that all callers provide a struct remote to transport_get().
> >
> > Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> > ---
> > This is sufficient to stop the segfault when tring "git ls-remote 
> > http://..." outside of a repo, but not to make it work, which requires 
> > either something simple but not ideal or something complex.
> 
> Thanks; I think this and your other patch are important fixes, and should
> go directly on 'maint'.  Do you prefer to queue them on 'next' to cook for
> a week instead?

I don't think a week on 'next' is likely to turn up any new information; 
these are all uncommon code paths. It might be worth seeing if the 
original reporter is happy with how it's behaving now, though.

	-Daniel
*This .sig left intentionally blank*