reftable: check for trailing newline in 'tables.list'

In the reftable format, the 'tables.list' file contains a
newline separated list of tables. While we parse this file, we do not
check or care about the last newline. Tighten the parser in
`parse_names()` to return an appropriate error if the last newline is
missing.

This requires modification to `parse_names()` to now return the error
while accepting the output as a third argument.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/reftable/basics.c b/reftable/basics.c
index 9988ebd635ebee756543238e7f08e970c964f89c..e969927b615fc193ef3368acc922fcb414bfdc04 100644 (file)
--- a/reftable/basics.c
+++ b/reftable/basics.c
@@ -195,44 +195,55 @@ size_t names_length(const char **names)
        return p - names;
 }
 
-char **parse_names(char *buf, int size)
+int parse_names(char *buf, int size, char ***out)
 {
        char **names = NULL;
        size_t names_cap = 0;
        size_t names_len = 0;
        char *p = buf;
        char *end = buf + size;
+       int err = 0;
 
        while (p < end) {
                char *next = strchr(p, '\n');
-               if (next && next < end) {
-                       *next = 0;
+               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:
+       *out = names;
+       return 0;
+done:
        for (size_t i = 0; i < names_len; i++)
                reftable_free(names[i]);
        reftable_free(names);
-       return NULL;
+       return err;
 }
 
 int names_equal(const char **a, const char **b)

diff --git a/reftable/basics.h b/reftable/basics.h
index 7d22f96261072ac8cb1b64e8d86a9c2feeb6afdd..e4b83b2b03ff8a8d5e9c34d97c1a6c5b39f7a8ed 100644 (file)
--- 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.
+ *
+ * Returns 0 on success, a reftable error code on error.
  */
-char **parse_names(char *buf, int size);
+int parse_names(char *buf, int size, char ***out);
 
 /* 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 4caf96aa1d69614284c6ccadebe0a148d9b20a6d..7df872d0fbb3acb835f47cda84bb23450d430b54 100644 (file)
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -169,12 +169,7 @@ 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;
-               goto done;
-       }
-
+       err = parse_names(buf, size, namesp);
 done:
        reftable_free(buf);
        return err;

diff --git a/t/unit-tests/u-reftable-basics.c b/t/unit-tests/u-reftable-basics.c
index a0471083e7336afabf2831ae46bd850c6323b776..73566ed0eb7e0731bd93d117f7fc30b19254d854 100644 (file)
--- a/t/unit-tests/u-reftable-basics.c
+++ b/t/unit-tests/u-reftable-basics.c
@@ -9,6 +9,7 @@ https://developers.google.com/open-source/licenses/bsd
 #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,18 @@ 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";
+       char **out = NULL;
+       int err = parse_names(in1, strlen(in1), &out);
+       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 = NULL;
+       err = parse_names(in2, strlen(in2), &out);
+       cl_assert(err == 0);
        cl_assert(out != NULL);
        cl_assert_equal_s(out[0], "a");
        cl_assert_equal_s(out[1], "b");
@@ -95,10 +100,21 @@ void test_reftable_basics__parse_names(void)
        free_names(out);
 }
 
+void test_reftable_basics__parse_names_missing_newline(void)
+{
+       char in1[] = "line\nline2";
+       char **out = NULL;
+       int err = parse_names(in1, strlen(in1), &out);
+       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));
+       char **out = NULL;
+       int err = parse_names(in, strlen(in), &out);
+       cl_assert(err == 0);
        cl_assert(out != NULL);
        cl_assert_equal_s(out[0], "a");
        /* simply '\n' should be dropped as empty string */