Subject: Re: [PATCH v2] strvec: `strvec_splice()` to a statically initialized vector

From: Junio C Hamano <[email-masked]>
Date: Wed, 04 Dec 2024 09:09:36 +0900

Rubén Justo <[email-masked]> writes:

> Note that an empty strvec instance (with zero elements) does not
> necessarily need to be an instance initialized with the singleton.

Correct.

When (vec.nr == 0), vec.v may be pointing at 

 (1) an allocated piece of memory, if the strvec was previously used
     to hold some strings; or
 (2) singleton array with NULL.

and vec.v[0] is NULL.  This is to allow you to pass vec.v[] as a
NULL terminated list of (char *) (aka argv[][]) to functions.

That can be said a bit differently and more concisely like so:

    A strvec instance with no elements can have its member .v
    pointing at empty_strvec[] or pointing at an allocated piece of
    memory, and either way .v[0] has NULL in it, to allow you to
    always treat vec.v[] as a NULL terminated array of strings, even
    immediately after initialization.

and then you can lose the strvec_pop() illustration below that talks
about an allocated piece of memory that was previously used.

> The recently introduced `strvec_splice()` API is expected to be
> normally used with non-empty strvec's.

It is perfectly sensible to expect that you can splice your stuff
into an empty strvec, so all this sentence is saying is that a
strvec is more often non-empty than empty. I'd recommend dropping
this sentence.

Something like

    When growing a strvec, we'd use a realloc() call on its .v[]
    member, but a care must be taken when it is pointing at
    empty_strvec[] and is not pointing at an allocated piece of
    memory.  strvec_push_nodup() and strvec_push() correctly do so.
    The recently added strvec_splice() forgot to.

should be sufficient.  Notice that I didn't have to invent a new
term "empty-singleton" at all ;-).

Thanks.