Re: [PATCH] promisor-remote: fix segfault when remote URL is missing

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 11, 2025, 16:57 UTC
Message-ID: <xmqqplin4ia8.fsf@gitster.g>
In-Reply-To: <CAP8UFD1ujnSYEnNO4xifwXj397V-=1kJSMGJ3j613+qVAkyg=Q@mail.gmail.com>

Christian Couder <christian.couder@gmail.com> writes:

Show 9 quoted lines

> Actually strvec_push() uses xstrdup() on the value it is passed and
> xstrdup() crashes if that value is NULL. So another way to avoid
> crashes would be to make xstrdup() BUG when it's passed NULL. Or maybe
> xstrdup() should just return NULL in this case?
>
> Also it looks like strvec_push_nodup() kind of works if it is passed a
> NULL. (It adds the NULL to the array and grows it.) So I wonder if the
> right solution for strvec_push() would be to make it kind of work in
> the same way.

Meaning "xstrdup" -> "xstrdup_or_null"?

It actually may be a reasonable thing to do, to remain parallel to the _nodup() variant, but given strvec is about making it easier to do argc/argv[], allowing NULL in it does not sound like it is in line with its stated purpose.

I also do not think checking inside xstrdup() is a good idea; it is at way too low a level.

> Anyway I think these are separate issues that deserve their own
> discussions and can wait for after the 2.49.0 release.

Absolutely.

> Here I am just
> providing a hotfix for the "promisor-remote" protocol capability.

Ooof, I didn't realize that cc/lop-remote escaped 'seen' with such a bug X-<.

Thanks.