Re: [RFC/PATCH 2/6] transport_get(): Don't SEGFAULT on missing url
- From
Johan Herland <johan@herland.net>
- Date
- Aug 12, 2009, 23:39 UTC
- Message-ID
- <200908130139.10876.johan@herland.net>
- In-Reply-To
- <7vab24wtzo.fsf@alter.siamese.dyndns.org>
On Thursday 13 August 2009, Junio C Hamano wrote:
Show 33 quoted lines
> Johan Herland <johan@herland.net> writes:
> > Signed-off-by: Johan Herland <johan@herland.net>
>
> 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@herland.net> www.herland.net