On Thursday 13 August 2009, Junio C Hamano wrote:
> Johan Herland <johan@xxxxxxxxxxx> writes:
> > Signed-off-by: Johan Herland <johan@xxxxxxxxxxx>
>
> How does url end up to be NULL? At the beginning of transport_get(), you
> do this:
>
> ret->remote = remote;
> if (!url && remote && remote->url)
> url = remote->url[0];
> ret->url = url;
> if (remote && remote->foreign_vcs) {
> transport_helper_init(ret);
> return ret;
> }
>
> and the case where remote.$name.vcs is defined, we do not need
> remote.$name.url.
>
> When (!url && remote && remote->url), is remote->url[0] allowed to be
> NULL? I am guessing it would be a bug in whoever prepared the remote,
> and if that is indeed the case, the patch shifts the symptoms without
> fixing the cause.
>
> When (remote && remote->foreign_vcs) does not hold, iow, if no remote is
> defined or the remote is defined but lacks remote.$name.url, you will go
> to the last else clause in the function that sets up a git_transport_data
> for the native transport, but it has ret->url == NULL.
>
> Whom does that transport talk with? Is such a transport of any use, or
> does it cause a segfault downstream in the call chain?
>
> In other words, I am wondering if this patch should just diagnose the
> case as an error, instead of pretending all is well.
You are right. Instead of pushing the segfault downstream, we should verify
that the url is non-NULL before returning it as part of ret (modulo the
foreign case, of course).
...Johan
--
Johan Herland, <johan@xxxxxxxxxxx>
www.herland.net
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html