[PATCH v3 3/8] reftable: check for trailing newline in 'tables.list'

From

Karthik Nayak <karthik.188@gmail.com>

Date

Sep 18, 2025, 08:11 UTC

Message-ID

<20250918-228-reftable-introduce-consistency-checks-v3-3-271af03eb34d@gmail.com>

In-Reply-To

<20250918-228-reftable-introduce-consistency-checks-v3-0-271af03eb34d@gmail.com>

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 reftable/basics.c                | 28 +++++++++++++++++++---------
 reftable/basics.h                |  7 ++++---
 reftable/stack.c                 |  6 ++----
 t/unit-tests/u-reftable-basics.c | 23 +++++++++++++++++++----
 4 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/reftable/basics.c b/reftable/basics.c
index 9988ebd635..75d4086769 100644
--- a/reftable/basics.c
+++ b/reftable/basics.c
@@ -195,7 +195,7 @@ size_t names_length(const char **names)
 	return p - names;
 }
 
-char **parse_names(char *buf, int size)
+char **parse_names(char *buf, int size, int *err)
 {
 	char **names = NULL;
 	size_t names_cap = 0;
@@ -205,30 +205,40 @@ char **parse_names(char *buf, int size)
 
 	while (p < end) {
 		char *next = strchr(p, '\n');
-		if (next && next < end) {
+		if (!next) {
+			*err = REFTABLE_FORMAT_ERROR;
+			goto done;
+		} else if (next < end) {
 			*next = 0;
 		} else {
 			next = end;
 		}
+
 		if (p < next) {
 			if (REFTABLE_ALLOC_GROW(names, names_len + 1,
-						names_cap))
-				goto err;
+						names_cap)) {
+				*err = REFTABLE_OUT_OF_MEMORY_ERROR;
+				goto done;
+			}
 
 			names[names_len] = reftable_strdup(p);
-			if (!names[names_len++])
-				goto err;
+			if (!names[names_len++]) {
+				*err = REFTABLE_OUT_OF_MEMORY_ERROR;
+				goto done;
+			}
 		}
 		p = next + 1;
 	}
 
-	if (REFTABLE_ALLOC_GROW(names, names_len + 1, names_cap))
-		goto err;
+	if (REFTABLE_ALLOC_GROW(names, names_len + 1, names_cap)) {
+		*err = REFTABLE_OUT_OF_MEMORY_ERROR;
+		goto done;
+	}
 	names[names_len] = NULL;
 
 	return names;
 
-err:
+done:
 	for (size_t i = 0; i < names_len; i++)
 		reftable_free(names[i]);
 	reftable_free(names);
diff --git a/reftable/basics.h b/reftable/basics.h
index 7d22f96261..019dfe6d7e 100644
--- a/reftable/basics.h
+++ b/reftable/basics.h
@@ -167,10 +167,11 @@ void free_names(char **a);
 
 /*
  * Parse a newline separated list of names. `size` is the length of the buffer,
- * without terminating '\0'. Empty names are discarded. Returns a `NULL`
- * pointer when allocations fail.
+ * without terminating '\0'. Empty names are discarded.
+ *
+ * Errors are assigned to the `err` variable.
  */
-char **parse_names(char *buf, int size);
+char **parse_names(char *buf, int size, int *err);
 
 /* compares two NULL-terminated arrays of strings. */
 int names_equal(const char **a, const char **b);
diff --git a/reftable/stack.c b/reftable/stack.c
index f91ce50bcd..955be1edb6 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -109,11 +109,9 @@ static int fd_read_lines(int fd, char ***namesp)
 	}
 	buf[size] = 0;
 
-	*namesp = parse_names(buf, size);
-	if (!*namesp) {
-		err = REFTABLE_OUT_OF_MEMORY_ERROR;
+	*namesp = parse_names(buf, size, &err);
+	if (!*namesp)
 		goto done;
-	}
 
 done:
 	reftable_free(buf);
diff --git a/t/unit-tests/u-reftable-basics.c b/t/unit-tests/u-reftable-basics.c
index a0471083e7..f77ec96429 100644
--- a/t/unit-tests/u-reftable-basics.c
+++ b/t/unit-tests/u-reftable-basics.c
@@ -9,6 +9,7 @@ license that can be found in the LICENSE file or at
 #include "unit-test.h"
 #include "lib-reftable.h"
 #include "reftable/basics.h"
+#include "reftable/reftable-error.h"
 
 struct integer_needle_lesseq_args {
 	int needle;
@@ -79,14 +80,17 @@ void test_reftable_basics__names_equal(void)
 void test_reftable_basics__parse_names(void)
 {
 	char in1[] = "line\n";
-	char in2[] = "a\nb\nc";
-	char **out = parse_names(in1, strlen(in1));
+	char in2[] = "a\nb\nc\n";
+	int err = 0;
+	char **out = parse_names(in1, strlen(in1), &err);
+	cl_assert(err == 0);
 	cl_assert(out != NULL);
 	cl_assert_equal_s(out[0], "line");
 	cl_assert(!out[1]);
 	free_names(out);
 
-	out = parse_names(in2, strlen(in2));
+	out = parse_names(in2, strlen(in2), &err);
+	cl_assert(err == 0);
 	cl_assert(out != NULL);
 	cl_assert_equal_s(out[0], "a");
 	cl_assert_equal_s(out[1], "b");
@@ -95,10 +99,21 @@ void test_reftable_basics__parse_names(void)
 	free_names(out);
 }
 
+void test_reftable_basics__parse_names_missing_newline(void)
+{
+	char in1[] = "line\nline2";
+	int err = 0;
+	char **out = parse_names(in1, strlen(in1), &err);
+	cl_assert(err == REFTABLE_FORMAT_ERROR);
+	cl_assert(out == NULL);
+}
+
 void test_reftable_basics__parse_names_drop_empty_string(void)
 {
 	char in[] = "a\n\nb\n";
-	char **out = parse_names(in, strlen(in));
+	int err = 0;
+	char **out = parse_names(in, strlen(in), &err);
+	cl_assert(err ==  0);
 	cl_assert(out != NULL);
 	cl_assert_equal_s(out[0], "a");
 	/* simply '\n' should be dropped as empty string */

-- 
2.51.0