Subject: Re: [PATCH 09/26] strvec: introduce new `strvec_splice()` function

From: Rubén Justo <[email-masked]>
Date: Sun, 10 Nov 2024 22:39:08 +0100

On Wed, Nov 06, 2024 at 04:10:44PM +0100, Patrick Steinhardt wrote:
> Introduce a new `strvec_splice()` function that can replace a range of
> strings in the vector with another array of strings. This function will
> be used in subsequent commits.
> 
> Signed-off-by: Patrick Steinhardt <[email-masked]>
> ---
>  strvec.c              | 19 +++++++++++++
>  strvec.h              |  9 ++++++
>  t/unit-tests/strvec.c | 65 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 93 insertions(+)
> 
> diff --git a/strvec.c b/strvec.c
> index f712070f574..81075c50cca 100644
> --- a/strvec.c
> +++ b/strvec.c
> @@ -56,6 +56,25 @@ void strvec_pushv(struct strvec *array, const char **items)
>  		strvec_push(array, *items);
>  }
>  
> +void strvec_splice(struct strvec *array, size_t pos, size_t len,
> +		   const char **replacement, size_t replacement_len)

There's nothing wrong with `replacement_len`, but it has caught my
attention because in other strvec APIs we use null-terminated arrays.

> +{
> +	if (pos + len > array->alloc)
> +		BUG("range outside of array boundary");
> +	if (replacement_len > len)
> +		ALLOC_GROW(array->v, array->nr + (replacement_len - len) + 1,
> +			   array->alloc);
> +	for (size_t i = 0; i < len; i++)
> +		free((char *)array->v[pos + i]);
> +	if (replacement_len != len) {
> +		memmove(array->v + pos + replacement_len, array->v + pos + len,
> +			(array->nr - pos - len + 1) * sizeof(char *));
> +		array->nr += (replacement_len - len);
> +	}
> +	for (size_t i = 0; i < replacement_len; i++)
> +		array->v[pos + i] = xstrdup(replacement[i]);
> +}
> +
>  const char *strvec_replace(struct strvec *array, size_t idx, const char *replacement)
>  {
>  	char *to_free;
> diff --git a/strvec.h b/strvec.h
> index 4b73c1f092e..4e61cc93369 100644
> --- a/strvec.h
> +++ b/strvec.h
> @@ -67,6 +67,15 @@ void strvec_pushl(struct strvec *, ...);
>  /* Push a null-terminated array of strings onto the end of the array. */
>  void strvec_pushv(struct strvec *, const char **);
>  
> +/*
> + * Replace `len` values starting at `pos` with the provided replacement
> + * strings. If `len` is zero this is effectively an insert at the given `pos`.
> + * If `replacement_len` is zero this is effectively a delete of `len` items
> + * starting at `pos`.
> + */
> +void strvec_splice(struct strvec *array, size_t pos, size_t len,
> +		   const char **replacement, size_t replacement_len);
> +
>  /**
>   * Replace the value at the given index with a new value. The index must be
>   * valid. Returns a pointer to the inserted value.
> diff --git a/t/unit-tests/strvec.c b/t/unit-tests/strvec.c
> index bf4c0cb172e..855b6023371 100644
> --- a/t/unit-tests/strvec.c
> +++ b/t/unit-tests/strvec.c
> @@ -88,6 +88,71 @@ void test_strvec__pushv(void)
>  	strvec_clear(&vec);
>  }
>  
> +void test_strvec__splice_with_same_size_replacement(void)
> +{
> +	struct strvec vec = STRVEC_INIT;
> +	const char *replacement[] = { "1" };
> +
> +	strvec_pushl(&vec, "foo", "bar", "baz", NULL);
> +	strvec_splice(&vec, 1, 1, replacement, ARRAY_SIZE(replacement));
> +	check_strvec(&vec, "foo", "1", "baz", NULL);
> +	strvec_clear(&vec);
> +}
> +
> +void test_strvec__splice_with_smaller_replacement(void)
> +{
> +	struct strvec vec = STRVEC_INIT;
> +	const char *replacement[] = { "1" };
> +
> +	strvec_pushl(&vec, "foo", "bar", "baz", NULL);
> +	strvec_splice(&vec, 1, 2, replacement, ARRAY_SIZE(replacement));
> +	check_strvec(&vec, "foo", "1", NULL);
> +	strvec_clear(&vec);
> +}
> +
> +void test_strvec__splice_with_bigger_replacement(void)
> +{
> +	struct strvec vec = STRVEC_INIT;
> +	const char *replacement[] = { "1", "2", "3" };
> +
> +	strvec_pushl(&vec, "foo", "bar", "baz", NULL);
> +	strvec_splice(&vec, 0, 2, replacement, ARRAY_SIZE(replacement));
> +	check_strvec(&vec, "1", "2", "3", "baz", NULL);
> +	strvec_clear(&vec);
> +}
> +
> +void test_strvec__splice_with_empty_replacement(void)
> +{
> +	struct strvec vec = STRVEC_INIT;
> +
> +	strvec_pushl(&vec, "foo", "bar", "baz", NULL);
> +	strvec_splice(&vec, 0, 2, NULL, 0);
> +	check_strvec(&vec, "baz", NULL);
> +	strvec_clear(&vec);
> +}
> +
> +void test_strvec__splice_with_empty_original(void)
> +{
> +	struct strvec vec = STRVEC_INIT;
> +	const char *replacement[] = { "1", "2" };
> +
> +	strvec_pushl(&vec, "foo", "bar", "baz", NULL);
> +	strvec_splice(&vec, 1, 0, replacement, ARRAY_SIZE(replacement));
> +	check_strvec(&vec, "foo", "1", "2", "bar", "baz", NULL);
> +	strvec_clear(&vec);
> +}
> +
> +void test_strvec__splice_at_tail(void)
> +{
> +	struct strvec vec = STRVEC_INIT;
> +	const char *replacement[] = { "1", "2" };
> +
> +	strvec_pushl(&vec, "foo", "bar", NULL);
> +	strvec_splice(&vec, 2, 0, replacement, ARRAY_SIZE(replacement));
> +	check_strvec(&vec, "foo", "bar", "1", "2", NULL);
> +	strvec_clear(&vec);
> +}
> +
>  void test_strvec__replace_at_head(void)
>  {
>  	struct strvec vec = STRVEC_INIT;
> -- 
> 2.47.0.229.g8f8d6eee53.dirty
>