# [PATCH] Require a struct remote in transport_get() 3 messages from 2009-11-04 to 2009-11-04. Participants: Daniel Barkalow, Junio C Hamano. Thread: https://gitlist.dev/t/21474 ## Daniel Barkalow, 2009-11-04 02:38 Subject: [PATCH] Require a struct remote in transport_get() Message-ID: URL: https://gitlist.dev/e/alpine.LNX.2.00.0911032133540.14365%40iabervon.org ``` 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 --- 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 ``` ## Junio C Hamano, 2009-11-04 05:42 Subject: Re: [PATCH] Require a struct remote in transport_get() Message-ID: <7vbpjin8v0.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vbpjin8v0.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Daniel Barkalow 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 > --- > 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? ``` ## Daniel Barkalow, 2009-11-04 07:27 Subject: Re: [PATCH] Require a struct remote in transport_get() Message-ID: URL: https://gitlist.dev/e/alpine.LNX.2.00.0911040221100.14365%40iabervon.org In-Reply-To: <7vbpjin8v0.fsf@alter.siamese.dyndns.org> ``` On Tue, 3 Nov 2009, Junio C Hamano wrote: > Daniel Barkalow 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 > > --- > > 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* ```