[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 <[hidden email]>
---
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Junio C Hamano]

Daniel Barkalow <[hidden email]> 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 <[hidden email]>
> ---
> 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?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Daniel Barkalow]

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

> Daniel Barkalow <[hidden email]> 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 <[hidden email]>
> > ---
> > 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*
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html