On Mon, Dec 02, 2024 at 10:49:03AM +0900, Junio C Hamano wrote:
> > Let's avoid an invalid pointer error in case a client of
> > `strvec_splice()` ends up with something similar to:
> >
> > struct strvec arr = STRVEC_INIT;
> > const char *rep[] = { "foo" };
> >
> > strvec_splice(&arr, 0, 0, rep, ARRAY_SIZE(rep));
>
> Well spotted, but the explanation can be a bit more helpful to
> casual readers. If there were a few paragraphs like below
>
> An empty strvec does not represent the array part of the
> structure with a NULL pointer, but with a singleton empty array,
> to help read-only applications. This is similar to how an empty
> strbuf uses a singleton empty string.
>
> This approach requires us to be careful when adding elements to
> an empty instance. The strvec_splice() API function we recently
> introduced however forgot to special case an empty strvec, and
> ended up applying realloc() to the singleton.
>
> before your proposed commit log message, I wouldn't have needed to
> go read the implementation of STRVEC_INIT to understand what the fix
> is about. From the fix by itself, it is a bit hard to see why
> empty_strvec needs to be special cased, until you re-read the
> implementation of STRVEC_INIT.
The explanation can certainly be improved. I'll send a v2
iteration soon. Thanks.
