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

From: Rubén Justo <rjusto@gmail.com>
Date: Nov 29, 2024, 17:23 UTC
Message-ID: <37d0abbf-c703-481d-9f26-b237aac54c05@gmail.com>

Let's avoid an invalid pointer error in case a client of `strvec_splice()` ends up with something similar to:

       struct strvec arr = STRVEC_INIT;
       const char *rep[] = { "foo" };

       strvec_splice(&arr, 0, 0, rep, ARRAY_SIZE(rep));

Signed-off-by: Rubén Justo <rjusto@gmail.com>
---

I've had some time to review the new iteration of the series where `strvec_splice()` was introduced and perhaps we want to consider cases where we end up using `strvec_splice()` with a statically initialized `struct strvec`, i.e:

       struct strvec value = STRVEC_INIT;
       int s = 0, e = 0;

       ... nothing added to `value` and "s == e == 0" ...

       const char *rep[] = { "foo" };
       strvec_splice(&arr, s, e, rep, ARRAY_SIZE(rep));

       ... realloc(): invalid pointer

Sorry for getting back to this so late. This slipped through in my review.

I know the series is already in `next`. To avoid adding noise to the series I'm not responding to the conversation, but here is a link to it:

  https://lore.kernel.org/git/20241120-b4-pks-leak-fixes-pt10-v3-0-d67f08f45c74@pks.im/

 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..64750e35e3 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);
 	for (size_t i = 0; i < replacement_len; i++)
 		array->v[idx + i] = xstrdup(replacement[i]);
 }
diff --git a/t/unit-tests/strvec.c b/t/unit-tests/strvec.c
index 855b602337..e66b7bbfae 100644
--- a/t/unit-tests/strvec.c
+++ b/t/unit-tests/strvec.c
@@ -88,6 +88,16 @@ void test_strvec__pushv(void)
 	strvec_clear(&vec);
 }
 
+void test_strvec__splice_just_initialized_strvec(void)
+{
+	struct strvec vec = STRVEC_INIT;
+	const char *replacement[] = { "foo" };
+
+	strvec_splice(&vec, 0, 0, replacement, ARRAY_SIZE(replacement));
+	check_strvec(&vec, "foo", NULL);
+	strvec_clear(&vec);
+}
+
 void test_strvec__splice_with_same_size_replacement(void)
 {
 	struct strvec vec = STRVEC_INIT;

-- 
2.47.0.280.geb6a512a19