Re: [PATCH v2] strvec: `strvec_splice()` to a statically initialized vector
On Mon, Dec 09, 2024 at 10:56:20AM +0900, Junio C Hamano wrote:
Show 22 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Junio C Hamano <gitster@pobox.com> writes:
> >
> >> Junio C Hamano <gitster@pobox.com> writes:
> >>
> >>> Rubén Justo <rjusto@gmail.com> writes:
> >>>
> >>>>> ...
> >>>>> Sorry. I'll re-roll later today.
> >>>
> >>> No need to say "sorry". Thanks for quickly reacting and starting to
> >>> work on it.
> >>
> >> Any progress?
> >>
> >> Thanks.
> >
> > Sorry, you did send and I did queue v3.
>
> ... and it seems to be causing problems, I didn't look very deep,
> but it looks similar to what I reported for the earlier round.
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.
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.
-Peff
Jeff King <peff@peff.net>