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

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 9, 2024, 07:33 UTC
Message-ID: <xmqq5xntpaya.fsf@gitster.g>
In-Reply-To: <20241209021556.GA1293399@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 18 quoted lines

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