Rubén Justo <rjusto@xxxxxxxxx> writes:
[snip]
>> > Thanks.
>> >
>> > strvec.c | 10 ++++++----
>> > t/unit-tests/strvec.c | 10 ++++++++++
>> > 2 files changed, 16 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/strvec.c b/strvec.c
>> > index d1cf4e2496..087c020f5b 100644
>> > --- a/strvec.c
>> > +++ b/strvec.c
>> > @@ -61,16 +61,18 @@ void strvec_splice(struct strvec *array, size_t idx, size_t len,
>> > {
>> > if (idx + len > array->nr)
>> > BUG("range outside of array boundary");
>> > - if (replacement_len > len)
>> > + if (replacement_len > len) {
>> > + if (array->v == empty_strvec)
>> > + array->v = NULL;
>> > ALLOC_GROW(array->v, array->nr + (replacement_len - len) + 1,
>> > array->alloc);
>> > + }
>> > for (size_t i = 0; i < len; i++)
>> > free((char *)array->v[idx + i]);
>> > - if (replacement_len != len) {
>> > + if ((replacement_len != len) && array->nr)
>> > memmove(array->v + idx + replacement_len, array->v + idx + len,
>> > (array->nr - idx - len + 1) * sizeof(char *));
>> > - array->nr += (replacement_len - len);
>> > - }
>> > + array->nr += replacement_len - len;
>>
>> Why is this second block of changes needed? Will array-nr ever be 0 when
>> we reach here?
>
> I'm not sure I understand your questions.
>
> At that point, `array->nr` is the initial number of entries in the
> vector. It can be 0 when `strvec_splice()` is applied to an empty
> vector.
>
> We are moving the line where we update "array->nr" outside the `if`
> block because we want to do it even when we are not moving existing
> entries. Again, this happens when `strvec_splice()` is applied to an
> empty vector.
>
Ah. I was considering that ALLOC_GROW would update `array->nr`, but it
doesn't. So you're right.
> Finally, we don't mind too much (or value more the simplicity) of the
> now unconditional update of "array->nr" because a clever compiler will
> give us the third arm of the if: "else -> do nothing". When
> `replacement_len == len` => "array->nr += 0" => do nothing.
>
Indeed.
Thanks
Attachment:
signature.asc
Description: PGP signature
