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

From: Junio C Hamano <[email-masked]>
Date: Tue, 11 Mar 2025 09:57:19 -0700

Christian Couder <[email-masked]> writes:

> 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.