{"thread":{"id":"64212","subject":"[PATCH v4 1/7] refs: remove unused headers","startedAt":"2025-09-26T07:25:56Z","lastAt":"2025-10-06T12:05:27Z","messageCount":14,"participants":["Karthik Nayak","Junio C Hamano","Patrick Steinhardt"],"isPatch":true,"patchVersion":4,"patchTotal":7},"messages":[{"id":"527395","messageId":"20250926-228-reftable-introduce-consistency-checks-v4-1-c96fd8551c0d@gmail.com","threadId":"64212","inReplyTo":"20250926-228-reftable-introduce-consistency-checks-v4-0-c96fd8551c0d@gmail.com","subject":"[PATCH v4 1/7] refs: remove unused headers","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-09-26T07:25:44Z","receivedAt":"2025-09-26T07:25:56Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"In the 'refs/' namespace, some of the included header files are not\nneeded, let's remove them.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n refs/debug.c            | 1 -\n refs/files-backend.c    | 1 -\n refs/reftable-backend.c | 1 -\n 3 files changed, 3 deletions(-)\n\ndiff --git a/refs/debug.c b/refs/debug.c\nindex 1cb955961e..697adbd0dc 100644\n--- a/refs/debug.c\n+++ b/refs/debug.c\n@@ -1,7 +1,6 @@\n #include \"git-compat-util.h\"\n #include \"hex.h\"\n #include \"refs-internal.h\"\n-#include \"string-list.h\"\n #include \"trace.h\"\n \n static struct trace_key trace_refs = TRACE_KEY_INIT(REFS);\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 1b3bf26add..d4fb033417 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -20,7 +20,6 @@\n #include \"../dir-iterator.h\"\n #include \"../lockfile.h\"\n #include \"../object.h\"\n-#include \"../object-file.h\"\n #include \"../path.h\"\n #include \"../dir.h\"\n #include \"../chdir-notify.h\"\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex 9e889da2ff..2152349cb9 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -11,7 +11,6 @@\n #include \"../hex.h\"\n #include \"../iterator.h\"\n #include \"../ident.h\"\n-#include \"../lockfile.h\"\n #include \"../object.h\"\n #include \"../path.h\"\n #include \"../refs.h\"\n\n-- \n2.51.0\n\n"},{"id":"527397","messageId":"20250926-228-reftable-introduce-consistency-checks-v4-0-c96fd8551c0d@gmail.com","threadId":"64212","inReplyTo":"20250819-228-reftable-introduce-consistency-checks-v1-0-8b8f6879fa9e@gmail.com","subject":"[PATCH v4 0/7] refs/reftable: add consistency checks","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-09-26T07:25:43Z","receivedAt":"2025-09-26T07:25:56Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The reference subsystems allows for adding backend specific consistency\nchecks. These checks are run as part of 'git refs verify'.\n\nWhile the files backend has some consistency checks added, the reftable\nbackend currently has none. This series first tightens the reftable\nbackend to make it a little more strict and then also adds the required\ninfrastructure and some simple consistency checks.\n\nSince the reftable backend is treated as a library within the Git\ncodebase, we don't want to spillover our internal fsck implementation\ninto the library. At the same time, the fsck checks need to access\ninternal structures of the reftable library which aren't exposed outside\nthe library.\n\nSo we solve this by adding a 'reftable/fsck.[ch]' which implements and\nexposes a checker for the reftable library and returns specific errors\nas defined by the library. We then add glue code within\n'refs/reftable-backend.c' to map these errors to errors which Git's fsck\nimplementation would understand. This allows us to separate concerns.\n\nWe add the following consistency checks:\n\n  1. Check for validating the reftable table name. This is treated as a\n  warning since the reftable specification only suggests a table name\n  but doesn't enforce it. Also there is a difference in the table name\n  used in Git vs that in jGit.\n\nWe tighten the reftable backend by raising a REFTABLE_FORMAT_ERROR error\nwhen:\n\n1. The 'tables.list' file doesn't have a trailing newline.\n\n---\nChanges in v4:\n- The biggest change is to iterate over the tables in a reftable stack\n  for consistency checks instead of all files inside the REFTABLE_DIR.\n  This avoids all race conditions. Also, since we only check the tables\n  in a stack, it no longer makes sense to check file type.\n- The discussion about update indices was concluded that tables indices\n  in a stack must be strictly monotonically increasing. While modifying\n  the code to do the same. I realized that we already have this check in\n  'reftable_addition_add()' where we check while adding a new table to\n  the stack: `wr->min_update_index < add->next_update_index`. So I've\n  dropped this patch from the series.\n- Change parse_names() to accept the output string array as an argument\n  and return an error instead. This makes the flow a little easier to\n  understand.\n- Link to v3: https://lore.kernel.org/r/20250918-228-reftable-introduce-consistency-checks-v3-0-271af03eb34d@gmail.com\n\nChanges in v3:\n- I took a long hiatus from this topic, mostly due to other priorities.\n  This has been rebased on top of '92c87bdc40 (The eighth batch,\n  2025-09-12)' since there were conflicts.\n- Junio suggested that two of the consistency checks (trailing newlines,\n  sequential update indices for tables in stack) should actually be\n  checked during runtime. I have made that change in this version.\n- I've cleaned up the code and modularized the 'reftable/fsck.c' code.\n- Invalid table name emits a warning, since the reftable spec doesn't\n  enforce it but only makes a suggestion.\n- Broken down the commits to make it easier to review.\n- Link to v2: https://lore.kernel.org/r/20250902-228-reftable-introduce-consistency-checks-v2-0-4f96b3834779@gmail.com\n\nChanges in v2:\n- Ensured that 'struct reftable_fsck_info' is passed around as a\n  pointer, this provides a smaller footprint (pointer size vs struct\n  size).\n- Run FSCK checks for other worktrees too, even if one of them fails.\n- Separate messaging for table name vs table check and add additional\n  test.\n- Use the relative path in messages used.\n- Small style and typo fixes.\n- Link to v1: https://lore.kernel.org/r/20250819-228-reftable-introduce-consistency-checks-v1-0-8b8f6879fa9e@gmail.com\n\n---\n Documentation/fsck-msgids.adoc   |   6 +--\n Makefile                         |   3 +-\n fsck.h                           |  39 +++++++--------\n meson.build                      |   1 +\n refs.c                           |   4 ++\n refs/debug.c                     |   1 -\n refs/files-backend.c             |   3 --\n refs/reftable-backend.c          |  58 ++++++++++++++++++++---\n reftable/basics.c                |  37 ++++++++++-----\n reftable/basics.h                |   5 +-\n reftable/fsck.c                  | 100 +++++++++++++++++++++++++++++++++++++++\n reftable/reftable-fsck.h         |  40 ++++++++++++++++\n reftable/stack.c                 |   7 +--\n t/meson.build                    |   1 +\n t/t0614-reftable-fsck.sh         |  38 +++++++++++++++\n t/unit-tests/u-reftable-basics.c |  24 ++++++++--\n 16 files changed, 308 insertions(+), 59 deletions(-)\n\nKarthik Nayak (7):\n      refs: remove unused headers\n      refs: move consistency check  msg to generic layer\n      reftable: check for trailing newline in 'tables.list'\n      Documentation/fsck-msgids: remove duplicate msg id\n      fsck: order 'fsck_msg_type' alphabetically\n      reftable: add code to facilitate consistency checks\n      refs/reftable: add fsck check for checking the table name\n\nRange-diff versus v3:\n\n1:  4522c10e6e = 1:  b91194e060 refs: remove unused headers\n2:  40a83fc6fa = 2:  d48afbf588 refs: move consistency check  msg to generic layer\n3:  df401e46f7 ! 3:  cd7ca2a585 reftable: check for trailing newline in 'tables.list'\n    @@ Metadata\n      ## Commit message ##\n         reftable: check for trailing newline in 'tables.list'\n     \n    -    In the reftable format, the 'tables.list' file contains a newline\n    -    separated list of tables. While we parse this file, we do not check or\n    -    care about trailing newlines. Tighten the parser in `parse_names()` to\n    -    return an appropriate error if there is no trailing newline.\n    +    In the reftable format, the 'tables.list' file contains a\n    +    newline separated list of tables. While we parse this file, we do not\n    +    check or care about the last newline. Tighten the parser in\n    +    `parse_names()` to return an appropriate error if the last newline is\n    +    missing.\n     \n    -    This requires modification to `parse_names()` to accept a third argument\n    -    which will hold the error value.\n    +    This requires modification to `parse_names()` to now return the error\n    +    while accepting the output as a third argument.\n     \n         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n     \n    @@ reftable/basics.c: size_t names_length(const char **names)\n      }\n      \n     -char **parse_names(char *buf, int size)\n    -+char **parse_names(char *buf, int size, int *err)\n    ++int parse_names(char *buf, int size, char ***out)\n      {\n      \tchar **names = NULL;\n      \tsize_t names_cap = 0;\n    -@@ reftable/basics.c: char **parse_names(char *buf, int size)\n    + \tsize_t names_len = 0;\n    + \tchar *p = buf;\n    + \tchar *end = buf + size;\n    ++\tint err = 0;\n      \n      \twhile (p < end) {\n      \t\tchar *next = strchr(p, '\\n');\n     -\t\tif (next && next < end) {\n    +-\t\t\t*next = 0;\n     +\t\tif (!next) {\n    -+\t\t\t*err = REFTABLE_FORMAT_ERROR;\n    ++\t\t\terr = REFTABLE_FORMAT_ERROR;\n     +\t\t\tgoto done;\n     +\t\t} else if (next < end) {\n    - \t\t\t*next = 0;\n    ++\t\t\t*next = '\\0';\n      \t\t} else {\n      \t\t\tnext = end;\n      \t\t}\n    @@ reftable/basics.c: char **parse_names(char *buf, int size)\n     -\t\t\t\t\t\tnames_cap))\n     -\t\t\t\tgoto err;\n     +\t\t\t\t\t\tnames_cap)) {\n    -+\t\t\t\t*err = REFTABLE_OUT_OF_MEMORY_ERROR;\n    ++\t\t\t\terr = REFTABLE_OUT_OF_MEMORY_ERROR;\n     +\t\t\t\tgoto done;\n     +\t\t\t}\n      \n    @@ reftable/basics.c: char **parse_names(char *buf, int size)\n     -\t\t\tif (!names[names_len++])\n     -\t\t\t\tgoto err;\n     +\t\t\tif (!names[names_len++]) {\n    -+\t\t\t\t*err = REFTABLE_OUT_OF_MEMORY_ERROR;\n    ++\t\t\t\terr = REFTABLE_OUT_OF_MEMORY_ERROR;\n     +\t\t\t\tgoto done;\n     +\t\t\t}\n      \t\t}\n    @@ reftable/basics.c: char **parse_names(char *buf, int size)\n     -\tif (REFTABLE_ALLOC_GROW(names, names_len + 1, names_cap))\n     -\t\tgoto err;\n     +\tif (REFTABLE_ALLOC_GROW(names, names_len + 1, names_cap)) {\n    -+\t\t*err = REFTABLE_OUT_OF_MEMORY_ERROR;\n    ++\t\terr = REFTABLE_OUT_OF_MEMORY_ERROR;\n     +\t\tgoto done;\n     +\t}\n      \tnames[names_len] = NULL;\n      \n    - \treturn names;\n    - \n    +-\treturn names;\n    +-\n     -err:\n    ++\t*out = names;\n    ++\treturn 0;\n     +done:\n      \tfor (size_t i = 0; i < names_len; i++)\n      \t\treftable_free(names[i]);\n      \treftable_free(names);\n    +-\treturn NULL;\n    ++\treturn err;\n    + }\n    + \n    + int names_equal(const char **a, const char **b)\n     \n      ## reftable/basics.h ##\n     @@ reftable/basics.h: void free_names(char **a);\n    @@ reftable/basics.h: void free_names(char **a);\n     - * without terminating '\\0'. Empty names are discarded. Returns a `NULL`\n     - * pointer when allocations fail.\n     + * without terminating '\\0'. Empty names are discarded.\n    -+ *\n    -+ * Errors are assigned to the `err` variable.\n       */\n     -char **parse_names(char *buf, int size);\n    -+char **parse_names(char *buf, int size, int *err);\n    ++int parse_names(char *buf, int size, char ***out);\n      \n      /* compares two NULL-terminated arrays of strings. */\n      int names_equal(const char **a, const char **b);\n    @@ reftable/stack.c: static int fd_read_lines(int fd, char ***namesp)\n     -\t*namesp = parse_names(buf, size);\n     -\tif (!*namesp) {\n     -\t\terr = REFTABLE_OUT_OF_MEMORY_ERROR;\n    -+\t*namesp = parse_names(buf, size, &err);\n    -+\tif (!*namesp)\n    - \t\tgoto done;\n    +-\t\tgoto done;\n     -\t}\n    - \n    +-\n    ++\terr = parse_names(buf, size, namesp);\n      done:\n      \treftable_free(buf);\n    + \treturn err;\n     \n      ## t/unit-tests/u-reftable-basics.c ##\n     @@ t/unit-tests/u-reftable-basics.c: license that can be found in the LICENSE file or at\n    @@ t/unit-tests/u-reftable-basics.c: void test_reftable_basics__names_equal(void)\n     -\tchar in2[] = \"a\\nb\\nc\";\n     -\tchar **out = parse_names(in1, strlen(in1));\n     +\tchar in2[] = \"a\\nb\\nc\\n\";\n    -+\tint err = 0;\n    -+\tchar **out = parse_names(in1, strlen(in1), &err);\n    ++\tchar **out = NULL;\n    ++\tint err = parse_names(in1, strlen(in1), &out);\n     +\tcl_assert(err == 0);\n      \tcl_assert(out != NULL);\n      \tcl_assert_equal_s(out[0], \"line\");\n    @@ t/unit-tests/u-reftable-basics.c: void test_reftable_basics__names_equal(void)\n      \tfree_names(out);\n      \n     -\tout = parse_names(in2, strlen(in2));\n    -+\tout = parse_names(in2, strlen(in2), &err);\n    ++\tout = NULL;\n    ++\terr = parse_names(in2, strlen(in2), &out);\n     +\tcl_assert(err == 0);\n      \tcl_assert(out != NULL);\n      \tcl_assert_equal_s(out[0], \"a\");\n    @@ t/unit-tests/u-reftable-basics.c: void test_reftable_basics__parse_names(void)\n     +void test_reftable_basics__parse_names_missing_newline(void)\n     +{\n     +\tchar in1[] = \"line\\nline2\";\n    -+\tint err = 0;\n    -+\tchar **out = parse_names(in1, strlen(in1), &err);\n    ++\tchar **out = NULL;\n    ++\tint err = parse_names(in1, strlen(in1), &out);\n     +\tcl_assert(err == REFTABLE_FORMAT_ERROR);\n     +\tcl_assert(out == NULL);\n     +}\n    @@ t/unit-tests/u-reftable-basics.c: void test_reftable_basics__parse_names(void)\n      {\n      \tchar in[] = \"a\\n\\nb\\n\";\n     -\tchar **out = parse_names(in, strlen(in));\n    -+\tint err = 0;\n    -+\tchar **out = parse_names(in, strlen(in), &err);\n    -+\tcl_assert(err ==  0);\n    ++\tchar **out = NULL;\n    ++\tint err = parse_names(in, strlen(in), &out);\n    ++\tcl_assert(err == 0);\n      \tcl_assert(out != NULL);\n      \tcl_assert_equal_s(out[0], \"a\");\n      \t/* simply '\\n' should be dropped as empty string */\n4:  435707f26c < -:  ---------- reftable: ensure tables in a stack use sequential update indices\n5:  ac6275ab87 = 4:  e3e0c0b4ae Documentation/fsck-msgids: remove duplicate msg id\n6:  6c02925af1 = 5:  24a8d93adc fsck: order 'fsck_msg_type' alphabetically\n7:  1ada7bc89c ! 6:  d83d763be1 reftable: add code to facilitate consistency checks\n    @@ Commit message\n         stack. The callee provides the function with callbacks to handle issue\n         and information reporting.\n     \n    -    The added check, goes over all files in the reftable directory and\n    -    validates that they have the expected file type and a valid name. It\n    -    raises specific errors for both.\n    +    The added check, goes over all tables in the reftable stack validates\n    +    that they have a valid name. It not, it raises an error.\n     \n         While here, move 'reftable/error.o' in the Makefile to retain\n         lexicographic ordering.\n    @@ reftable/fsck.c (new)\n     @@\n     +#include \"basics.h\"\n     +#include \"reftable-fsck.h\"\n    ++#include \"reftable-table.h\"\n     +#include \"stack.h\"\n     +\n    -+static bool valid_table_name(const char *name, uint64_t *min_update_index,\n    -+\t\t\t     uint64_t *max_update_index)\n    ++static bool table_has_valid_name(const char *name)\n     +{\n     +\tconst char *ptr = name;\n     +\tchar *endptr;\n    @@ reftable/fsck.c (new)\n     +\t/* strtoull doesn't set errno on success */\n     +\terrno = 0;\n     +\n    -+\t*min_update_index = strtoull(ptr, &endptr, 16);\n    -+\tif (errno == EINVAL)\n    ++\tstrtoull(ptr, &endptr, 16);\n    ++\tif (errno)\n     +\t\treturn false;\n     +\tptr = endptr;\n     +\n    -+\tif (strncmp(ptr, \"-\", 1))\n    ++\tif (*ptr != '-')\n     +\t\treturn false;\n     +\tptr++;\n     +\n    -+\t*max_update_index = strtoull(ptr, &endptr, 16);\n    -+\tif (errno == EINVAL)\n    ++\tstrtoull(ptr, &endptr, 16);\n    ++\tif (errno)\n     +\t\treturn false;\n     +\tptr = endptr;\n     +\n    @@ reftable/fsck.c (new)\n     +\tptr++;\n     +\n     +\tstrtoul(ptr, &endptr, 16);\n    -+\tif (errno == EINVAL)\n    ++\tif (errno)\n     +\t\treturn false;\n     +\tptr = endptr;\n     +\n    @@ reftable/fsck.c (new)\n     +\treturn true;\n     +}\n     +\n    -+static int stack_check_all_files_in_dir(struct reftable_stack *stack,\n    -+\t\t\t\t\treftable_fsck_report_fn report_fn,\n    -+\t\t\t\t\tvoid *cb_data)\n    ++typedef int (*table_check_fn)(struct reftable_table *table,\n    ++\t\t\t      reftable_fsck_report_fn report_fn,\n    ++\t\t\t      void *cb_data);\n    ++\n    ++static int table_check_name(struct reftable_table *table,\n    ++\t\t\t    reftable_fsck_report_fn report_fn,\n    ++\t\t\t    void *cb_data)\n     +{\n    -+\tDIR *dir = opendir(stack->reftable_dir);\n    -+\tstruct reftable_fsck_info info;\n    -+\tstruct dirent *d = NULL;\n    -+\tuint64_t min, max;\n    -+\tint err = 0;\n    ++\tif (!table_has_valid_name(table->name)) {\n    ++\t\tstruct reftable_fsck_info info;\n    ++\n    ++\t\tinfo.error = REFTABLE_FSCK_ERROR_TABLE_NAME;\n    ++\t\tinfo.msg = \"invalid reftable table name\";\n    ++\t\tinfo.path = table->name;\n     +\n    -+\tif (!dir)\n    -+\t\treturn 0;\n    -+\n    -+\twhile ((d = readdir(dir))) {\n    -+\t\tif (!strcmp(d->d_name, \"tables.list\"))\n    -+\t\t\tcontinue;\n    -+\n    -+\t\tif ((d->d_name[0] == '.' &&\n    -+\t\t     (d->d_name[1] == '\\0' ||\n    -+\t\t      (d->d_name[1] == '.' && d->d_name[2] == '\\0'))))\n    -+\t\t\tcontinue;\n    -+\n    -+\t\tif (d->d_type == DT_REG) {\n    -+\t\t\tif (!valid_table_name(d->d_name, &min, &max)) {\n    -+\t\t\t\tinfo.error = REFTABLE_FSCK_ERROR_TABLE_NAME;\n    -+\t\t\t\tinfo.msg = \"file with invalid table name\";\n    -+\t\t\t\tinfo.path = d->d_name;\n    -+\n    -+\t\t\t\terr |= report_fn(&info, cb_data);\n    -+\t\t\t}\n    -+\t\t} else {\n    -+\t\t\tinfo.error = REFTABLE_FSCK_ERROR_INVALID_FILE_TYPE;\n    -+\t\t\tinfo.msg = \"file with unexpected type\";\n    -+\t\t\tinfo.path = d->d_name;\n    -+\n    -+\t\t\terr |= report_fn(&info, cb_data);\n    -+\t\t}\n    ++\t\treturn report_fn(&info, cb_data);\n     +\t}\n     +\n    -+\tclosedir(dir);\n    -+\treturn err;\n    ++\treturn 0;\n     +}\n     +\n    -+static int stack_checks(struct reftable_stack *stack,\n    ++static int table_checks(struct reftable_table *table,\n     +\t\t\treftable_fsck_report_fn report_fn,\n    ++\t\t\treftable_fsck_verbose_fn verbose_fn UNUSED,\n     +\t\t\tvoid *cb_data)\n     +{\n    -+\tstruct reftable_buf msg = REFTABLE_BUF_INIT;\n    -+\tchar **names = NULL;\n    ++\ttable_check_fn table_check_fns[] = {\n    ++\t\ttable_check_name,\n    ++\t\tNULL,\n    ++\t};\n     +\tint err = 0;\n     +\n    -+\tif (stack == NULL)\n    -+\t\tgoto out;\n    ++\tfor (size_t i = 0; table_check_fns[i]; i++)\n    ++\t\terr |= table_check_fns[i](table, report_fn, cb_data);\n     +\n    -+\terr |= stack_check_all_files_in_dir(stack, report_fn, cb_data);\n    -+\n    -+out:\n    -+\tfree_names(names);\n    -+\treftable_buf_release(&msg);\n     +\treturn err;\n     +}\n     +\n    @@ reftable/fsck.c (new)\n     +\t\t\treftable_fsck_verbose_fn verbose_fn,\n     +\t\t\tvoid *cb_data)\n     +{\n    -+\tverbose_fn(\"Checking reftable: stack checks\", cb_data);\n    -+\treturn stack_checks(stack, report_fn, cb_data);\n    ++\tstruct reftable_buf msg = REFTABLE_BUF_INIT;\n    ++\tint err = 0;\n    ++\n    ++\tfor (size_t i = 0; i < stack->tables_len; i++) {\n    ++\t\treftable_buf_reset(&msg);\n    ++\t\treftable_buf_addstr(&msg, \"Checking table: \");\n    ++\t\treftable_buf_addstr(&msg, stack->tables[i]->name);\n    ++\t\tverbose_fn(msg.buf, cb_data);\n    ++\n    ++\t\terr |= table_checks(stack->tables[i], report_fn, verbose_fn, cb_data);\n    ++\t}\n    ++\n    ++\treftable_buf_release(&msg);\n    ++\treturn err;\n     +}\n     \n      ## reftable/reftable-fsck.h (new) ##\n    @@ reftable/reftable-fsck.h (new)\n     +#include \"reftable-stack.h\"\n     +\n     +enum reftable_fsck_error {\n    -+\t/* Non regular file in the reftable directory */\n    -+\tREFTABLE_FSCK_ERROR_INVALID_FILE_TYPE = 0,\n     +\t/* Invalid table name */\n    -+\tREFTABLE_FSCK_ERROR_TABLE_NAME,\n    ++\tREFTABLE_FSCK_ERROR_TABLE_NAME = 0,\n     +\t/* Used for bounds checking, must be last */\n    -+\tREFTABLE_FSCK_MAX_VALUE\n    ++\tREFTABLE_FSCK_MAX_VALUE,\n     +};\n     +\n     +/* Represents an individual error encountered during the FSCK checks. */\n8:  77be84e23f ! 7:  c49b7887d8 refs/reftable: add fsck check for checking the table name\n    @@ Commit message\n           ${min_update_index}-${max_update_index}-${random}.ref as a naming\n           convention.\n     \n    -    So treat non-conformant file names as warnings. Introduce another check\n    -    to check for file types, non-expected filetypes will be treated as\n    -    errors.\n    +    So treat non-conformant file names as warnings.\n    +\n    +    While adding the fsck header to 'refs/reftable-backend.c', modify the\n    +    list to maintain lexicographical ordering.\n     \n         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n     \n    @@ Documentation/fsck-msgids.adoc\n      `badReferentName`::\n      \t(ERROR) The referent name of a symref is invalid.\n      \n    -+`badReftableFiletype`::\n    -+\t(ERROR) File with unexpected type in reftable directory.\n    -+\n     +`badReftableTableName`::\n     +\t(WARN) A reftable table has an invalid name.\n     +\n    @@ Documentation/fsck-msgids.adoc\n      \n     \n      ## fsck.h ##\n    -@@ fsck.h: enum fsck_msg_type {\n    - \tFUNC(BAD_PACKED_REF_HEADER, ERROR) \\\n    - \tFUNC(BAD_PARENT_SHA1, ERROR) \\\n    - \tFUNC(BAD_REFERENT_NAME, ERROR) \\\n    -+\tFUNC(BAD_REFTABLE_FILETYPE, ERROR) \\\n    - \tFUNC(BAD_REF_CONTENT, ERROR) \\\n    - \tFUNC(BAD_REF_FILETYPE, ERROR) \\\n    - \tFUNC(BAD_REF_NAME, ERROR) \\\n     @@ fsck.h: enum fsck_msg_type {\n      \tFUNC(UNKNOWN_TYPE, ERROR) \\\n      \tFUNC(ZERO_PADDED_DATE, ERROR) \\\n    @@ refs/reftable-backend.c: static int reftable_be_reflog_expire(struct ref_store *\n     +}\n     +\n     +static const enum fsck_msg_id fsck_msg_id_map[] = {\n    -+\t[REFTABLE_FSCK_ERROR_INVALID_FILE_TYPE] = FSCK_MSG_BAD_REFTABLE_FILETYPE,\n     +\t[REFTABLE_FSCK_ERROR_TABLE_NAME] = FSCK_MSG_BAD_REFTABLE_TABLE_NAME,\n     +};\n     +\n    @@ refs/reftable-backend.c: static int reftable_be_reflog_expire(struct ref_store *\n     +\tenum fsck_msg_id msg_id;\n     +\n     +\tif (info->error < 0 || info->error >= REFTABLE_FSCK_MAX_VALUE)\n    -+\t\tBUG(\"unknown fsck error: %d\", info->error);\n    ++\t\tBUG(\"unknown fsck error: %d\", (int)info->error);\n     +\n     +\tmsg_id = fsck_msg_id_map[info->error];\n     +\n     +\tif (!msg_id)\n    -+\t\tBUG(\"fsck_msg_id value missing for reftable error: %d\", info->error);\n    ++\t\tBUG(\"fsck_msg_id value missing for reftable error: %d\", (int)info->error);\n     +\n     +\treturn fsck_report_ref(o, &report, msg_id, \"%s\", info->msg);\n     +}\n    @@ t/t0614-reftable-fsck.sh (new)\n     +\t\t\tgit refs verify 2>err &&\n     +\t\t\ttest_must_be_empty err &&\n     +\n    -+\t\t\ttouch \".git/reftable/$TABLE_NAME\" &&\n    ++\t\t\tEXISTING_TABLE=$(head -n1 .git/reftable/tables.list) &&\n    ++\t\t\tmv \".git/reftable/$EXISTING_TABLE\" \".git/reftable/$TABLE_NAME\" &&\n    ++\t\t\tsed \"s/${EXISTING_TABLE}/${TABLE_NAME}/g\" .git/reftable/tables.list > tables.list &&\n    ++\t\t\tmv tables.list .git/reftable/tables.list &&\n     +\n     +\t\t\tgit refs verify 2>err &&\n     +\t\t\tcat >expect <<-EOF &&\n    -+\t\t\twarning: ${TABLE_NAME}: badReftableTableName: file with invalid table name\n    ++\t\t\twarning: ${TABLE_NAME}: badReftableTableName: invalid reftable table name\n     +\t\t\tEOF\n     +\t\t\ttest_cmp expect err\n     +\t\t)\n     +\t'\n     +done\n     +\n    -+test_expect_success \"invalid file type should be checked\" '\n    -+\ttest_when_finished \"rm -rf repo\" &&\n    -+\tgit init repo &&\n    -+\t(\n    -+\t\tcd repo &&\n    -+\t\tgit commit --allow-empty -m initial &&\n    -+\n    -+\t\tgit refs verify 2>err &&\n    -+\t\ttest_must_be_empty err &&\n    -+\n    -+\t\tmkdir \".git/reftable/foo\" &&\n    -+\n    -+\t\ttest_must_fail git refs verify 2>err &&\n    -+\t\tcat >expect <<-EOF &&\n    -+\t\terror: foo: badReftableFiletype: file with unexpected type\n    -+\t\tEOF\n    -+\t\ttest_cmp expect err\n    -+\t)\n    -+'\n    -+\n     +test_done\n\n\nbase-commit: a483264b01b977f3e65a4419103c21e6af7412a2\nchange-id: 20250714-228-reftable-introduce-consistency-checks-379ded93c544\n\nThanks\n- Karthik\n\n"},{"id":"527396","messageId":"20250926-228-reftable-introduce-consistency-checks-v4-2-c96fd8551c0d@gmail.com","threadId":"64212","inReplyTo":"20250926-228-reftable-introduce-consistency-checks-v4-0-c96fd8551c0d@gmail.com","subject":"[PATCH v4 2/7] refs: move consistency check msg to generic layer","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-09-26T07:25:45Z","receivedAt":"2025-09-26T07:25:57Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The files-backend prints a message before the consistency checks run.\nMove this to the generic layer so both the files and reftable backend\ncan benefit from this message.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n refs.c               | 4 ++++\n refs/files-backend.c | 2 --\n 2 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 4ff55cf24f..4a7c394226 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -32,6 +32,7 @@\n #include \"commit.h\"\n #include \"wildmatch.h\"\n #include \"ident.h\"\n+#include \"fsck.h\"\n \n /*\n  * List of all available backends\n@@ -323,6 +324,9 @@ int check_refname_format(const char *refname, int flags)\n int refs_fsck(struct ref_store *refs, struct fsck_options *o,\n \t      struct worktree *wt)\n {\n+\tif (o->verbose)\n+\t\tfprintf_ln(stderr, _(\"Checking references consistency\"));\n+\n \treturn refs->be->fsck(refs, o, wt);\n }\n \ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex d4fb033417..603b1343d8 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3906,8 +3906,6 @@ static int files_fsck_refs(struct ref_store *ref_store,\n \t\tNULL,\n \t};\n \n-\tif (o->verbose)\n-\t\tfprintf_ln(stderr, _(\"Checking references consistency\"));\n \treturn files_fsck_refs_dir(ref_store, o, \"refs\", wt, fsck_refs_fn);\n }\n \n\n-- \n2.51.0\n\n"},{"id":"527398","messageId":"20250926-228-reftable-introduce-consistency-checks-v4-3-c96fd8551c0d@gmail.com","threadId":"64212","inReplyTo":"20250926-228-reftable-introduce-consistency-checks-v4-0-c96fd8551c0d@gmail.com","subject":"[PATCH v4 3/7] reftable: check for trailing newline in 'tables.list'","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-09-26T07:25:46Z","receivedAt":"2025-09-26T07:25:58Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"In the reftable format, the 'tables.list' file contains a\nnewline separated list of tables. While we parse this file, we do not\ncheck or care about the last newline. Tighten the parser in\n`parse_names()` to return an appropriate error if the last newline is\nmissing.\n\nThis requires modification to `parse_names()` to now return the error\nwhile accepting the output as a third argument.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n reftable/basics.c                | 37 ++++++++++++++++++++++++-------------\n reftable/basics.h                |  5 ++---\n reftable/stack.c                 |  7 +------\n t/unit-tests/u-reftable-basics.c | 24 ++++++++++++++++++++----\n 4 files changed, 47 insertions(+), 26 deletions(-)\n\ndiff --git a/reftable/basics.c b/reftable/basics.c\nindex 9988ebd635..e969927b61 100644\n--- a/reftable/basics.c\n+++ b/reftable/basics.c\n@@ -195,44 +195,55 @@ size_t names_length(const char **names)\n \treturn p - names;\n }\n \n-char **parse_names(char *buf, int size)\n+int parse_names(char *buf, int size, char ***out)\n {\n \tchar **names = NULL;\n \tsize_t names_cap = 0;\n \tsize_t names_len = 0;\n \tchar *p = buf;\n \tchar *end = buf + size;\n+\tint err = 0;\n \n \twhile (p < end) {\n \t\tchar *next = strchr(p, '\\n');\n-\t\tif (next && next < end) {\n-\t\t\t*next = 0;\n+\t\tif (!next) {\n+\t\t\terr = REFTABLE_FORMAT_ERROR;\n+\t\t\tgoto done;\n+\t\t} else if (next < end) {\n+\t\t\t*next = '\\0';\n \t\t} else {\n \t\t\tnext = end;\n \t\t}\n+\n \t\tif (p < next) {\n \t\t\tif (REFTABLE_ALLOC_GROW(names, names_len + 1,\n-\t\t\t\t\t\tnames_cap))\n-\t\t\t\tgoto err;\n+\t\t\t\t\t\tnames_cap)) {\n+\t\t\t\terr = REFTABLE_OUT_OF_MEMORY_ERROR;\n+\t\t\t\tgoto done;\n+\t\t\t}\n \n \t\t\tnames[names_len] = reftable_strdup(p);\n-\t\t\tif (!names[names_len++])\n-\t\t\t\tgoto err;\n+\t\t\tif (!names[names_len++]) {\n+\t\t\t\terr = REFTABLE_OUT_OF_MEMORY_ERROR;\n+\t\t\t\tgoto done;\n+\t\t\t}\n \t\t}\n \t\tp = next + 1;\n \t}\n \n-\tif (REFTABLE_ALLOC_GROW(names, names_len + 1, names_cap))\n-\t\tgoto err;\n+\tif (REFTABLE_ALLOC_GROW(names, names_len + 1, names_cap)) {\n+\t\terr = REFTABLE_OUT_OF_MEMORY_ERROR;\n+\t\tgoto done;\n+\t}\n \tnames[names_len] = NULL;\n \n-\treturn names;\n-\n-err:\n+\t*out = names;\n+\treturn 0;\n+done:\n \tfor (size_t i = 0; i < names_len; i++)\n \t\treftable_free(names[i]);\n \treftable_free(names);\n-\treturn NULL;\n+\treturn err;\n }\n \n int names_equal(const char **a, const char **b)\ndiff --git a/reftable/basics.h b/reftable/basics.h\nindex 7d22f96261..693db9524f 100644\n--- a/reftable/basics.h\n+++ b/reftable/basics.h\n@@ -167,10 +167,9 @@ void free_names(char **a);\n \n /*\n  * Parse a newline separated list of names. `size` is the length of the buffer,\n- * without terminating '\\0'. Empty names are discarded. Returns a `NULL`\n- * pointer when allocations fail.\n+ * without terminating '\\0'. Empty names are discarded.\n  */\n-char **parse_names(char *buf, int size);\n+int parse_names(char *buf, int size, char ***out);\n \n /* compares two NULL-terminated arrays of strings. */\n int names_equal(const char **a, const char **b);\ndiff --git a/reftable/stack.c b/reftable/stack.c\nindex f91ce50bcd..65d89820bd 100644\n--- a/reftable/stack.c\n+++ b/reftable/stack.c\n@@ -109,12 +109,7 @@ static int fd_read_lines(int fd, char ***namesp)\n \t}\n \tbuf[size] = 0;\n \n-\t*namesp = parse_names(buf, size);\n-\tif (!*namesp) {\n-\t\terr = REFTABLE_OUT_OF_MEMORY_ERROR;\n-\t\tgoto done;\n-\t}\n-\n+\terr = parse_names(buf, size, namesp);\n done:\n \treftable_free(buf);\n \treturn err;\ndiff --git a/t/unit-tests/u-reftable-basics.c b/t/unit-tests/u-reftable-basics.c\nindex a0471083e7..73566ed0eb 100644\n--- a/t/unit-tests/u-reftable-basics.c\n+++ b/t/unit-tests/u-reftable-basics.c\n@@ -9,6 +9,7 @@ license that can be found in the LICENSE file or at\n #include \"unit-test.h\"\n #include \"lib-reftable.h\"\n #include \"reftable/basics.h\"\n+#include \"reftable/reftable-error.h\"\n \n struct integer_needle_lesseq_args {\n \tint needle;\n@@ -79,14 +80,18 @@ void test_reftable_basics__names_equal(void)\n void test_reftable_basics__parse_names(void)\n {\n \tchar in1[] = \"line\\n\";\n-\tchar in2[] = \"a\\nb\\nc\";\n-\tchar **out = parse_names(in1, strlen(in1));\n+\tchar in2[] = \"a\\nb\\nc\\n\";\n+\tchar **out = NULL;\n+\tint err = parse_names(in1, strlen(in1), &out);\n+\tcl_assert(err == 0);\n \tcl_assert(out != NULL);\n \tcl_assert_equal_s(out[0], \"line\");\n \tcl_assert(!out[1]);\n \tfree_names(out);\n \n-\tout = parse_names(in2, strlen(in2));\n+\tout = NULL;\n+\terr = parse_names(in2, strlen(in2), &out);\n+\tcl_assert(err == 0);\n \tcl_assert(out != NULL);\n \tcl_assert_equal_s(out[0], \"a\");\n \tcl_assert_equal_s(out[1], \"b\");\n@@ -95,10 +100,21 @@ void test_reftable_basics__parse_names(void)\n \tfree_names(out);\n }\n \n+void test_reftable_basics__parse_names_missing_newline(void)\n+{\n+\tchar in1[] = \"line\\nline2\";\n+\tchar **out = NULL;\n+\tint err = parse_names(in1, strlen(in1), &out);\n+\tcl_assert(err == REFTABLE_FORMAT_ERROR);\n+\tcl_assert(out == NULL);\n+}\n+\n void test_reftable_basics__parse_names_drop_empty_string(void)\n {\n \tchar in[] = \"a\\n\\nb\\n\";\n-\tchar **out = parse_names(in, strlen(in));\n+\tchar **out = NULL;\n+\tint err = parse_names(in, strlen(in), &out);\n+\tcl_assert(err == 0);\n \tcl_assert(out != NULL);\n \tcl_assert_equal_s(out[0], \"a\");\n \t/* simply '\\n' should be dropped as empty string */\n\n-- \n2.51.0\n\n"},{"id":"527399","messageId":"20250926-228-reftable-introduce-consistency-checks-v4-4-c96fd8551c0d@gmail.com","threadId":"64212","inReplyTo":"20250926-228-reftable-introduce-consistency-checks-v4-0-c96fd8551c0d@gmail.com","subject":"[PATCH v4 4/7] Documentation/fsck-msgids: remove duplicate msg id","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-09-26T07:25:47Z","receivedAt":"2025-09-26T07:25:59Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The `gitmodulesLarge` is repeated twice. Remove the second duplicate.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n Documentation/fsck-msgids.adoc | 3 ---\n 1 file changed, 3 deletions(-)\n\ndiff --git a/Documentation/fsck-msgids.adoc b/Documentation/fsck-msgids.adoc\nindex 0ba4f9a27e..1c912615f9 100644\n--- a/Documentation/fsck-msgids.adoc\n+++ b/Documentation/fsck-msgids.adoc\n@@ -104,9 +104,6 @@\n `gitmodulesParse`::\n \t(INFO) Could not parse `.gitmodules` blob.\n \n-`gitmodulesLarge`;\n-\t(ERROR) `.gitmodules` blob is too large to parse.\n-\n `gitmodulesPath`::\n \t(ERROR) `.gitmodules` path is invalid.\n \n\n-- \n2.51.0\n\n"},{"id":"527400","messageId":"20250926-228-reftable-introduce-consistency-checks-v4-5-c96fd8551c0d@gmail.com","threadId":"64212","inReplyTo":"20250926-228-reftable-introduce-consistency-checks-v4-0-c96fd8551c0d@gmail.com","subject":"[PATCH v4 5/7] fsck: order 'fsck_msg_type' alphabetically","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-09-26T07:25:48Z","receivedAt":"2025-09-26T07:26:00Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The list of 'fsck_msg_type' seem to be alphabetically ordered, but there\nare a few small misses. Fix this by sorting the sub-sections of the\nlist to maintain alphabetical ordering.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n fsck.h | 38 +++++++++++++++++++-------------------\n 1 file changed, 19 insertions(+), 19 deletions(-)\n\ndiff --git a/fsck.h b/fsck.h\nindex dd7df3d5b3..6b0db235e0 100644\n--- a/fsck.h\n+++ b/fsck.h\n@@ -33,15 +33,27 @@ enum fsck_msg_type {\n \tFUNC(BAD_PACKED_REF_ENTRY, ERROR) \\\n \tFUNC(BAD_PACKED_REF_HEADER, ERROR) \\\n \tFUNC(BAD_PARENT_SHA1, ERROR) \\\n+\tFUNC(BAD_REFERENT_NAME, ERROR) \\\n \tFUNC(BAD_REF_CONTENT, ERROR) \\\n \tFUNC(BAD_REF_FILETYPE, ERROR) \\\n \tFUNC(BAD_REF_NAME, ERROR) \\\n-\tFUNC(BAD_REFERENT_NAME, ERROR) \\\n \tFUNC(BAD_TIMEZONE, ERROR) \\\n \tFUNC(BAD_TREE, ERROR) \\\n \tFUNC(BAD_TREE_SHA1, ERROR) \\\n \tFUNC(BAD_TYPE, ERROR) \\\n \tFUNC(DUPLICATE_ENTRIES, ERROR) \\\n+\tFUNC(GITATTRIBUTES_BLOB, ERROR) \\\n+\tFUNC(GITATTRIBUTES_LARGE, ERROR) \\\n+\tFUNC(GITATTRIBUTES_LINE_LENGTH, ERROR) \\\n+\tFUNC(GITATTRIBUTES_MISSING, ERROR) \\\n+\tFUNC(GITMODULES_BLOB, ERROR) \\\n+\tFUNC(GITMODULES_LARGE, ERROR) \\\n+\tFUNC(GITMODULES_MISSING, ERROR) \\\n+\tFUNC(GITMODULES_NAME, ERROR) \\\n+\tFUNC(GITMODULES_PATH, ERROR) \\\n+\tFUNC(GITMODULES_SYMLINK, ERROR) \\\n+\tFUNC(GITMODULES_UPDATE, ERROR) \\\n+\tFUNC(GITMODULES_URL, ERROR) \\\n \tFUNC(MISSING_AUTHOR, ERROR) \\\n \tFUNC(MISSING_COMMITTER, ERROR) \\\n \tFUNC(MISSING_EMAIL, ERROR) \\\n@@ -60,39 +72,27 @@ enum fsck_msg_type {\n \tFUNC(TREE_NOT_SORTED, ERROR) \\\n \tFUNC(UNKNOWN_TYPE, ERROR) \\\n \tFUNC(ZERO_PADDED_DATE, ERROR) \\\n-\tFUNC(GITMODULES_MISSING, ERROR) \\\n-\tFUNC(GITMODULES_BLOB, ERROR) \\\n-\tFUNC(GITMODULES_LARGE, ERROR) \\\n-\tFUNC(GITMODULES_NAME, ERROR) \\\n-\tFUNC(GITMODULES_SYMLINK, ERROR) \\\n-\tFUNC(GITMODULES_URL, ERROR) \\\n-\tFUNC(GITMODULES_PATH, ERROR) \\\n-\tFUNC(GITMODULES_UPDATE, ERROR) \\\n-\tFUNC(GITATTRIBUTES_MISSING, ERROR) \\\n-\tFUNC(GITATTRIBUTES_LARGE, ERROR) \\\n-\tFUNC(GITATTRIBUTES_LINE_LENGTH, ERROR) \\\n-\tFUNC(GITATTRIBUTES_BLOB, ERROR) \\\n \t/* warnings */ \\\n \tFUNC(EMPTY_NAME, WARN) \\\n \tFUNC(FULL_PATHNAME, WARN) \\\n \tFUNC(HAS_DOT, WARN) \\\n \tFUNC(HAS_DOTDOT, WARN) \\\n \tFUNC(HAS_DOTGIT, WARN) \\\n+\tFUNC(LARGE_PATHNAME, WARN) \\\n \tFUNC(NULL_SHA1, WARN) \\\n-\tFUNC(ZERO_PADDED_FILEMODE, WARN) \\\n \tFUNC(NUL_IN_COMMIT, WARN) \\\n-\tFUNC(LARGE_PATHNAME, WARN) \\\n+\tFUNC(ZERO_PADDED_FILEMODE, WARN) \\\n \t/* infos (reported as warnings, but ignored by default) */ \\\n \tFUNC(BAD_FILEMODE, INFO) \\\n+\tFUNC(BAD_TAG_NAME, INFO) \\\n \tFUNC(EMPTY_PACKED_REFS_FILE, INFO) \\\n-\tFUNC(GITMODULES_PARSE, INFO) \\\n-\tFUNC(GITIGNORE_SYMLINK, INFO) \\\n \tFUNC(GITATTRIBUTES_SYMLINK, INFO) \\\n+\tFUNC(GITIGNORE_SYMLINK, INFO) \\\n+\tFUNC(GITMODULES_PARSE, INFO) \\\n \tFUNC(MAILMAP_SYMLINK, INFO) \\\n-\tFUNC(BAD_TAG_NAME, INFO) \\\n \tFUNC(MISSING_TAGGER_ENTRY, INFO) \\\n-\tFUNC(SYMLINK_REF, INFO) \\\n \tFUNC(REF_MISSING_NEWLINE, INFO) \\\n+\tFUNC(SYMLINK_REF, INFO) \\\n \tFUNC(SYMREF_TARGET_IS_NOT_A_REF, INFO) \\\n \tFUNC(TRAILING_REF_CONTENT, INFO) \\\n \t/* ignored (elevated when requested) */ \\\n\n-- \n2.51.0\n\n"},{"id":"527401","messageId":"20250926-228-reftable-introduce-consistency-checks-v4-6-c96fd8551c0d@gmail.com","threadId":"64212","inReplyTo":"20250926-228-reftable-introduce-consistency-checks-v4-0-c96fd8551c0d@gmail.com","subject":"[PATCH v4 6/7] reftable: add code to facilitate consistency checks","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-09-26T07:25:49Z","receivedAt":"2025-09-26T07:26:01Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The `git refs verify` command is used to run consistency checks on the\nreference backends. This command is also invoked when users run 'git\nfsck'. While the files-backend has some fsck checks added, the reftable\nbackend lacks such checks. Let's add the required infrastructure and a\ncheck to test for the files present in the reftable directory.\n\nSince the reftable library is treated as an independent library we\nshould ensure that the library code works independently without\nknowledge about Git's internals. To do this, add both 'reftable/fsck.c'\nand 'reftable/reftable-fsck.h'. Which provide an entry point\n'reftable_fsck_check' for running fsck checks over a provided reftable\nstack. The callee provides the function with callbacks to handle issue\nand information reporting.\n\nThe added check, goes over all tables in the reftable stack validates\nthat they have a valid name. It not, it raises an error.\n\nWhile here, move 'reftable/error.o' in the Makefile to retain\nlexicographic ordering.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n Makefile                 |   3 +-\n meson.build              |   1 +\n reftable/fsck.c          | 100 +++++++++++++++++++++++++++++++++++++++++++++++\n reftable/reftable-fsck.h |  40 +++++++++++++++++++\n 4 files changed, 143 insertions(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 4c95affadb..03fbaf2b21 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2732,9 +2732,10 @@ XDIFF_OBJS += xdiff/xutils.o\n xdiff-objs: $(XDIFF_OBJS)\n \n REFTABLE_OBJS += reftable/basics.o\n-REFTABLE_OBJS += reftable/error.o\n REFTABLE_OBJS += reftable/block.o\n REFTABLE_OBJS += reftable/blocksource.o\n+REFTABLE_OBJS += reftable/error.o\n+REFTABLE_OBJS += reftable/fsck.o\n REFTABLE_OBJS += reftable/iter.o\n REFTABLE_OBJS += reftable/merged.o\n REFTABLE_OBJS += reftable/pq.o\ndiff --git a/meson.build b/meson.build\nindex b3dfcc0497..8914252910 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -452,6 +452,7 @@ libgit_sources = [\n   'reftable/error.c',\n   'reftable/block.c',\n   'reftable/blocksource.c',\n+  'reftable/fsck.c',\n   'reftable/iter.c',\n   'reftable/merged.c',\n   'reftable/pq.c',\ndiff --git a/reftable/fsck.c b/reftable/fsck.c\nnew file mode 100644\nindex 0000000000..26b9115b14\n--- /dev/null\n+++ b/reftable/fsck.c\n@@ -0,0 +1,100 @@\n+#include \"basics.h\"\n+#include \"reftable-fsck.h\"\n+#include \"reftable-table.h\"\n+#include \"stack.h\"\n+\n+static bool table_has_valid_name(const char *name)\n+{\n+\tconst char *ptr = name;\n+\tchar *endptr;\n+\n+\t/* strtoull doesn't set errno on success */\n+\terrno = 0;\n+\n+\tstrtoull(ptr, &endptr, 16);\n+\tif (errno)\n+\t\treturn false;\n+\tptr = endptr;\n+\n+\tif (*ptr != '-')\n+\t\treturn false;\n+\tptr++;\n+\n+\tstrtoull(ptr, &endptr, 16);\n+\tif (errno)\n+\t\treturn false;\n+\tptr = endptr;\n+\n+\tif (*ptr != '-')\n+\t\treturn false;\n+\tptr++;\n+\n+\tstrtoul(ptr, &endptr, 16);\n+\tif (errno)\n+\t\treturn false;\n+\tptr = endptr;\n+\n+\tif (strcmp(ptr, \".ref\") && strcmp(ptr, \".log\"))\n+\t\treturn false;\n+\n+\treturn true;\n+}\n+\n+typedef int (*table_check_fn)(struct reftable_table *table,\n+\t\t\t      reftable_fsck_report_fn report_fn,\n+\t\t\t      void *cb_data);\n+\n+static int table_check_name(struct reftable_table *table,\n+\t\t\t    reftable_fsck_report_fn report_fn,\n+\t\t\t    void *cb_data)\n+{\n+\tif (!table_has_valid_name(table->name)) {\n+\t\tstruct reftable_fsck_info info;\n+\n+\t\tinfo.error = REFTABLE_FSCK_ERROR_TABLE_NAME;\n+\t\tinfo.msg = \"invalid reftable table name\";\n+\t\tinfo.path = table->name;\n+\n+\t\treturn report_fn(&info, cb_data);\n+\t}\n+\n+\treturn 0;\n+}\n+\n+static int table_checks(struct reftable_table *table,\n+\t\t\treftable_fsck_report_fn report_fn,\n+\t\t\treftable_fsck_verbose_fn verbose_fn UNUSED,\n+\t\t\tvoid *cb_data)\n+{\n+\ttable_check_fn table_check_fns[] = {\n+\t\ttable_check_name,\n+\t\tNULL,\n+\t};\n+\tint err = 0;\n+\n+\tfor (size_t i = 0; table_check_fns[i]; i++)\n+\t\terr |= table_check_fns[i](table, report_fn, cb_data);\n+\n+\treturn err;\n+}\n+\n+int reftable_fsck_check(struct reftable_stack *stack,\n+\t\t\treftable_fsck_report_fn report_fn,\n+\t\t\treftable_fsck_verbose_fn verbose_fn,\n+\t\t\tvoid *cb_data)\n+{\n+\tstruct reftable_buf msg = REFTABLE_BUF_INIT;\n+\tint err = 0;\n+\n+\tfor (size_t i = 0; i < stack->tables_len; i++) {\n+\t\treftable_buf_reset(&msg);\n+\t\treftable_buf_addstr(&msg, \"Checking table: \");\n+\t\treftable_buf_addstr(&msg, stack->tables[i]->name);\n+\t\tverbose_fn(msg.buf, cb_data);\n+\n+\t\terr |= table_checks(stack->tables[i], report_fn, verbose_fn, cb_data);\n+\t}\n+\n+\treftable_buf_release(&msg);\n+\treturn err;\n+}\ndiff --git a/reftable/reftable-fsck.h b/reftable/reftable-fsck.h\nnew file mode 100644\nindex 0000000000..007a392cf9\n--- /dev/null\n+++ b/reftable/reftable-fsck.h\n@@ -0,0 +1,40 @@\n+#ifndef REFTABLE_FSCK_H\n+#define REFTABLE_FSCK_H\n+\n+#include \"reftable-stack.h\"\n+\n+enum reftable_fsck_error {\n+\t/* Invalid table name */\n+\tREFTABLE_FSCK_ERROR_TABLE_NAME = 0,\n+\t/* Used for bounds checking, must be last */\n+\tREFTABLE_FSCK_MAX_VALUE,\n+};\n+\n+/* Represents an individual error encountered during the FSCK checks. */\n+struct reftable_fsck_info {\n+\tenum reftable_fsck_error error;\n+\tconst char *msg;\n+\tconst char *path;\n+};\n+\n+typedef int reftable_fsck_report_fn(struct reftable_fsck_info *info,\n+\t\t\t\t    void *cb_data);\n+typedef void reftable_fsck_verbose_fn(const char *msg, void *cb_data);\n+\n+/*\n+ * Given a reftable stack, perform consistency checks on the stack.\n+ *\n+ * If an issue is encountered, the issue is reported to the callee via the\n+ * provided 'report_fn'. If the issue is non-recoverable the flow will not\n+ * continue. If it is recoverable, the flow will continue and further issues\n+ * will be reported as identified.\n+ *\n+ * The 'verbose_fn' will be invoked to provide verbose information about\n+ * the progress and state of the consistency checks.\n+ */\n+int reftable_fsck_check(struct reftable_stack *stack,\n+\t\t\treftable_fsck_report_fn report_fn,\n+\t\t\treftable_fsck_verbose_fn verbose_fn,\n+\t\t\tvoid *cb_data);\n+\n+#endif /* REFTABLE_FSCK_H */\n\n-- \n2.51.0\n\n"},{"id":"527402","messageId":"20250926-228-reftable-introduce-consistency-checks-v4-7-c96fd8551c0d@gmail.com","threadId":"64212","inReplyTo":"20250926-228-reftable-introduce-consistency-checks-v4-0-c96fd8551c0d@gmail.com","subject":"[PATCH v4 7/7] refs/reftable: add fsck check for checking the table name","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-09-26T07:25:50Z","receivedAt":"2025-09-26T07:26:01Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Add glue code in 'refs/reftable-backend.c' which calls the reftable\nlibrary to perform the fsck checks. Here we also map the reftable errors\nto Git' fsck errors.\n\nIntroduce a check to validate table names for a given reftable stack.\nAlso add 'badReftableTableName' as a corresponding error within Git. The\nreftable specification mentions:\n\n  It suggested to use\n  ${min_update_index}-${max_update_index}-${random}.ref as a naming\n  convention.\n\nSo treat non-conformant file names as warnings.\n\nWhile adding the fsck header to 'refs/reftable-backend.c', modify the\nlist to maintain lexicographical ordering.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n Documentation/fsck-msgids.adoc |  3 +++\n fsck.h                         |  1 +\n refs/reftable-backend.c        | 57 ++++++++++++++++++++++++++++++++++++++----\n t/meson.build                  |  1 +\n t/t0614-reftable-fsck.sh       | 38 ++++++++++++++++++++++++++++\n 5 files changed, 95 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/fsck-msgids.adoc b/Documentation/fsck-msgids.adoc\nindex 1c912615f9..81f11ba125 100644\n--- a/Documentation/fsck-msgids.adoc\n+++ b/Documentation/fsck-msgids.adoc\n@@ -38,6 +38,9 @@\n `badReferentName`::\n \t(ERROR) The referent name of a symref is invalid.\n \n+`badReftableTableName`::\n+\t(WARN) A reftable table has an invalid name.\n+\n `badTagName`::\n \t(INFO) A tag has an invalid format.\n \ndiff --git a/fsck.h b/fsck.h\nindex 6b0db235e0..759df97655 100644\n--- a/fsck.h\n+++ b/fsck.h\n@@ -73,6 +73,7 @@ enum fsck_msg_type {\n \tFUNC(UNKNOWN_TYPE, ERROR) \\\n \tFUNC(ZERO_PADDED_DATE, ERROR) \\\n \t/* warnings */ \\\n+\tFUNC(BAD_REFTABLE_TABLE_NAME, WARN) \\\n \tFUNC(EMPTY_NAME, WARN) \\\n \tFUNC(FULL_PATHNAME, WARN) \\\n \tFUNC(HAS_DOT, WARN) \\\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex 2152349cb9..b106fd8b53 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -6,6 +6,7 @@\n #include \"../config.h\"\n #include \"../dir.h\"\n #include \"../environment.h\"\n+#include \"../fsck.h\"\n #include \"../gettext.h\"\n #include \"../hash.h\"\n #include \"../hex.h\"\n@@ -15,10 +16,11 @@\n #include \"../path.h\"\n #include \"../refs.h\"\n #include \"../reftable/reftable-basics.h\"\n-#include \"../reftable/reftable-stack.h\"\n-#include \"../reftable/reftable-record.h\"\n #include \"../reftable/reftable-error.h\"\n+#include \"../reftable/reftable-fsck.h\"\n #include \"../reftable/reftable-iterator.h\"\n+#include \"../reftable/reftable-record.h\"\n+#include \"../reftable/reftable-stack.h\"\n #include \"../repo-settings.h\"\n #include \"../setup.h\"\n #include \"../strmap.h\"\n@@ -2707,11 +2709,56 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store,\n \treturn ret;\n }\n \n-static int reftable_be_fsck(struct ref_store *ref_store UNUSED,\n-\t\t\t    struct fsck_options *o UNUSED,\n+static void reftable_fsck_verbose_handler(const char *msg, void *cb_data)\n+{\n+\tstruct fsck_options *o = cb_data;\n+\n+\tif (o->verbose)\n+\t\tfprintf_ln(stderr, \"%s\", msg);\n+}\n+\n+static const enum fsck_msg_id fsck_msg_id_map[] = {\n+\t[REFTABLE_FSCK_ERROR_TABLE_NAME] = FSCK_MSG_BAD_REFTABLE_TABLE_NAME,\n+};\n+\n+static int reftable_fsck_error_handler(struct reftable_fsck_info *info,\n+\t\t\t\t       void *cb_data)\n+{\n+\tstruct fsck_ref_report report = { .path = info->path };\n+\tstruct fsck_options *o = cb_data;\n+\tenum fsck_msg_id msg_id;\n+\n+\tif (info->error < 0 || info->error >= REFTABLE_FSCK_MAX_VALUE)\n+\t\tBUG(\"unknown fsck error: %d\", (int)info->error);\n+\n+\tmsg_id = fsck_msg_id_map[info->error];\n+\n+\tif (!msg_id)\n+\t\tBUG(\"fsck_msg_id value missing for reftable error: %d\", (int)info->error);\n+\n+\treturn fsck_report_ref(o, &report, msg_id, \"%s\", info->msg);\n+}\n+\n+static int reftable_be_fsck(struct ref_store *ref_store, struct fsck_options *o,\n \t\t\t    struct worktree *wt UNUSED)\n {\n-\treturn 0;\n+\tstruct reftable_ref_store *refs;\n+\tstruct strmap_entry *entry;\n+\tstruct hashmap_iter iter;\n+\tint ret = 0;\n+\n+\trefs = reftable_be_downcast(ref_store, REF_STORE_READ, \"fsck\");\n+\n+\tret |= reftable_fsck_check(refs->main_backend.stack, reftable_fsck_error_handler,\n+\t\t\t\t   reftable_fsck_verbose_handler, o);\n+\n+\tstrmap_for_each_entry(&refs->worktree_backends, &iter, entry) {\n+\t\tstruct reftable_backend *b = (struct reftable_backend *)entry->value;\n+\t\tret |= reftable_fsck_check(b->stack, reftable_fsck_error_handler,\n+\t\t\t\t\t   reftable_fsck_verbose_handler, o);\n+\t}\n+\n+\treturn ret;\n }\n \n struct ref_storage_be refs_be_reftable = {\ndiff --git a/t/meson.build b/t/meson.build\nindex 7974795fe4..ec1fc0b2a1 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -146,6 +146,7 @@ integration_tests = [\n   't0611-reftable-httpd.sh',\n   't0612-reftable-jgit-compatibility.sh',\n   't0613-reftable-write-options.sh',\n+  't0614-reftable-fsck.sh',\n   't1000-read-tree-m-3way.sh',\n   't1001-read-tree-m-2way.sh',\n   't1002-read-tree-m-u-2way.sh',\ndiff --git a/t/t0614-reftable-fsck.sh b/t/t0614-reftable-fsck.sh\nnew file mode 100755\nindex 0000000000..250d244e66\n--- /dev/null\n+++ b/t/t0614-reftable-fsck.sh\n@@ -0,0 +1,38 @@\n+#!/bin/sh\n+\n+test_description='Test reftable backend consistency check'\n+\n+GIT_TEST_DEFAULT_REF_FORMAT=reftable\n+export GIT_TEST_DEFAULT_REF_FORMAT\n+\n+. ./test-lib.sh\n+\n+for TABLE_NAME in \"foo-bar-e4d12d59.ref\" \\\n+\t\"0x00000000zzzz-0x00000000zzzz-e4d12d59.ref\" \\\n+\t\"0x000000000001-0x000000000002-e4d12d59.abc\" \\\n+\t\"0x000000000001-0x000000000002-e4d12d59.refabc\"; do\n+\ttest_expect_success \"table name $TABLE_NAME should be checked\" '\n+\t\ttest_when_finished \"rm -rf repo\" &&\n+\t\tgit init repo &&\n+\t\t(\n+\t\t\tcd repo &&\n+\t\t\tgit commit --allow-empty -m initial &&\n+\n+\t\t\tgit refs verify 2>err &&\n+\t\t\ttest_must_be_empty err &&\n+\n+\t\t\tEXISTING_TABLE=$(head -n1 .git/reftable/tables.list) &&\n+\t\t\tmv \".git/reftable/$EXISTING_TABLE\" \".git/reftable/$TABLE_NAME\" &&\n+\t\t\tsed \"s/${EXISTING_TABLE}/${TABLE_NAME}/g\" .git/reftable/tables.list > tables.list &&\n+\t\t\tmv tables.list .git/reftable/tables.list &&\n+\n+\t\t\tgit refs verify 2>err &&\n+\t\t\tcat >expect <<-EOF &&\n+\t\t\twarning: ${TABLE_NAME}: badReftableTableName: invalid reftable table name\n+\t\t\tEOF\n+\t\t\ttest_cmp expect err\n+\t\t)\n+\t'\n+done\n+\n+test_done\n\n-- \n2.51.0\n\n"},{"id":"527446","messageId":"xmqq1pnsc46y.fsf@gitster.g","threadId":"64212","inReplyTo":"20250926-228-reftable-introduce-consistency-checks-v4-0-c96fd8551c0d@gmail.com","subject":"Re: [PATCH v4 0/7] refs/reftable: add consistency checks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-09-26T21:08:21Z","receivedAt":"2025-09-26T21:08:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak <karthik.188@gmail.com> writes:\n\n> Changes in v4:\n> - The biggest change is to iterate over the tables in a reftable stack\n>   for consistency checks instead of all files inside the REFTABLE_DIR.\n>   This avoids all race conditions. Also, since we only check the tables\n>   in a stack, it no longer makes sense to check file type.\n\nNice.\n\n> - The discussion about update indices was concluded that tables indices\n>   in a stack must be strictly monotonically increasing. While modifying\n>   the code to do the same. I realized that we already have this check in\n>   'reftable_addition_add()' where we check while adding a new table to\n>   the stack: `wr->min_update_index < add->next_update_index`. So I've\n>   dropped this patch from the series.\n\nGreat.  Reading over patches and noticing that it is not needed is\nthe best kind of proofreading ;-)\n\n> - Change parse_names() to accept the output string array as an argument\n>   and return an error instead. This makes the flow a little easier to\n>   understand.\n\nWonderful.\n\nWill queue.  Thanks.\n"},{"id":"527791","messageId":"aN5lmEx7Q57YEyHH@pks.im","threadId":"64212","inReplyTo":"20250926-228-reftable-introduce-consistency-checks-v4-3-c96fd8551c0d@gmail.com","subject":"Re: [PATCH v4 3/7] reftable: check for trailing newline in 'tables.list'","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-02T11:44:24Z","receivedAt":"2025-10-02T11:44:31Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Sep 26, 2025 at 09:25:46AM +0200, Karthik Nayak wrote:\n> diff --git a/reftable/basics.c b/reftable/basics.c\n> index 9988ebd635..e969927b61 100644\n> --- a/reftable/basics.c\n> +++ b/reftable/basics.c\n> @@ -195,44 +195,55 @@ size_t names_length(const char **names)\n>  \treturn p - names;\n>  }\n>  \n> -char **parse_names(char *buf, int size)\n> +int parse_names(char *buf, int size, char ***out)\n>  {\n\nYup, this changed function signature reads a lot nicer to me now and is\nmore in line with our usual coding style.\n\n> diff --git a/reftable/basics.h b/reftable/basics.h\n> index 7d22f96261..693db9524f 100644\n> --- a/reftable/basics.h\n> +++ b/reftable/basics.h\n> @@ -167,10 +167,9 @@ void free_names(char **a);\n>  \n>  /*\n>   * Parse a newline separated list of names. `size` is the length of the buffer,\n> - * without terminating '\\0'. Empty names are discarded. Returns a `NULL`\n> - * pointer when allocations fail.\n> + * without terminating '\\0'. Empty names are discarded.\n>   */\n> -char **parse_names(char *buf, int size);\n> +int parse_names(char *buf, int size, char ***out);\n\nTiny nit, not worth a reroll: we may still want to document that a\nreturn value of 0 means success, and that it otherwise returns a\nreftable error code.\n\nPatrick\n"},{"id":"527792","messageId":"aN5lngvE39JGwGSx@pks.im","threadId":"64212","inReplyTo":"20250926-228-reftable-introduce-consistency-checks-v4-6-c96fd8551c0d@gmail.com","subject":"Re: [PATCH v4 6/7] reftable: add code to facilitate consistency checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-02T11:44:30Z","receivedAt":"2025-10-02T11:44:36Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Sep 26, 2025 at 09:25:49AM +0200, Karthik Nayak wrote:\n> diff --git a/reftable/fsck.c b/reftable/fsck.c\n> new file mode 100644\n> index 0000000000..26b9115b14\n> --- /dev/null\n> +++ b/reftable/fsck.c\n> @@ -0,0 +1,100 @@\n[snip]\n> +static int table_checks(struct reftable_table *table,\n> +\t\t\treftable_fsck_report_fn report_fn,\n> +\t\t\treftable_fsck_verbose_fn verbose_fn UNUSED,\n> +\t\t\tvoid *cb_data)\n> +{\n> +\ttable_check_fn table_check_fns[] = {\n> +\t\ttable_check_name,\n> +\t\tNULL,\n> +\t};\n> +\tint err = 0;\n> +\n> +\tfor (size_t i = 0; table_check_fns[i]; i++)\n> +\t\terr |= table_check_fns[i](table, report_fn, cb_data);\n> +\n> +\treturn err;\n> +}\n\nOkay, good. We now only verify individual table names part of the stack,\nand don't scan the directory anymore. Furthermore, it is easy to add\nmore tests by adding to the function array.\n\nPatrick\n"},{"id":"527793","messageId":"aN5lrhCLQFnw7qUB@pks.im","threadId":"64212","inReplyTo":"20250926-228-reftable-introduce-consistency-checks-v4-7-c96fd8551c0d@gmail.com","subject":"Re: [PATCH v4 7/7] refs/reftable: add fsck check for checking the table name","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-02T11:44:46Z","receivedAt":"2025-10-02T11:44:52Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Sep 26, 2025 at 09:25:50AM +0200, Karthik Nayak wrote:\n> diff --git a/t/t0614-reftable-fsck.sh b/t/t0614-reftable-fsck.sh\n> new file mode 100755\n> index 0000000000..250d244e66\n> --- /dev/null\n> +++ b/t/t0614-reftable-fsck.sh\n> @@ -0,0 +1,38 @@\n> +#!/bin/sh\n> +\n> +test_description='Test reftable backend consistency check'\n> +\n> +GIT_TEST_DEFAULT_REF_FORMAT=reftable\n> +export GIT_TEST_DEFAULT_REF_FORMAT\n> +\n> +. ./test-lib.sh\n> +\n> +for TABLE_NAME in \"foo-bar-e4d12d59.ref\" \\\n> +\t\"0x00000000zzzz-0x00000000zzzz-e4d12d59.ref\" \\\n> +\t\"0x000000000001-0x000000000002-e4d12d59.abc\" \\\n> +\t\"0x000000000001-0x000000000002-e4d12d59.refabc\"; do\n> +\ttest_expect_success \"table name $TABLE_NAME should be checked\" '\n> +\t\ttest_when_finished \"rm -rf repo\" &&\n> +\t\tgit init repo &&\n> +\t\t(\n> +\t\t\tcd repo &&\n> +\t\t\tgit commit --allow-empty -m initial &&\n> +\n> +\t\t\tgit refs verify 2>err &&\n> +\t\t\ttest_must_be_empty err &&\n> +\n> +\t\t\tEXISTING_TABLE=$(head -n1 .git/reftable/tables.list) &&\n> +\t\t\tmv \".git/reftable/$EXISTING_TABLE\" \".git/reftable/$TABLE_NAME\" &&\n> +\t\t\tsed \"s/${EXISTING_TABLE}/${TABLE_NAME}/g\" .git/reftable/tables.list > tables.list &&\n> +\t\t\tmv tables.list .git/reftable/tables.list &&\n> +\n> +\t\t\tgit refs verify 2>err &&\n> +\t\t\tcat >expect <<-EOF &&\n> +\t\t\twarning: ${TABLE_NAME}: badReftableTableName: invalid reftable table name\n> +\t\t\tEOF\n> +\t\t\ttest_cmp expect err\n> +\t\t)\n> +\t'\n> +done\n> +\n> +test_done\n\nNit: we don't have any test that verifies that `git refs verify` doesn't\ncomplain with a well-formed stack.\n\nOther than that this series looks good to me, thanks! I think we might\nwant to have one final reroll, but once that's out I think this should\nbe ready to be merged down.\n\nPatrick\n"},{"id":"527975","messageId":"CAOLa=ZT48BV5Xs8R0Mwi1tpMHKC1jrR8AGScHHAq37xU4_UpOw@mail.gmail.com","threadId":"64212","inReplyTo":"aN5lmEx7Q57YEyHH@pks.im","subject":"Re: [PATCH v4 3/7] reftable: check for trailing newline in 'tables.list'","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-10-06T12:02:27Z","receivedAt":"2025-10-06T12:02:39Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Fri, Sep 26, 2025 at 09:25:46AM +0200, Karthik Nayak wrote:\n>> diff --git a/reftable/basics.c b/reftable/basics.c\n>> index 9988ebd635..e969927b61 100644\n>> --- a/reftable/basics.c\n>> +++ b/reftable/basics.c\n>> @@ -195,44 +195,55 @@ size_t names_length(const char **names)\n>>  \treturn p - names;\n>>  }\n>>\n>> -char **parse_names(char *buf, int size)\n>> +int parse_names(char *buf, int size, char ***out)\n>>  {\n>\n> Yup, this changed function signature reads a lot nicer to me now and is\n> more in line with our usual coding style.\n>\n\nI have to agree with that. It would be so much nicer, if we could return\n(value, err) in C. Anyways this seems a lot more consistent.\n\n>> diff --git a/reftable/basics.h b/reftable/basics.h\n>> index 7d22f96261..693db9524f 100644\n>> --- a/reftable/basics.h\n>> +++ b/reftable/basics.h\n>> @@ -167,10 +167,9 @@ void free_names(char **a);\n>>\n>>  /*\n>>   * Parse a newline separated list of names. `size` is the length of the buffer,\n>> - * without terminating '\\0'. Empty names are discarded. Returns a `NULL`\n>> - * pointer when allocations fail.\n>> + * without terminating '\\0'. Empty names are discarded.\n>>   */\n>> -char **parse_names(char *buf, int size);\n>> +int parse_names(char *buf, int size, char ***out);\n>\n> Tiny nit, not worth a reroll: we may still want to document that a\n> return value of 0 means success, and that it otherwise returns a\n> reftable error code.\n>\n\nYes, I'll add that in my re-roll.\n\n> Patrick\n"},{"id":"527976","messageId":"CAOLa=ZRU01MQCLwWx4zEZNRtqNBvUJnMo8euE+J86q2a9adSwA@mail.gmail.com","threadId":"64212","inReplyTo":"aN5lrhCLQFnw7qUB@pks.im","subject":"Re: [PATCH v4 7/7] refs/reftable: add fsck check for checking the table name","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-10-06T12:05:25Z","receivedAt":"2025-10-06T12:05:27Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Fri, Sep 26, 2025 at 09:25:50AM +0200, Karthik Nayak wrote:\n>> diff --git a/t/t0614-reftable-fsck.sh b/t/t0614-reftable-fsck.sh\n>> new file mode 100755\n>> index 0000000000..250d244e66\n>> --- /dev/null\n>> +++ b/t/t0614-reftable-fsck.sh\n>> @@ -0,0 +1,38 @@\n>> +#!/bin/sh\n>> +\n>> +test_description='Test reftable backend consistency check'\n>> +\n>> +GIT_TEST_DEFAULT_REF_FORMAT=reftable\n>> +export GIT_TEST_DEFAULT_REF_FORMAT\n>> +\n>> +. ./test-lib.sh\n>> +\n>> +for TABLE_NAME in \"foo-bar-e4d12d59.ref\" \\\n>> +\t\"0x00000000zzzz-0x00000000zzzz-e4d12d59.ref\" \\\n>> +\t\"0x000000000001-0x000000000002-e4d12d59.abc\" \\\n>> +\t\"0x000000000001-0x000000000002-e4d12d59.refabc\"; do\n>> +\ttest_expect_success \"table name $TABLE_NAME should be checked\" '\n>> +\t\ttest_when_finished \"rm -rf repo\" &&\n>> +\t\tgit init repo &&\n>> +\t\t(\n>> +\t\t\tcd repo &&\n>> +\t\t\tgit commit --allow-empty -m initial &&\n>> +\n>> +\t\t\tgit refs verify 2>err &&\n>> +\t\t\ttest_must_be_empty err &&\n>> +\n>> +\t\t\tEXISTING_TABLE=$(head -n1 .git/reftable/tables.list) &&\n>> +\t\t\tmv \".git/reftable/$EXISTING_TABLE\" \".git/reftable/$TABLE_NAME\" &&\n>> +\t\t\tsed \"s/${EXISTING_TABLE}/${TABLE_NAME}/g\" .git/reftable/tables.list > tables.list &&\n>> +\t\t\tmv tables.list .git/reftable/tables.list &&\n>> +\n>> +\t\t\tgit refs verify 2>err &&\n>> +\t\t\tcat >expect <<-EOF &&\n>> +\t\t\twarning: ${TABLE_NAME}: badReftableTableName: invalid reftable table name\n>> +\t\t\tEOF\n>> +\t\t\ttest_cmp expect err\n>> +\t\t)\n>> +\t'\n>> +done\n>> +\n>> +test_done\n>\n> Nit: we don't have any test that verifies that `git refs verify` doesn't\n> complain with a well-formed stack.\n\nThe above test does run `git refs verify` on the repository before\nmodifying the 'tables.list' file. Do you mean a stack with > 1 tables? I\nthink that would be worthwhile. Let me do that.\n\n>\n> Other than that this series looks good to me, thanks! I think we might\n> want to have one final reroll, but once that's out I think this should\n> be ready to be merged down.\n>\n> Patrick\n\nReally appreciate the quick and thorough reviews.\n\nThanks,\nKarthik\n"}]}