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

From: Junio C Hamano <[email-masked]>
Date: Mon, 09 Dec 2024 16:33:33 +0900

Jeff King <[email-masked]> writes:

> I think it is this off-by-one:
>
> diff --git a/strvec.c b/strvec.c
> index 62283fcef2..d67596e571 100644
> --- a/strvec.c
> +++ b/strvec.c
> @@ -66,7 +66,7 @@ void strvec_splice(struct strvec *array, size_t idx, size_t len,
>  			array->v = NULL;
>  		ALLOC_GROW(array->v, array->nr + (replacement_len - len) + 1,
>  			   array->alloc);
> -		array->v[array->nr + (replacement_len - len) + 1] = NULL;
> +		array->v[array->nr + (replacement_len - len)] = NULL;
>  	}
>  	for (size_t i = 0; i < len; i++)
>  		free((char *)array->v[idx + i]);
>
> We allocate with "+1" to account for the NULL, but when we index to
> assign the slot, we count from 0.

Ah, of course.  Usually v[len] is what you never touch (because
0..(len-1) are the valid index into an array of length len), unless
the array has a sentinel at the end, in which case you have the
sentinel there.  v[len + 1] would obviously be out of bounds.

> Or more concretely for the test case, we are adding 1 replacement item
> to a 0-element array, and the result will have 1 item. So we allocate
> 2 slots, and slot 1 is the NULL.

Thanks.