{"thread":{"id":"57229","subject":"[PATCH 0/7] reftable: avoid reading and writing empty keys","startedAt":"2022-01-12T18:08:02Z","lastAt":"2022-02-23T21:37:49Z","messageCount":33,"participants":["Han-Wen Nienhuys via GitGitGadget","Junio C Hamano","Han-Wen Nienhuys"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"446034","messageId":"pull.1185.git.git.1642010868.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":null,"subject":"[PATCH 0/7] reftable: avoid reading and writing empty keys","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-12T18:07:41Z","receivedAt":"2022-01-12T18:08:02Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"this series makes sure that the object record does not have to consider\nempty keys (and therefore, a NULL memcpy destination)\n\nwhile we're at it add some more tests, and fix a naming mistake.\n\nHan-Wen Nienhuys (7):\n Documentation: object_id_len goes up to 31\n reftable: reject 0 object_id_len\n reftable: add a test that verifies that writing empty keys fails\n reftable: avoid writing empty keys at the block layer\n reftable: ensure that obj_id_len is >= 2 on writing\n reftable: add test for length of disambiguating prefix\n reftable: rename writer_stats to reftable_writer_stats\n\n Documentation/technical/reftable.txt | 2 +-\n reftable/block.c | 27 ++++---\n reftable/block_test.c | 5 ++\n reftable/reader.c | 5 ++\n reftable/readwrite_test.c | 103 ++++++++++++++++++++++++++-\n reftable/reftable-writer.h | 2 +-\n reftable/writer.c | 9 +--\n 7 files changed, 135 insertions(+), 18 deletions(-)\n\n\nbase-commit: 90d242d36e248acfae0033274b524bfa55a947fd\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1185%2Fhanwen%2Fobj-id-len-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1185/hanwen/obj-id-len-v1\nPull-Request: https://github.com/git/git/pull/1185\n-- \ngitgitgadget\n"},{"id":"446035","messageId":"2d2e1177ff7c15f9a026ebe1e1be17791d9930f0.1642010868.git.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.git.git.1642010868.gitgitgadget@gmail.com","subject":"[PATCH 1/7] Documentation: object_id_len goes up to 31","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-12T18:07:42Z","receivedAt":"2022-01-12T18:08:10Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThe value is stored in a 5-bit field, so we can't support more without\na format version upgrade.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n Documentation/technical/reftable.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/technical/reftable.txt b/Documentation/technical/reftable.txt\nindex d7c3b645cfb..6a67cc4174f 100644\n--- a/Documentation/technical/reftable.txt\n+++ b/Documentation/technical/reftable.txt\n@@ -443,7 +443,7 @@ Obj block format\n Object blocks are optional. Writers may choose to omit object blocks,\n especially if readers will not use the object name to ref mapping.\n \n-Object blocks use unique, abbreviated 2-32 object name keys, mapping to\n+Object blocks use unique, abbreviated 2-31 byte object name keys, mapping to\n ref blocks containing references pointing to that object directly, or as\n the peeled value of an annotated tag. Like ref blocks, object blocks use\n the file's standard block size. The abbreviation length is available in\n-- \ngitgitgadget\n\n"},{"id":"446036","messageId":"747c9e9a4c8f18c324535ca0ab620d41c1dcc38b.1642010868.git.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.git.git.1642010868.gitgitgadget@gmail.com","subject":"[PATCH 2/7] reftable: reject 0 object_id_len","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-12T18:07:43Z","receivedAt":"2022-01-12T18:08:15Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThe spec says 2 <= object_id_len <= 31. We are lenient and allow 1,\nbut we forbid 0, so we can we can be sure that we never read a\n0-length key.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n reftable/reader.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/reftable/reader.c b/reftable/reader.c\nindex 006709a645a..21843f5e935 100644\n--- a/reftable/reader.c\n+++ b/reftable/reader.c\n@@ -155,6 +155,11 @@ static int parse_footer(struct reftable_reader *r, uint8_t *footer,\n \tr->log_offsets.is_present = (first_block_typ == BLOCK_TYPE_LOG ||\n \t\t\t\t r->log_offsets.offset > 0);\n \tr->obj_offsets.is_present = r->obj_offsets.offset > 0;\n+\tif (r->obj_offsets.is_present && !r->object_id_len) {\n+\t\terr = REFTABLE_FORMAT_ERROR;\n+\t\tgoto done;\n+\t}\n+\n \terr = 0;\n done:\n \treturn err;\n-- \ngitgitgadget\n\n"},{"id":"446037","messageId":"4eefedb0d07f762c98e8699c56a58b80df36ebe9.1642010868.git.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.git.git.1642010868.gitgitgadget@gmail.com","subject":"[PATCH 3/7] reftable: add a test that verifies that writing empty keys fails","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-12T18:07:44Z","receivedAt":"2022-01-12T18:08:18Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nEmpty keys can only be written as ref records with empty names. The\nlog record has a logical timestamp in the key, so the key is never\nempty.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n reftable/readwrite_test.c | 24 ++++++++++++++++++++++++\n 1 file changed, 24 insertions(+)\n\ndiff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c\nindex 70c7aedba2c..a315c8992e8 100644\n--- a/reftable/readwrite_test.c\n+++ b/reftable/readwrite_test.c\n@@ -602,6 +602,29 @@ static void test_write_empty_table(void)\n \tstrbuf_release(&buf);\n }\n \n+static void test_write_empty_key(void)\n+{\n+\tstruct reftable_write_options opts = { 0 };\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct reftable_writer *w =\n+\t\treftable_new_writer(&strbuf_add_void, &buf, &opts);\n+\tstruct reftable_ref_record ref = {\n+\t\t.refname = \"\",\n+\t\t.update_index = 1,\n+\t\t.value_type = REFTABLE_REF_DELETION,\n+\t};\n+\tint err;\n+\n+\treftable_writer_set_limits(w, 1, 1);\n+\terr = reftable_writer_add_ref(w, &ref);\n+\tEXPECT(err == REFTABLE_API_ERROR);\n+\n+\terr = reftable_writer_close(w);\n+\tEXPECT(err == REFTABLE_EMPTY_TABLE_ERROR);\n+\treftable_writer_free(w);\n+\tstrbuf_release(&buf);\n+}\n+\n static void test_write_key_order(void)\n {\n \tstruct reftable_write_options opts = { 0 };\n@@ -681,6 +704,7 @@ int readwrite_test_main(int argc, const char *argv[])\n \tRUN_TEST(test_table_read_write_seek_index);\n \tRUN_TEST(test_table_refs_for_no_index);\n \tRUN_TEST(test_table_refs_for_obj_index);\n+\tRUN_TEST(test_write_empty_key);\n \tRUN_TEST(test_write_empty_table);\n \tRUN_TEST(test_log_overflow);\n \treturn 0;\n-- \ngitgitgadget\n\n"},{"id":"446038","messageId":"e4c1cc58265ca7ae7b32b9faf41324883011d1a6.1642010868.git.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.git.git.1642010868.gitgitgadget@gmail.com","subject":"[PATCH 4/7] reftable: avoid writing empty keys at the block layer","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-12T18:07:45Z","receivedAt":"2022-01-12T18:08:23Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThe public interface (reftable_writer) already ensures that keys are\nwritten in strictly increasing order, and an empty key by definition\nfails this check.\n\nHowever, by also enforcing this at the block layer, it is easier to\nverify that records (which are written into blocks) never have to\nconsider the possibility of empty keys.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n reftable/block.c | 27 +++++++++++++++++----------\n reftable/block_test.c | 5 +++++\n reftable/writer.c | 3 +--\n 3 files changed, 23 insertions(+), 12 deletions(-)\n\ndiff --git a/reftable/block.c b/reftable/block.c\nindex 855e3f5c947..8725eaaf64f 100644\n--- a/reftable/block.c\n+++ b/reftable/block.c\n@@ -88,8 +88,9 @@ uint8_t block_writer_type(struct block_writer *bw)\n \treturn bw->buf[bw->header_off];\n }\n \n-/* adds the reftable_record to the block. Returns -1 if it does not fit, 0 on\n- success */\n+/* Adds the reftable_record to the block. Returns -1 if it does not fit, 0 on\n+ success. Returns REFTABLE_API_ERROR if attempting to write a record with\n+ empty key. */\n int block_writer_add(struct block_writer *w, struct reftable_record *rec)\n {\n \tstruct strbuf empty = STRBUF_INIT;\n@@ -105,8 +106,14 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec)\n \tint is_restart = 0;\n \tstruct strbuf key = STRBUF_INIT;\n \tint n = 0;\n+\tint err = -1;\n \n \treftable_record_key(rec, &key);\n+\tif (!key.len) {\n+\t\terr = REFTABLE_API_ERROR;\n+\t\tgoto done;\n+\t}\n+\n \tn = reftable_encode_key(&is_restart, out, last, key,\n \t\t\t\treftable_record_val_type(rec));\n \tif (n < 0)\n@@ -118,16 +125,11 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec)\n \t\tgoto done;\n \tstring_view_consume(&out, n);\n \n-\tif (block_writer_register_restart(w, start.len - out.len, is_restart,\n-\t\t\t\t\t &key) < 0)\n-\t\tgoto done;\n-\n-\tstrbuf_release(&key);\n-\treturn 0;\n-\n+\terr = block_writer_register_restart(w, start.len - out.len, is_restart,\n+\t\t\t\t\t &key);\n done:\n \tstrbuf_release(&key);\n-\treturn -1;\n+\treturn err;\n }\n \n int block_writer_finish(struct block_writer *w)\n@@ -324,6 +326,9 @@ int block_iter_next(struct block_iter *it, struct reftable_record *rec)\n \tif (n < 0)\n \t\treturn -1;\n \n+\tif (!key.len)\n+\t\treturn REFTABLE_FORMAT_ERROR;\n+\n \tstring_view_consume(&in, n);\n \tn = reftable_record_decode(rec, key, extra, in, it->br->hash_size);\n \tif (n < 0)\n@@ -350,6 +355,8 @@ int block_reader_first_key(struct block_reader *br, struct strbuf *key)\n \tint n = reftable_decode_key(key, &extra, empty, in);\n \tif (n < 0)\n \t\treturn n;\n+\tif (!key->len)\n+\t\treturn -1;\n \n \treturn 0;\n }\ndiff --git a/reftable/block_test.c b/reftable/block_test.c\nindex 4b3ea262dcb..5112ddbf468 100644\n--- a/reftable/block_test.c\n+++ b/reftable/block_test.c\n@@ -42,6 +42,11 @@ static void test_block_read_write(void)\n \t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n \treftable_record_from_ref(&rec, &ref);\n \n+\tref.refname = \"\";\n+\tref.value_type = REFTABLE_REF_DELETION;\n+\tn = block_writer_add(&bw, &rec);\n+\tEXPECT(n == REFTABLE_API_ERROR);\n+\n \tfor (i = 0; i < N; i++) {\n \t\tchar name[100];\n \t\tuint8_t hash[GIT_SHA1_RAWSZ];\ndiff --git a/reftable/writer.c b/reftable/writer.c\nindex 35c8649c9b7..e3c042b9d84 100644\n--- a/reftable/writer.c\n+++ b/reftable/writer.c\n@@ -238,14 +238,13 @@ static int writer_add_record(struct reftable_writer *w,\n \n \twriter_reinit_block_writer(w, reftable_record_type(rec));\n \terr = block_writer_add(w->block_writer, rec);\n-\tif (err < 0) {\n+\tif (err == -1) {\n \t\t/* we are writing into memory, so an error can only mean it\n \t\t * doesn't fit. */\n \t\terr = REFTABLE_ENTRY_TOO_BIG_ERROR;\n \t\tgoto done;\n \t}\n \n-\terr = 0;\n done:\n \tstrbuf_release(&key);\n \treturn err;\n-- \ngitgitgadget\n\n"},{"id":"446039","messageId":"3a72aba447c1405595922bc64b7f7a1a873a033a.1642010868.git.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.git.git.1642010868.gitgitgadget@gmail.com","subject":"[PATCH 5/7] reftable: ensure that obj_id_len is >= 2 on writing","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-12T18:07:46Z","receivedAt":"2022-01-12T18:08:27Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nWhen writing the same hash many times, we might decide to use a\nlength-1 object ID prefix for the ObjectID => ref table, which is out\nof spec.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n reftable/readwrite_test.c | 37 +++++++++++++++++++++++++++++++++++++\n reftable/writer.c | 4 +++-\n 2 files changed, 40 insertions(+), 1 deletion(-)\n\ndiff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c\nindex a315c8992e8..b4371b75724 100644\n--- a/reftable/readwrite_test.c\n+++ b/reftable/readwrite_test.c\n@@ -602,6 +602,42 @@ static void test_write_empty_table(void)\n \tstrbuf_release(&buf);\n }\n \n+static void test_write_object_id_min_length(void)\n+{\n+\tstruct reftable_write_options opts = {\n+\t\t.block_size = 75,\n+\t};\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct reftable_writer *w =\n+\t\treftable_new_writer(&strbuf_add_void, &buf, &opts);\n+\tuint8_t hash[GIT_SHA1_RAWSZ] = {42};\n+\tstruct reftable_ref_record ref = {\n+\t\t.update_index = 1,\n+\t\t.value_type = REFTABLE_REF_VAL1,\n+\t\t.value.val1 = hash,\n+\t};\n+\tint err;\n+\tint i;\n+\n+\treftable_writer_set_limits(w, 1, 1);\n+\n+\t/* Write the same hash in many refs. If there is only 1 hash, the\n+\t * disambiguating prefix is length 0 */\n+\tfor (i = 0; i < 256; i++) {\n+\t\tchar name[256];\n+\t\tsnprintf(name, sizeof(name), \"ref%05d\", i);\n+\t\tref.refname = name;\n+\t\terr = reftable_writer_add_ref(w, &ref);\n+\t\tEXPECT_ERR(err);\n+\t}\n+\n+\terr = reftable_writer_close(w);\n+\tEXPECT_ERR(err);\n+\tEXPECT(writer_stats(w)->object_id_len == 2);\n+\treftable_writer_free(w);\n+\tstrbuf_release(&buf);\n+}\n+\n static void test_write_empty_key(void)\n {\n \tstruct reftable_write_options opts = { 0 };\n@@ -707,5 +743,6 @@ int readwrite_test_main(int argc, const char *argv[])\n \tRUN_TEST(test_write_empty_key);\n \tRUN_TEST(test_write_empty_table);\n \tRUN_TEST(test_log_overflow);\n+\tRUN_TEST(test_write_object_id_min_length);\n \treturn 0;\n }\ndiff --git a/reftable/writer.c b/reftable/writer.c\nindex e3c042b9d84..f94af531351 100644\n--- a/reftable/writer.c\n+++ b/reftable/writer.c\n@@ -508,7 +508,9 @@ static void object_record_free(void *void_arg, void *key)\n static int writer_dump_object_index(struct reftable_writer *w)\n {\n \tstruct write_record_arg closure = { .w = w };\n-\tstruct common_prefix_arg common = { NULL };\n+\tstruct common_prefix_arg common = {\n+\t\t.max = 1,\t\t/* obj_id_len should be >= 2. */\n+\t};\n \tif (w->obj_index_tree) {\n \t\tinfix_walk(w->obj_index_tree, &update_common, &common);\n \t}\n-- \ngitgitgadget\n\n"},{"id":"446040","messageId":"a5dfa04888493801037daa8fc27f4e9c6a5fd346.1642010868.git.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.git.git.1642010868.gitgitgadget@gmail.com","subject":"[PATCH 6/7] reftable: add test for length of disambiguating prefix","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-12T18:07:47Z","receivedAt":"2022-01-12T18:08:41Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThe ID => ref map is trimming object IDs to a disambiguating prefix.\nCheck that we are computing their length correctly.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n reftable/readwrite_test.c | 38 ++++++++++++++++++++++++++++++++++++++\n 1 file changed, 38 insertions(+)\n\ndiff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c\nindex b4371b75724..5f45cee39d6 100644\n--- a/reftable/readwrite_test.c\n+++ b/reftable/readwrite_test.c\n@@ -638,6 +638,43 @@ static void test_write_object_id_min_length(void)\n \tstrbuf_release(&buf);\n }\n \n+static void test_write_object_id_length(void)\n+{\n+\tstruct reftable_write_options opts = {\n+\t\t.block_size = 75,\n+\t};\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct reftable_writer *w =\n+\t\treftable_new_writer(&strbuf_add_void, &buf, &opts);\n+\tuint8_t hash[GIT_SHA1_RAWSZ] = {42};\n+\tstruct reftable_ref_record ref = {\n+\t\t.update_index = 1,\n+\t\t.value_type = REFTABLE_REF_VAL1,\n+\t\t.value.val1 = hash,\n+\t};\n+\tint err;\n+\tint i;\n+\n+\treftable_writer_set_limits(w, 1, 1);\n+\n+\t/* Write the same hash in many refs. If there is only 1 hash, the\n+\t * disambiguating prefix is length 0 */\n+\tfor (i = 0; i < 256; i++) {\n+\t\tchar name[256];\n+\t\tsnprintf(name, sizeof(name), \"ref%05d\", i);\n+\t\tref.refname = name;\n+\t\tref.value.val1[15] = i;\n+\t\terr = reftable_writer_add_ref(w, &ref);\n+\t\tEXPECT_ERR(err);\n+\t}\n+\n+\terr = reftable_writer_close(w);\n+\tEXPECT_ERR(err);\n+\tEXPECT(writer_stats(w)->object_id_len == 16);\n+\treftable_writer_free(w);\n+\tstrbuf_release(&buf);\n+}\n+\n static void test_write_empty_key(void)\n {\n \tstruct reftable_write_options opts = { 0 };\n@@ -743,6 +780,7 @@ int readwrite_test_main(int argc, const char *argv[])\n \tRUN_TEST(test_write_empty_key);\n \tRUN_TEST(test_write_empty_table);\n \tRUN_TEST(test_log_overflow);\n+\tRUN_TEST(test_write_object_id_length);\n \tRUN_TEST(test_write_object_id_min_length);\n \treturn 0;\n }\n-- \ngitgitgadget\n\n"},{"id":"446041","messageId":"37aa7744c844f040995eca93d3b794d3af3087a9.1642010868.git.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.git.git.1642010868.gitgitgadget@gmail.com","subject":"[PATCH 7/7] reftable: rename writer_stats to reftable_writer_stats","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-12T18:07:48Z","receivedAt":"2022-01-12T18:09:07Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThis function is part of the reftable API, so it should use the\nreftable_ prefix\n\nSigned-off-by: Han-Wen Nienhuys \n---\n reftable/readwrite_test.c | 8 ++++----\n reftable/reftable-writer.h | 2 +-\n reftable/writer.c | 2 +-\n 3 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c\nindex 5f45cee39d6..b1ff46c18a9 100644\n--- a/reftable/readwrite_test.c\n+++ b/reftable/readwrite_test.c\n@@ -100,7 +100,7 @@ static void write_table(char ***names, struct strbuf *buf, int N,\n \tn = reftable_writer_close(w);\n \tEXPECT(n == 0);\n \n-\tstats = writer_stats(w);\n+\tstats = reftable_writer_stats(w);\n \tfor (i = 0; i < stats->ref_stats.blocks; i++) {\n \t\tint off = i * opts.block_size;\n \t\tif (off == 0) {\n@@ -239,7 +239,7 @@ static void test_log_write_read(void)\n \tn = reftable_writer_close(w);\n \tEXPECT(n == 0);\n \n-\tstats = writer_stats(w);\n+\tstats = reftable_writer_stats(w);\n \tEXPECT(stats->log_stats.blocks > 0);\n \treftable_writer_free(w);\n \tw = NULL;\n@@ -633,7 +633,7 @@ static void test_write_object_id_min_length(void)\n \n \terr = reftable_writer_close(w);\n \tEXPECT_ERR(err);\n-\tEXPECT(writer_stats(w)->object_id_len == 2);\n+\tEXPECT(reftable_writer_stats(w)->object_id_len == 2);\n \treftable_writer_free(w);\n \tstrbuf_release(&buf);\n }\n@@ -670,7 +670,7 @@ static void test_write_object_id_length(void)\n \n \terr = reftable_writer_close(w);\n \tEXPECT_ERR(err);\n-\tEXPECT(writer_stats(w)->object_id_len == 16);\n+\tEXPECT(reftable_writer_stats(w)->object_id_len == 16);\n \treftable_writer_free(w);\n \tstrbuf_release(&buf);\n }\ndiff --git a/reftable/reftable-writer.h b/reftable/reftable-writer.h\nindex a560dc17255..db8de197f6c 100644\n--- a/reftable/reftable-writer.h\n+++ b/reftable/reftable-writer.h\n@@ -143,7 +143,7 @@ int reftable_writer_close(struct reftable_writer *w);\n \n This struct becomes invalid when the writer is freed.\n */\n-const struct reftable_stats *writer_stats(struct reftable_writer *w);\n+const struct reftable_stats *reftable_writer_stats(struct reftable_writer *w);\n \n /* reftable_writer_free deallocates memory for the writer */\n void reftable_writer_free(struct reftable_writer *w);\ndiff --git a/reftable/writer.c b/reftable/writer.c\nindex f94af531351..495784e6294 100644\n--- a/reftable/writer.c\n+++ b/reftable/writer.c\n@@ -688,7 +688,7 @@ static int writer_flush_block(struct reftable_writer *w)\n \treturn writer_flush_nonempty_block(w);\n }\n \n-const struct reftable_stats *writer_stats(struct reftable_writer *w)\n+const struct reftable_stats *reftable_writer_stats(struct reftable_writer *w)\n {\n \treturn &w->stats;\n }\n-- \ngitgitgadget\n"},{"id":"446194","messageId":"xmqqpmovds5q.fsf@gitster.g","threadId":"57229","inReplyTo":"e4c1cc58265ca7ae7b32b9faf41324883011d1a6.1642010868.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 4/7] reftable: avoid writing empty keys at the block layer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-14T01:26:09Z","receivedAt":"2022-01-14T01:26:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Han-Wen Nienhuys via GitGitGadget\" writes:\n\n> diff --git a/reftable/block_test.c b/reftable/block_test.c\n> index 4b3ea262dcb..5112ddbf468 100644\n> --- a/reftable/block_test.c\n> +++ b/reftable/block_test.c\n> @@ -42,6 +42,11 @@ static void test_block_read_write(void)\n> \t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n> \treftable_record_from_ref(&rec, &ref);\n> \n> +\tref.refname = \"\";\n> +\tref.value_type = REFTABLE_REF_DELETION;\n> +\tn = block_writer_add(&bw, &rec);\n> +\tEXPECT(n == REFTABLE_API_ERROR);\n> +\n\nThe preimage of this hunk has been invalidated by your 9c498398\n(reftable: make reftable_record a tagged union, 2021-12-22).\n\nI see that the hn/reftable-coverity-fixes topic, which the commit is\na part of, has been expecting a reroll since last year---are you\nplannning to rebuild that series after landing this series first?\n\n"},{"id":"446357","messageId":"CAFQ2z_NLauUqf8FdWbQVxVmqx5xQ11E5t5SwDDyKyLSa9QU8KA@mail.gmail.com","threadId":"57229","inReplyTo":"xmqqpmovds5q.fsf@gitster.g","subject":"Re: [PATCH 4/7] reftable: avoid writing empty keys at the block layer","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2022-01-17T13:10:42Z","receivedAt":"2022-01-17T13:10:56Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Fri, Jan 14, 2022 at 2:26 AM Junio C Hamano wrote:\n> > + ref.refname = \"\";\n> > + ref.value_type = REFTABLE_REF_DELETION;\n> > + n = block_writer_add(&bw, &rec);\n> > + EXPECT(n == REFTABLE_API_ERROR);\n> > +\n>\n> The preimage of this hunk has been invalidated by your 9c498398\n> (reftable: make reftable_record a tagged union, 2021-12-22).\n>\n> I see that the hn/reftable-coverity-fixes topic, which the commit is\n> a part of, has been expecting a reroll since last year---are you\n\nI sent a reroll on Jan 12. As that has had more scrutiny, it's best if\nthat lands first. This series is less urgent.\n\n> plannning to rebuild that series after landing this series first?\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\n\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\n\nRegistergericht und -nummer: Hamburg, HRB 86891\n\nSitz der Gesellschaft: Hamburg\n\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"446377","messageId":"xmqqlezeuqhx.fsf@gitster.g","threadId":"57229","inReplyTo":"CAFQ2z_NLauUqf8FdWbQVxVmqx5xQ11E5t5SwDDyKyLSa9QU8KA@mail.gmail.com","subject":"Re: [PATCH 4/7] reftable: avoid writing empty keys at the block layer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-17T19:11:22Z","receivedAt":"2022-01-17T19:11:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Han-Wen Nienhuys writes:\n\n>> I see that the hn/reftable-coverity-fixes topic, which the commit is\n>> a part of, has been expecting a reroll since last year---are you\n>\n> I sent a reroll on Jan 12. As that has had more scrutiny, it's best if\n> that lands first. This series is less urgent.\n\nHmph, it probably was missed during pre-release freeze.\n\nhttps://lore.kernel.org/git/e16bf0c5212ae85daa0d6aa2c78d551824b542bd.1640199396.git.gitgitgadget@gmail.com/\n\nnot showing the updated ones did not help, either but stuff outside\nthe upcoming release are not urgent right now, so it is OK.\n\nThanks.\n\n\n"},{"id":"448697","messageId":"pull.1185.v2.git.git.1645106124.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.git.git.1642010868.gitgitgadget@gmail.com","subject":"[PATCH v2 0/7] reftable: avoid reading and writing empty keys","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-17T13:55:17Z","receivedAt":"2022-02-17T13:55:36Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"this series makes sure that the object record does not have to consider\nempty keys (and therefore, a NULL memcpy destination)\n\nwhile we're at it add some more tests, and fix a naming mistake.\n\nHan-Wen Nienhuys (7):\n Documentation: object_id_len goes up to 31\n reftable: reject 0 object_id_len\n reftable: add a test that verifies that writing empty keys fails\n reftable: avoid writing empty keys at the block layer\n reftable: ensure that obj_id_len is >= 2 on writing\n reftable: add test for length of disambiguating prefix\n reftable: rename writer_stats to reftable_writer_stats\n\n Documentation/technical/reftable.txt | 2 +-\n reftable/block.c | 27 ++++---\n reftable/block_test.c | 5 ++\n reftable/reader.c | 5 ++\n reftable/readwrite_test.c | 105 ++++++++++++++++++++++++++-\n reftable/reftable-writer.h | 2 +-\n reftable/writer.c | 9 ++-\n 7 files changed, 136 insertions(+), 19 deletions(-)\n\n\nbase-commit: 45fe28c951c3e70666ee4ef8379772851a8e4d32\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1185%2Fhanwen%2Fobj-id-len-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1185/hanwen/obj-id-len-v2\nPull-Request: https://github.com/git/git/pull/1185\n\nRange-diff vs v1:\n\n 1: 2d2e1177ff7 = 1: 80d29e8f269 Documentation: object_id_len goes up to 31\n 2: 747c9e9a4c8 = 2: 4c1a19fc4ae reftable: reject 0 object_id_len\n 3: 4eefedb0d07 = 3: 600b115f8b1 reftable: add a test that verifies that writing empty keys fails\n 4: e4c1cc58265 ! 4: ba036ee8543 reftable: avoid writing empty keys at the block layer\n @@ reftable/block.c: int block_reader_first_key(struct block_reader *br, struct str\n \n ## reftable/block_test.c ##\n @@ reftable/block_test.c: static void test_block_read_write(void)\n + \tblock_writer_init(&bw, BLOCK_TYPE_REF, block.data, block_size,\n \t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n - \treftable_record_from_ref(&rec, &ref);\n \n -+\tref.refname = \"\";\n -+\tref.value_type = REFTABLE_REF_DELETION;\n ++\trec.u.ref.refname = \"\";\n ++\trec.u.ref.value_type = REFTABLE_REF_DELETION;\n +\tn = block_writer_add(&bw, &rec);\n +\tEXPECT(n == REFTABLE_API_ERROR);\n +\n 5: 3a72aba447c = 5: 2bd3d44ba57 reftable: ensure that obj_id_len is >= 2 on writing\n 6: a5dfa048884 = 6: 82d36ee0e0d reftable: add test for length of disambiguating prefix\n 7: 37aa7744c84 ! 7: c6ffdb3471c reftable: rename writer_stats to reftable_writer_stats\n @@ reftable/readwrite_test.c: static void test_log_write_read(void)\n \tn = reftable_writer_close(w);\n \tEXPECT(n == 0);\n \n +-\tstats = writer_stats(w);\n ++\tstats = reftable_writer_stats(w);\n + \tEXPECT(stats->log_stats.blocks > 0);\n + \treftable_writer_free(w);\n + \tw = NULL;\n +@@ reftable/readwrite_test.c: static void test_log_zlib_corruption(void)\n + \tn = reftable_writer_close(w);\n + \tEXPECT(n == 0);\n + \n -\tstats = writer_stats(w);\n +\tstats = reftable_writer_stats(w);\n \tEXPECT(stats->log_stats.blocks > 0);\n\n-- \ngitgitgadget\n"},{"id":"448698","messageId":"80d29e8f269bf0888d5b1db5f941d1a9bf89c86a.1645106124.git.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.v2.git.git.1645106124.gitgitgadget@gmail.com","subject":"[PATCH v2 1/7] Documentation: object_id_len goes up to 31","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-17T13:55:18Z","receivedAt":"2022-02-17T13:55:38Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThe value is stored in a 5-bit field, so we can't support more without\na format version upgrade.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n Documentation/technical/reftable.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/technical/reftable.txt b/Documentation/technical/reftable.txt\nindex d7c3b645cfb..6a67cc4174f 100644\n--- a/Documentation/technical/reftable.txt\n+++ b/Documentation/technical/reftable.txt\n@@ -443,7 +443,7 @@ Obj block format\n Object blocks are optional. Writers may choose to omit object blocks,\n especially if readers will not use the object name to ref mapping.\n \n-Object blocks use unique, abbreviated 2-32 object name keys, mapping to\n+Object blocks use unique, abbreviated 2-31 byte object name keys, mapping to\n ref blocks containing references pointing to that object directly, or as\n the peeled value of an annotated tag. Like ref blocks, object blocks use\n the file's standard block size. The abbreviation length is available in\n-- \ngitgitgadget\n\n"},{"id":"448699","messageId":"4c1a19fc4aef2742e2733b804221186aa164f721.1645106124.git.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.v2.git.git.1645106124.gitgitgadget@gmail.com","subject":"[PATCH v2 2/7] reftable: reject 0 object_id_len","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-17T13:55:19Z","receivedAt":"2022-02-17T13:55:40Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThe spec says 2 <= object_id_len <= 31. We are lenient and allow 1,\nbut we forbid 0, so we can we can be sure that we never read a\n0-length key.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n reftable/reader.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/reftable/reader.c b/reftable/reader.c\nindex 00906e7a2de..54b4025105c 100644\n--- a/reftable/reader.c\n+++ b/reftable/reader.c\n@@ -155,6 +155,11 @@ static int parse_footer(struct reftable_reader *r, uint8_t *footer,\n \tr->log_offsets.is_present = (first_block_typ == BLOCK_TYPE_LOG ||\n \t\t\t\t r->log_offsets.offset > 0);\n \tr->obj_offsets.is_present = r->obj_offsets.offset > 0;\n+\tif (r->obj_offsets.is_present && !r->object_id_len) {\n+\t\terr = REFTABLE_FORMAT_ERROR;\n+\t\tgoto done;\n+\t}\n+\n \terr = 0;\n done:\n \treturn err;\n-- \ngitgitgadget\n\n"},{"id":"448700","messageId":"600b115f8b17322997bef9092349c13d32b6a121.1645106124.git.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.v2.git.git.1645106124.gitgitgadget@gmail.com","subject":"[PATCH v2 3/7] reftable: add a test that verifies that writing empty keys fails","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-17T13:55:20Z","receivedAt":"2022-02-17T13:55:40Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nEmpty keys can only be written as ref records with empty names. The\nlog record has a logical timestamp in the key, so the key is never\nempty.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n reftable/readwrite_test.c | 24 ++++++++++++++++++++++++\n 1 file changed, 24 insertions(+)\n\ndiff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c\nindex 605ba0f9fd4..fd5922e55f6 100644\n--- a/reftable/readwrite_test.c\n+++ b/reftable/readwrite_test.c\n@@ -667,6 +667,29 @@ static void test_write_empty_table(void)\n \tstrbuf_release(&buf);\n }\n \n+static void test_write_empty_key(void)\n+{\n+\tstruct reftable_write_options opts = { 0 };\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct reftable_writer *w =\n+\t\treftable_new_writer(&strbuf_add_void, &buf, &opts);\n+\tstruct reftable_ref_record ref = {\n+\t\t.refname = \"\",\n+\t\t.update_index = 1,\n+\t\t.value_type = REFTABLE_REF_DELETION,\n+\t};\n+\tint err;\n+\n+\treftable_writer_set_limits(w, 1, 1);\n+\terr = reftable_writer_add_ref(w, &ref);\n+\tEXPECT(err == REFTABLE_API_ERROR);\n+\n+\terr = reftable_writer_close(w);\n+\tEXPECT(err == REFTABLE_EMPTY_TABLE_ERROR);\n+\treftable_writer_free(w);\n+\tstrbuf_release(&buf);\n+}\n+\n static void test_write_key_order(void)\n {\n \tstruct reftable_write_options opts = { 0 };\n@@ -746,6 +769,7 @@ int readwrite_test_main(int argc, const char *argv[])\n \tRUN_TEST(test_table_read_write_seek_index);\n \tRUN_TEST(test_table_refs_for_no_index);\n \tRUN_TEST(test_table_refs_for_obj_index);\n+\tRUN_TEST(test_write_empty_key);\n \tRUN_TEST(test_write_empty_table);\n \tRUN_TEST(test_log_overflow);\n \treturn 0;\n-- \ngitgitgadget\n\n"},{"id":"448701","messageId":"ba036ee8543b2dc28ac046eb0c8c0aef9e751c80.1645106124.git.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.v2.git.git.1645106124.gitgitgadget@gmail.com","subject":"[PATCH v2 4/7] reftable: avoid writing empty keys at the block layer","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-17T13:55:21Z","receivedAt":"2022-02-17T13:55:41Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThe public interface (reftable_writer) already ensures that keys are\nwritten in strictly increasing order, and an empty key by definition\nfails this check.\n\nHowever, by also enforcing this at the block layer, it is easier to\nverify that records (which are written into blocks) never have to\nconsider the possibility of empty keys.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n reftable/block.c | 27 +++++++++++++++++----------\n reftable/block_test.c | 5 +++++\n reftable/writer.c | 3 +--\n 3 files changed, 23 insertions(+), 12 deletions(-)\n\ndiff --git a/reftable/block.c b/reftable/block.c\nindex 2170748c5e9..4a095afe1e2 100644\n--- a/reftable/block.c\n+++ b/reftable/block.c\n@@ -88,8 +88,9 @@ uint8_t block_writer_type(struct block_writer *bw)\n \treturn bw->buf[bw->header_off];\n }\n \n-/* adds the reftable_record to the block. Returns -1 if it does not fit, 0 on\n- success */\n+/* Adds the reftable_record to the block. Returns -1 if it does not fit, 0 on\n+ success. Returns REFTABLE_API_ERROR if attempting to write a record with\n+ empty key. */\n int block_writer_add(struct block_writer *w, struct reftable_record *rec)\n {\n \tstruct strbuf empty = STRBUF_INIT;\n@@ -105,8 +106,14 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec)\n \tint is_restart = 0;\n \tstruct strbuf key = STRBUF_INIT;\n \tint n = 0;\n+\tint err = -1;\n \n \treftable_record_key(rec, &key);\n+\tif (!key.len) {\n+\t\terr = REFTABLE_API_ERROR;\n+\t\tgoto done;\n+\t}\n+\n \tn = reftable_encode_key(&is_restart, out, last, key,\n \t\t\t\treftable_record_val_type(rec));\n \tif (n < 0)\n@@ -118,16 +125,11 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec)\n \t\tgoto done;\n \tstring_view_consume(&out, n);\n \n-\tif (block_writer_register_restart(w, start.len - out.len, is_restart,\n-\t\t\t\t\t &key) < 0)\n-\t\tgoto done;\n-\n-\tstrbuf_release(&key);\n-\treturn 0;\n-\n+\terr = block_writer_register_restart(w, start.len - out.len, is_restart,\n+\t\t\t\t\t &key);\n done:\n \tstrbuf_release(&key);\n-\treturn -1;\n+\treturn err;\n }\n \n int block_writer_finish(struct block_writer *w)\n@@ -332,6 +334,9 @@ int block_iter_next(struct block_iter *it, struct reftable_record *rec)\n \tif (n < 0)\n \t\treturn -1;\n \n+\tif (!key.len)\n+\t\treturn REFTABLE_FORMAT_ERROR;\n+\n \tstring_view_consume(&in, n);\n \tn = reftable_record_decode(rec, key, extra, in, it->br->hash_size);\n \tif (n < 0)\n@@ -358,6 +363,8 @@ int block_reader_first_key(struct block_reader *br, struct strbuf *key)\n \tint n = reftable_decode_key(key, &extra, empty, in);\n \tif (n < 0)\n \t\treturn n;\n+\tif (!key->len)\n+\t\treturn -1;\n \n \treturn 0;\n }\ndiff --git a/reftable/block_test.c b/reftable/block_test.c\nindex fa2ee092ec0..cb88af4a563 100644\n--- a/reftable/block_test.c\n+++ b/reftable/block_test.c\n@@ -42,6 +42,11 @@ static void test_block_read_write(void)\n \tblock_writer_init(&bw, BLOCK_TYPE_REF, block.data, block_size,\n \t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n \n+\trec.u.ref.refname = \"\";\n+\trec.u.ref.value_type = REFTABLE_REF_DELETION;\n+\tn = block_writer_add(&bw, &rec);\n+\tEXPECT(n == REFTABLE_API_ERROR);\n+\n \tfor (i = 0; i < N; i++) {\n \t\tchar name[100];\n \t\tuint8_t hash[GIT_SHA1_RAWSZ];\ndiff --git a/reftable/writer.c b/reftable/writer.c\nindex 944c2329ab5..d54215a50dc 100644\n--- a/reftable/writer.c\n+++ b/reftable/writer.c\n@@ -240,14 +240,13 @@ static int writer_add_record(struct reftable_writer *w,\n \n \twriter_reinit_block_writer(w, reftable_record_type(rec));\n \terr = block_writer_add(w->block_writer, rec);\n-\tif (err < 0) {\n+\tif (err == -1) {\n \t\t/* we are writing into memory, so an error can only mean it\n \t\t * doesn't fit. */\n \t\terr = REFTABLE_ENTRY_TOO_BIG_ERROR;\n \t\tgoto done;\n \t}\n \n-\terr = 0;\n done:\n \tstrbuf_release(&key);\n \treturn err;\n-- \ngitgitgadget\n\n"},{"id":"448702","messageId":"2bd3d44ba57ddb43c09367b45a8f056233d465e9.1645106124.git.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.v2.git.git.1645106124.gitgitgadget@gmail.com","subject":"[PATCH v2 5/7] reftable: ensure that obj_id_len is >= 2 on writing","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-17T13:55:22Z","receivedAt":"2022-02-17T13:55:43Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nWhen writing the same hash many times, we might decide to use a\nlength-1 object ID prefix for the ObjectID => ref table, which is out\nof spec.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n reftable/readwrite_test.c | 37 +++++++++++++++++++++++++++++++++++++\n reftable/writer.c | 4 +++-\n 2 files changed, 40 insertions(+), 1 deletion(-)\n\ndiff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c\nindex fd5922e55f6..35142eb070e 100644\n--- a/reftable/readwrite_test.c\n+++ b/reftable/readwrite_test.c\n@@ -667,6 +667,42 @@ static void test_write_empty_table(void)\n \tstrbuf_release(&buf);\n }\n \n+static void test_write_object_id_min_length(void)\n+{\n+\tstruct reftable_write_options opts = {\n+\t\t.block_size = 75,\n+\t};\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct reftable_writer *w =\n+\t\treftable_new_writer(&strbuf_add_void, &buf, &opts);\n+\tuint8_t hash[GIT_SHA1_RAWSZ] = {42};\n+\tstruct reftable_ref_record ref = {\n+\t\t.update_index = 1,\n+\t\t.value_type = REFTABLE_REF_VAL1,\n+\t\t.value.val1 = hash,\n+\t};\n+\tint err;\n+\tint i;\n+\n+\treftable_writer_set_limits(w, 1, 1);\n+\n+\t/* Write the same hash in many refs. If there is only 1 hash, the\n+\t * disambiguating prefix is length 0 */\n+\tfor (i = 0; i < 256; i++) {\n+\t\tchar name[256];\n+\t\tsnprintf(name, sizeof(name), \"ref%05d\", i);\n+\t\tref.refname = name;\n+\t\terr = reftable_writer_add_ref(w, &ref);\n+\t\tEXPECT_ERR(err);\n+\t}\n+\n+\terr = reftable_writer_close(w);\n+\tEXPECT_ERR(err);\n+\tEXPECT(writer_stats(w)->object_id_len == 2);\n+\treftable_writer_free(w);\n+\tstrbuf_release(&buf);\n+}\n+\n static void test_write_empty_key(void)\n {\n \tstruct reftable_write_options opts = { 0 };\n@@ -772,5 +808,6 @@ int readwrite_test_main(int argc, const char *argv[])\n \tRUN_TEST(test_write_empty_key);\n \tRUN_TEST(test_write_empty_table);\n \tRUN_TEST(test_log_overflow);\n+\tRUN_TEST(test_write_object_id_min_length);\n \treturn 0;\n }\ndiff --git a/reftable/writer.c b/reftable/writer.c\nindex d54215a50dc..5e4e6e93416 100644\n--- a/reftable/writer.c\n+++ b/reftable/writer.c\n@@ -515,7 +515,9 @@ static void object_record_free(void *void_arg, void *key)\n static int writer_dump_object_index(struct reftable_writer *w)\n {\n \tstruct write_record_arg closure = { .w = w };\n-\tstruct common_prefix_arg common = { NULL };\n+\tstruct common_prefix_arg common = {\n+\t\t.max = 1,\t\t/* obj_id_len should be >= 2. */\n+\t};\n \tif (w->obj_index_tree) {\n \t\tinfix_walk(w->obj_index_tree, &update_common, &common);\n \t}\n-- \ngitgitgadget\n\n"},{"id":"448703","messageId":"82d36ee0e0d7fedd40e4cc9013dbf469218fb68f.1645106124.git.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.v2.git.git.1645106124.gitgitgadget@gmail.com","subject":"[PATCH v2 6/7] reftable: add test for length of disambiguating prefix","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-17T13:55:23Z","receivedAt":"2022-02-17T13:55:47Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThe ID => ref map is trimming object IDs to a disambiguating prefix.\nCheck that we are computing their length correctly.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n reftable/readwrite_test.c | 38 ++++++++++++++++++++++++++++++++++++++\n 1 file changed, 38 insertions(+)\n\ndiff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c\nindex 35142eb070e..a1b835785a3 100644\n--- a/reftable/readwrite_test.c\n+++ b/reftable/readwrite_test.c\n@@ -703,6 +703,43 @@ static void test_write_object_id_min_length(void)\n \tstrbuf_release(&buf);\n }\n \n+static void test_write_object_id_length(void)\n+{\n+\tstruct reftable_write_options opts = {\n+\t\t.block_size = 75,\n+\t};\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct reftable_writer *w =\n+\t\treftable_new_writer(&strbuf_add_void, &buf, &opts);\n+\tuint8_t hash[GIT_SHA1_RAWSZ] = {42};\n+\tstruct reftable_ref_record ref = {\n+\t\t.update_index = 1,\n+\t\t.value_type = REFTABLE_REF_VAL1,\n+\t\t.value.val1 = hash,\n+\t};\n+\tint err;\n+\tint i;\n+\n+\treftable_writer_set_limits(w, 1, 1);\n+\n+\t/* Write the same hash in many refs. If there is only 1 hash, the\n+\t * disambiguating prefix is length 0 */\n+\tfor (i = 0; i < 256; i++) {\n+\t\tchar name[256];\n+\t\tsnprintf(name, sizeof(name), \"ref%05d\", i);\n+\t\tref.refname = name;\n+\t\tref.value.val1[15] = i;\n+\t\terr = reftable_writer_add_ref(w, &ref);\n+\t\tEXPECT_ERR(err);\n+\t}\n+\n+\terr = reftable_writer_close(w);\n+\tEXPECT_ERR(err);\n+\tEXPECT(writer_stats(w)->object_id_len == 16);\n+\treftable_writer_free(w);\n+\tstrbuf_release(&buf);\n+}\n+\n static void test_write_empty_key(void)\n {\n \tstruct reftable_write_options opts = { 0 };\n@@ -808,6 +845,7 @@ int readwrite_test_main(int argc, const char *argv[])\n \tRUN_TEST(test_write_empty_key);\n \tRUN_TEST(test_write_empty_table);\n \tRUN_TEST(test_log_overflow);\n+\tRUN_TEST(test_write_object_id_length);\n \tRUN_TEST(test_write_object_id_min_length);\n \treturn 0;\n }\n-- \ngitgitgadget\n\n"},{"id":"448704","messageId":"c6ffdb3471c1280a7f3a4293e95d666cf839b192.1645106124.git.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.v2.git.git.1645106124.gitgitgadget@gmail.com","subject":"[PATCH v2 7/7] reftable: rename writer_stats to reftable_writer_stats","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-17T13:55:24Z","receivedAt":"2022-02-17T13:55:51Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThis function is part of the reftable API, so it should use the\nreftable_ prefix\n\nSigned-off-by: Han-Wen Nienhuys \n---\n reftable/readwrite_test.c | 10 +++++-----\n reftable/reftable-writer.h | 2 +-\n reftable/writer.c | 2 +-\n 3 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c\nindex a1b835785a3..469ab79a5ad 100644\n--- a/reftable/readwrite_test.c\n+++ b/reftable/readwrite_test.c\n@@ -100,7 +100,7 @@ static void write_table(char ***names, struct strbuf *buf, int N,\n \tn = reftable_writer_close(w);\n \tEXPECT(n == 0);\n \n-\tstats = writer_stats(w);\n+\tstats = reftable_writer_stats(w);\n \tfor (i = 0; i < stats->ref_stats.blocks; i++) {\n \t\tint off = i * opts.block_size;\n \t\tif (off == 0) {\n@@ -239,7 +239,7 @@ static void test_log_write_read(void)\n \tn = reftable_writer_close(w);\n \tEXPECT(n == 0);\n \n-\tstats = writer_stats(w);\n+\tstats = reftable_writer_stats(w);\n \tEXPECT(stats->log_stats.blocks > 0);\n \treftable_writer_free(w);\n \tw = NULL;\n@@ -330,7 +330,7 @@ static void test_log_zlib_corruption(void)\n \tn = reftable_writer_close(w);\n \tEXPECT(n == 0);\n \n-\tstats = writer_stats(w);\n+\tstats = reftable_writer_stats(w);\n \tEXPECT(stats->log_stats.blocks > 0);\n \treftable_writer_free(w);\n \tw = NULL;\n@@ -698,7 +698,7 @@ static void test_write_object_id_min_length(void)\n \n \terr = reftable_writer_close(w);\n \tEXPECT_ERR(err);\n-\tEXPECT(writer_stats(w)->object_id_len == 2);\n+\tEXPECT(reftable_writer_stats(w)->object_id_len == 2);\n \treftable_writer_free(w);\n \tstrbuf_release(&buf);\n }\n@@ -735,7 +735,7 @@ static void test_write_object_id_length(void)\n \n \terr = reftable_writer_close(w);\n \tEXPECT_ERR(err);\n-\tEXPECT(writer_stats(w)->object_id_len == 16);\n+\tEXPECT(reftable_writer_stats(w)->object_id_len == 16);\n \treftable_writer_free(w);\n \tstrbuf_release(&buf);\n }\ndiff --git a/reftable/reftable-writer.h b/reftable/reftable-writer.h\nindex a560dc17255..db8de197f6c 100644\n--- a/reftable/reftable-writer.h\n+++ b/reftable/reftable-writer.h\n@@ -143,7 +143,7 @@ int reftable_writer_close(struct reftable_writer *w);\n \n This struct becomes invalid when the writer is freed.\n */\n-const struct reftable_stats *writer_stats(struct reftable_writer *w);\n+const struct reftable_stats *reftable_writer_stats(struct reftable_writer *w);\n \n /* reftable_writer_free deallocates memory for the writer */\n void reftable_writer_free(struct reftable_writer *w);\ndiff --git a/reftable/writer.c b/reftable/writer.c\nindex 5e4e6e93416..6d979e245ff 100644\n--- a/reftable/writer.c\n+++ b/reftable/writer.c\n@@ -695,7 +695,7 @@ static int writer_flush_block(struct reftable_writer *w)\n \treturn writer_flush_nonempty_block(w);\n }\n \n-const struct reftable_stats *writer_stats(struct reftable_writer *w)\n+const struct reftable_stats *reftable_writer_stats(struct reftable_writer *w)\n {\n \treturn &w->stats;\n }\n-- \ngitgitgadget\n"},{"id":"448749","messageId":"xmqqee4159r6.fsf@gitster.g","threadId":"57229","inReplyTo":"ba036ee8543b2dc28ac046eb0c8c0aef9e751c80.1645106124.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 4/7] reftable: avoid writing empty keys at the block layer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-17T23:55:09Z","receivedAt":"2022-02-17T23:55:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Han-Wen Nienhuys via GitGitGadget\" writes:\n\n> @@ -105,8 +106,14 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec)\n> \tint is_restart = 0;\n> \tstruct strbuf key = STRBUF_INIT;\n> \tint n = 0;\n> +\tint err = -1;\n> \n> \treftable_record_key(rec, &key);\n> +\tif (!key.len) {\n> +\t\terr = REFTABLE_API_ERROR;\n> +\t\tgoto done;\n> +\t}\n\nOK; we get an API_ERROR when trying to write a bad one. And ...\n\n> @@ -332,6 +334,9 @@ int block_iter_next(struct block_iter *it, struct reftable_record *rec)\n> \tif (n < 0)\n> \t\treturn -1;\n> \n> +\tif (!key.len)\n> +\t\treturn REFTABLE_FORMAT_ERROR;\n\n... we get a FORMAT_ERROR when the data we try to read is bad\n(i.e. not our fault). OK.\n\n> @@ -358,6 +363,8 @@ int block_reader_first_key(struct block_reader *br, struct strbuf *key)\n> \tint n = reftable_decode_key(key, &extra, empty, in);\n> \tif (n < 0)\n> \t\treturn n;\n> +\tif (!key->len)\n> +\t\treturn -1;\n\nIt is curious that this gets a different error out of the same\nsequence, i.e. decode-key did not return an error but the length of\nthe key happens to be 0, not FORMAT_ERROR.\n\n> diff --git a/reftable/writer.c b/reftable/writer.c\n> index 944c2329ab5..d54215a50dc 100644\n> --- a/reftable/writer.c\n> +++ b/reftable/writer.c\n> @@ -240,14 +240,13 @@ static int writer_add_record(struct reftable_writer *w,\n> \n> \twriter_reinit_block_writer(w, reftable_record_type(rec));\n> \terr = block_writer_add(w->block_writer, rec);\n> -\tif (err < 0) {\n> +\tif (err == -1) {\n> \t\t/* we are writing into memory, so an error can only mean it\n> \t\t * doesn't fit. */\n> \t\terr = REFTABLE_ENTRY_TOO_BIG_ERROR;\n> \t\tgoto done;\n> \t}\n> \n> -\terr = 0;\n\nIs this \"doesn't fit\" related to \"we catch 0-length keys\", or an\nunrelated fix was included in this step by \"rebase -i\" mistake?\n\n"},{"id":"448750","messageId":"xmqq8ru959fz.fsf@gitster.g","threadId":"57229","inReplyTo":"2bd3d44ba57ddb43c09367b45a8f056233d465e9.1645106124.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 5/7] reftable: ensure that obj_id_len is >= 2 on writing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-18T00:01:52Z","receivedAt":"2022-02-18T00:02:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Han-Wen Nienhuys via GitGitGadget\" writes:\n\n> diff --git a/reftable/writer.c b/reftable/writer.c\n> index d54215a50dc..5e4e6e93416 100644\n> --- a/reftable/writer.c\n> +++ b/reftable/writer.c\n> @@ -515,7 +515,9 @@ static void object_record_free(void *void_arg, void *key)\n> static int writer_dump_object_index(struct reftable_writer *w)\n> {\n> \tstruct write_record_arg closure = { .w = w };\n> -\tstruct common_prefix_arg common = { NULL };\n> +\tstruct common_prefix_arg common = {\n> +\t\t.max = 1,\t\t/* obj_id_len should be >= 2. */\n> +\t};\n\nIt feels somewhat strange that we have to set .max to set the floor\nfor the minimum length, but given the way update_common() uses and\nmaintains the member, it is more like \"max size we have seen so\nfar\", and by pretending that we have already seen common prefix of\nlength 1, we'd force ourselves that we need at least 2 to\ndifferentiate.\n\n> \tif (w->obj_index_tree) {\n> \t\tinfix_walk(w->obj_index_tree, &update_common, &common);\n> \t}\n"},{"id":"448751","messageId":"xmqq4k4x59e9.fsf@gitster.g","threadId":"57229","inReplyTo":"pull.1185.v2.git.git.1645106124.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/7] reftable: avoid reading and writing empty keys","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-18T00:02:54Z","receivedAt":"2022-02-18T00:03:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Han-Wen Nienhuys via GitGitGadget\" writes:\n\n> this series makes sure that the object record does not have to consider\n> empty keys (and therefore, a NULL memcpy destination)\n>\n> while we're at it add some more tests, and fix a naming mistake.\n\nLooking good. Will queue.\n\nThanks.\n"},{"id":"448752","messageId":"xmqqwnht3tgt.fsf@gitster.g","threadId":"57229","inReplyTo":"4c1a19fc4aef2742e2733b804221186aa164f721.1645106124.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 2/7] reftable: reject 0 object_id_len","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-18T00:32:18Z","receivedAt":"2022-02-18T00:34:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Han-Wen Nienhuys via GitGitGadget\" writes:\n\n> From: Han-Wen Nienhuys \n>\n> The spec says 2 <= object_id_len <= 31. We are lenient and allow 1,\n> but we forbid 0, so we can we can be sure that we never read a\n\ns/we can we can/we can/;\n\n> 0-length key.\n>\n> Signed-off-by: Han-Wen Nienhuys \n> ---\n> reftable/reader.c | 5 +++++\n> 1 file changed, 5 insertions(+)\n>\n> diff --git a/reftable/reader.c b/reftable/reader.c\n> index 00906e7a2de..54b4025105c 100644\n> --- a/reftable/reader.c\n> +++ b/reftable/reader.c\n> @@ -155,6 +155,11 @@ static int parse_footer(struct reftable_reader *r, uint8_t *footer,\n> \tr->log_offsets.is_present = (first_block_typ == BLOCK_TYPE_LOG ||\n> \t\t\t\t r->log_offsets.offset > 0);\n> \tr->obj_offsets.is_present = r->obj_offsets.offset > 0;\n> +\tif (r->obj_offsets.is_present && !r->object_id_len) {\n> +\t\terr = REFTABLE_FORMAT_ERROR;\n> +\t\tgoto done;\n> +\t}\n> +\n> \terr = 0;\n> done:\n> \treturn err;\n"},{"id":"448978","messageId":"CAFQ2z_OkZwvvxY=9A8cVGVEyM49oWXQA_4zngu1QnKce-zb2gQ@mail.gmail.com","threadId":"57229","inReplyTo":"xmqqee4159r6.fsf@gitster.g","subject":"Re: [PATCH v2 4/7] reftable: avoid writing empty keys at the block layer","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2022-02-21T14:32:11Z","receivedAt":"2022-02-21T14:33:24Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Fri, Feb 18, 2022 at 12:55 AM Junio C Hamano wrote:\n> > @@ -358,6 +363,8 @@ int block_reader_first_key(struct block_reader *br, struct strbuf *key)\n> > int n = reftable_decode_key(key, &extra, empty, in);\n> > if (n < 0)\n> > return n;\n> > + if (!key->len)\n> > + return -1;\n>\n> It is curious that this gets a different error out of the same\n> sequence, i.e. decode-key did not return an error but the length of\n> the key happens to be 0, not FORMAT_ERROR.\n\nfixed.\n\n> > --- a/reftable/writer.c\n> > +++ b/reftable/writer.c\n> > @@ -240,14 +240,13 @@ static int writer_add_record(struct reftable_writer *w,\n> >\n> > writer_reinit_block_writer(w, reftable_record_type(rec));\n> > err = block_writer_add(w->block_writer, rec);\n> > - if (err < 0) {\n> > + if (err == -1) {\n> > /* we are writing into memory, so an error can only mean it\n> > * doesn't fit. */\n> > err = REFTABLE_ENTRY_TOO_BIG_ERROR;\n> > goto done;\n> > }\n> >\n> > - err = 0;\n>\n> Is this \"doesn't fit\" related to \"we catch 0-length keys\", or an\n> unrelated fix was included in this step by \"rebase -i\" mistake?\n\nWe don't want to reinterpret API_ERROR (from block_writer_add) as\nENTRY_TOO_BIG_ERROR, so we have to tweak the condition here.\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\n\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\n\nRegistergericht und -nummer: Hamburg, HRB 86891\n\nSitz der Gesellschaft: Hamburg\n\nGeschäftsführer: Paul Manicle, Liana Sebastian\n"},{"id":"449028","messageId":"pull.1185.v3.git.git.1645469170.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.v2.git.git.1645106124.gitgitgadget@gmail.com","subject":"[PATCH v3 0/7] reftable: avoid reading and writing empty keys","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-21T18:46:03Z","receivedAt":"2022-02-21T18:46:21Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"this series makes sure that the object record does not have to consider\nempty keys (and therefore, a NULL memcpy destination)\n\nwhile we're at it add some more tests, and fix a naming mistake.\n\nHan-Wen Nienhuys (7):\n Documentation: object_id_len goes up to 31\n reftable: reject 0 object_id_len\n reftable: add a test that verifies that writing empty keys fails\n reftable: avoid writing empty keys at the block layer\n reftable: ensure that obj_id_len is >= 2 on writing\n reftable: add test for length of disambiguating prefix\n reftable: rename writer_stats to reftable_writer_stats\n\n Documentation/technical/reftable.txt | 2 +-\n reftable/block.c | 27 ++++---\n reftable/block_test.c | 5 ++\n reftable/reader.c | 5 ++\n reftable/readwrite_test.c | 105 ++++++++++++++++++++++++++-\n reftable/reftable-writer.h | 2 +-\n reftable/writer.c | 9 ++-\n 7 files changed, 136 insertions(+), 19 deletions(-)\n\n\nbase-commit: 45fe28c951c3e70666ee4ef8379772851a8e4d32\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1185%2Fhanwen%2Fobj-id-len-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1185/hanwen/obj-id-len-v3\nPull-Request: https://github.com/git/git/pull/1185\n\nRange-diff vs v2:\n\n 1: 80d29e8f269 = 1: 80d29e8f269 Documentation: object_id_len goes up to 31\n 2: 4c1a19fc4ae ! 2: 68e7bc32ff8 reftable: reject 0 object_id_len\n @@ Commit message\n reftable: reject 0 object_id_len\n \n The spec says 2 <= object_id_len <= 31. We are lenient and allow 1,\n - but we forbid 0, so we can we can be sure that we never read a\n - 0-length key.\n + but we forbid 0, so we can be sure that we never read a 0-length key.\n \n Signed-off-by: Han-Wen Nienhuys \n \n 3: 600b115f8b1 = 3: 8b5aebdb07a reftable: add a test that verifies that writing empty keys fails\n 4: ba036ee8543 ! 4: a9372cacd1b reftable: avoid writing empty keys at the block layer\n @@ reftable/block.c: int block_reader_first_key(struct block_reader *br, struct str\n \tif (n < 0)\n \t\treturn n;\n +\tif (!key->len)\n -+\t\treturn -1;\n ++\t\treturn REFTABLE_FORMAT_ERROR;\n \n \treturn 0;\n }\n 5: 2bd3d44ba57 = 5: 0b8a42399dd reftable: ensure that obj_id_len is >= 2 on writing\n 6: 82d36ee0e0d = 6: bdccd969475 reftable: add test for length of disambiguating prefix\n 7: c6ffdb3471c = 7: 72499a14e38 reftable: rename writer_stats to reftable_writer_stats\n\n-- \ngitgitgadget\n"},{"id":"449029","messageId":"68e7bc32ff88da04c66993f68a84bbb401bc54c8.1645469170.git.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.v3.git.git.1645469170.gitgitgadget@gmail.com","subject":"[PATCH v3 2/7] reftable: reject 0 object_id_len","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-21T18:46:05Z","receivedAt":"2022-02-21T18:46:21Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThe spec says 2 <= object_id_len <= 31. We are lenient and allow 1,\nbut we forbid 0, so we can be sure that we never read a 0-length key.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n reftable/reader.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/reftable/reader.c b/reftable/reader.c\nindex 00906e7a2de..54b4025105c 100644\n--- a/reftable/reader.c\n+++ b/reftable/reader.c\n@@ -155,6 +155,11 @@ static int parse_footer(struct reftable_reader *r, uint8_t *footer,\n \tr->log_offsets.is_present = (first_block_typ == BLOCK_TYPE_LOG ||\n \t\t\t\t r->log_offsets.offset > 0);\n \tr->obj_offsets.is_present = r->obj_offsets.offset > 0;\n+\tif (r->obj_offsets.is_present && !r->object_id_len) {\n+\t\terr = REFTABLE_FORMAT_ERROR;\n+\t\tgoto done;\n+\t}\n+\n \terr = 0;\n done:\n \treturn err;\n-- \ngitgitgadget\n\n"},{"id":"449030","messageId":"80d29e8f269bf0888d5b1db5f941d1a9bf89c86a.1645469170.git.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.v3.git.git.1645469170.gitgitgadget@gmail.com","subject":"[PATCH v3 1/7] Documentation: object_id_len goes up to 31","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-21T18:46:04Z","receivedAt":"2022-02-21T18:46:25Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThe value is stored in a 5-bit field, so we can't support more without\na format version upgrade.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n Documentation/technical/reftable.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/technical/reftable.txt b/Documentation/technical/reftable.txt\nindex d7c3b645cfb..6a67cc4174f 100644\n--- a/Documentation/technical/reftable.txt\n+++ b/Documentation/technical/reftable.txt\n@@ -443,7 +443,7 @@ Obj block format\n Object blocks are optional. Writers may choose to omit object blocks,\n especially if readers will not use the object name to ref mapping.\n \n-Object blocks use unique, abbreviated 2-32 object name keys, mapping to\n+Object blocks use unique, abbreviated 2-31 byte object name keys, mapping to\n ref blocks containing references pointing to that object directly, or as\n the peeled value of an annotated tag. Like ref blocks, object blocks use\n the file's standard block size. The abbreviation length is available in\n-- \ngitgitgadget\n\n"},{"id":"449031","messageId":"a9372cacd1b7f62530df948c0ba99cf276d34cad.1645469170.git.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.v3.git.git.1645469170.gitgitgadget@gmail.com","subject":"[PATCH v3 4/7] reftable: avoid writing empty keys at the block layer","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-21T18:46:07Z","receivedAt":"2022-02-21T18:46:37Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThe public interface (reftable_writer) already ensures that keys are\nwritten in strictly increasing order, and an empty key by definition\nfails this check.\n\nHowever, by also enforcing this at the block layer, it is easier to\nverify that records (which are written into blocks) never have to\nconsider the possibility of empty keys.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n reftable/block.c | 27 +++++++++++++++++----------\n reftable/block_test.c | 5 +++++\n reftable/writer.c | 3 +--\n 3 files changed, 23 insertions(+), 12 deletions(-)\n\ndiff --git a/reftable/block.c b/reftable/block.c\nindex 2170748c5e9..34d4d073692 100644\n--- a/reftable/block.c\n+++ b/reftable/block.c\n@@ -88,8 +88,9 @@ uint8_t block_writer_type(struct block_writer *bw)\n \treturn bw->buf[bw->header_off];\n }\n \n-/* adds the reftable_record to the block. Returns -1 if it does not fit, 0 on\n- success */\n+/* Adds the reftable_record to the block. Returns -1 if it does not fit, 0 on\n+ success. Returns REFTABLE_API_ERROR if attempting to write a record with\n+ empty key. */\n int block_writer_add(struct block_writer *w, struct reftable_record *rec)\n {\n \tstruct strbuf empty = STRBUF_INIT;\n@@ -105,8 +106,14 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec)\n \tint is_restart = 0;\n \tstruct strbuf key = STRBUF_INIT;\n \tint n = 0;\n+\tint err = -1;\n \n \treftable_record_key(rec, &key);\n+\tif (!key.len) {\n+\t\terr = REFTABLE_API_ERROR;\n+\t\tgoto done;\n+\t}\n+\n \tn = reftable_encode_key(&is_restart, out, last, key,\n \t\t\t\treftable_record_val_type(rec));\n \tif (n < 0)\n@@ -118,16 +125,11 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec)\n \t\tgoto done;\n \tstring_view_consume(&out, n);\n \n-\tif (block_writer_register_restart(w, start.len - out.len, is_restart,\n-\t\t\t\t\t &key) < 0)\n-\t\tgoto done;\n-\n-\tstrbuf_release(&key);\n-\treturn 0;\n-\n+\terr = block_writer_register_restart(w, start.len - out.len, is_restart,\n+\t\t\t\t\t &key);\n done:\n \tstrbuf_release(&key);\n-\treturn -1;\n+\treturn err;\n }\n \n int block_writer_finish(struct block_writer *w)\n@@ -332,6 +334,9 @@ int block_iter_next(struct block_iter *it, struct reftable_record *rec)\n \tif (n < 0)\n \t\treturn -1;\n \n+\tif (!key.len)\n+\t\treturn REFTABLE_FORMAT_ERROR;\n+\n \tstring_view_consume(&in, n);\n \tn = reftable_record_decode(rec, key, extra, in, it->br->hash_size);\n \tif (n < 0)\n@@ -358,6 +363,8 @@ int block_reader_first_key(struct block_reader *br, struct strbuf *key)\n \tint n = reftable_decode_key(key, &extra, empty, in);\n \tif (n < 0)\n \t\treturn n;\n+\tif (!key->len)\n+\t\treturn REFTABLE_FORMAT_ERROR;\n \n \treturn 0;\n }\ndiff --git a/reftable/block_test.c b/reftable/block_test.c\nindex fa2ee092ec0..cb88af4a563 100644\n--- a/reftable/block_test.c\n+++ b/reftable/block_test.c\n@@ -42,6 +42,11 @@ static void test_block_read_write(void)\n \tblock_writer_init(&bw, BLOCK_TYPE_REF, block.data, block_size,\n \t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n \n+\trec.u.ref.refname = \"\";\n+\trec.u.ref.value_type = REFTABLE_REF_DELETION;\n+\tn = block_writer_add(&bw, &rec);\n+\tEXPECT(n == REFTABLE_API_ERROR);\n+\n \tfor (i = 0; i < N; i++) {\n \t\tchar name[100];\n \t\tuint8_t hash[GIT_SHA1_RAWSZ];\ndiff --git a/reftable/writer.c b/reftable/writer.c\nindex 944c2329ab5..d54215a50dc 100644\n--- a/reftable/writer.c\n+++ b/reftable/writer.c\n@@ -240,14 +240,13 @@ static int writer_add_record(struct reftable_writer *w,\n \n \twriter_reinit_block_writer(w, reftable_record_type(rec));\n \terr = block_writer_add(w->block_writer, rec);\n-\tif (err < 0) {\n+\tif (err == -1) {\n \t\t/* we are writing into memory, so an error can only mean it\n \t\t * doesn't fit. */\n \t\terr = REFTABLE_ENTRY_TOO_BIG_ERROR;\n \t\tgoto done;\n \t}\n \n-\terr = 0;\n done:\n \tstrbuf_release(&key);\n \treturn err;\n-- \ngitgitgadget\n\n"},{"id":"449032","messageId":"8b5aebdb07a6182196217f6750fbda95c17d1402.1645469170.git.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.v3.git.git.1645469170.gitgitgadget@gmail.com","subject":"[PATCH v3 3/7] reftable: add a test that verifies that writing empty keys fails","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-21T18:46:06Z","receivedAt":"2022-02-21T18:46:39Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nEmpty keys can only be written as ref records with empty names. The\nlog record has a logical timestamp in the key, so the key is never\nempty.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n reftable/readwrite_test.c | 24 ++++++++++++++++++++++++\n 1 file changed, 24 insertions(+)\n\ndiff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c\nindex 605ba0f9fd4..fd5922e55f6 100644\n--- a/reftable/readwrite_test.c\n+++ b/reftable/readwrite_test.c\n@@ -667,6 +667,29 @@ static void test_write_empty_table(void)\n \tstrbuf_release(&buf);\n }\n \n+static void test_write_empty_key(void)\n+{\n+\tstruct reftable_write_options opts = { 0 };\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct reftable_writer *w =\n+\t\treftable_new_writer(&strbuf_add_void, &buf, &opts);\n+\tstruct reftable_ref_record ref = {\n+\t\t.refname = \"\",\n+\t\t.update_index = 1,\n+\t\t.value_type = REFTABLE_REF_DELETION,\n+\t};\n+\tint err;\n+\n+\treftable_writer_set_limits(w, 1, 1);\n+\terr = reftable_writer_add_ref(w, &ref);\n+\tEXPECT(err == REFTABLE_API_ERROR);\n+\n+\terr = reftable_writer_close(w);\n+\tEXPECT(err == REFTABLE_EMPTY_TABLE_ERROR);\n+\treftable_writer_free(w);\n+\tstrbuf_release(&buf);\n+}\n+\n static void test_write_key_order(void)\n {\n \tstruct reftable_write_options opts = { 0 };\n@@ -746,6 +769,7 @@ int readwrite_test_main(int argc, const char *argv[])\n \tRUN_TEST(test_table_read_write_seek_index);\n \tRUN_TEST(test_table_refs_for_no_index);\n \tRUN_TEST(test_table_refs_for_obj_index);\n+\tRUN_TEST(test_write_empty_key);\n \tRUN_TEST(test_write_empty_table);\n \tRUN_TEST(test_log_overflow);\n \treturn 0;\n-- \ngitgitgadget\n\n"},{"id":"449033","messageId":"0b8a42399dd7aa04fdc791d25969a3b085190c6f.1645469170.git.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.v3.git.git.1645469170.gitgitgadget@gmail.com","subject":"[PATCH v3 5/7] reftable: ensure that obj_id_len is >= 2 on writing","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-21T18:46:08Z","receivedAt":"2022-02-21T18:46:40Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nWhen writing the same hash many times, we might decide to use a\nlength-1 object ID prefix for the ObjectID => ref table, which is out\nof spec.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n reftable/readwrite_test.c | 37 +++++++++++++++++++++++++++++++++++++\n reftable/writer.c | 4 +++-\n 2 files changed, 40 insertions(+), 1 deletion(-)\n\ndiff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c\nindex fd5922e55f6..35142eb070e 100644\n--- a/reftable/readwrite_test.c\n+++ b/reftable/readwrite_test.c\n@@ -667,6 +667,42 @@ static void test_write_empty_table(void)\n \tstrbuf_release(&buf);\n }\n \n+static void test_write_object_id_min_length(void)\n+{\n+\tstruct reftable_write_options opts = {\n+\t\t.block_size = 75,\n+\t};\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct reftable_writer *w =\n+\t\treftable_new_writer(&strbuf_add_void, &buf, &opts);\n+\tuint8_t hash[GIT_SHA1_RAWSZ] = {42};\n+\tstruct reftable_ref_record ref = {\n+\t\t.update_index = 1,\n+\t\t.value_type = REFTABLE_REF_VAL1,\n+\t\t.value.val1 = hash,\n+\t};\n+\tint err;\n+\tint i;\n+\n+\treftable_writer_set_limits(w, 1, 1);\n+\n+\t/* Write the same hash in many refs. If there is only 1 hash, the\n+\t * disambiguating prefix is length 0 */\n+\tfor (i = 0; i < 256; i++) {\n+\t\tchar name[256];\n+\t\tsnprintf(name, sizeof(name), \"ref%05d\", i);\n+\t\tref.refname = name;\n+\t\terr = reftable_writer_add_ref(w, &ref);\n+\t\tEXPECT_ERR(err);\n+\t}\n+\n+\terr = reftable_writer_close(w);\n+\tEXPECT_ERR(err);\n+\tEXPECT(writer_stats(w)->object_id_len == 2);\n+\treftable_writer_free(w);\n+\tstrbuf_release(&buf);\n+}\n+\n static void test_write_empty_key(void)\n {\n \tstruct reftable_write_options opts = { 0 };\n@@ -772,5 +808,6 @@ int readwrite_test_main(int argc, const char *argv[])\n \tRUN_TEST(test_write_empty_key);\n \tRUN_TEST(test_write_empty_table);\n \tRUN_TEST(test_log_overflow);\n+\tRUN_TEST(test_write_object_id_min_length);\n \treturn 0;\n }\ndiff --git a/reftable/writer.c b/reftable/writer.c\nindex d54215a50dc..5e4e6e93416 100644\n--- a/reftable/writer.c\n+++ b/reftable/writer.c\n@@ -515,7 +515,9 @@ static void object_record_free(void *void_arg, void *key)\n static int writer_dump_object_index(struct reftable_writer *w)\n {\n \tstruct write_record_arg closure = { .w = w };\n-\tstruct common_prefix_arg common = { NULL };\n+\tstruct common_prefix_arg common = {\n+\t\t.max = 1,\t\t/* obj_id_len should be >= 2. */\n+\t};\n \tif (w->obj_index_tree) {\n \t\tinfix_walk(w->obj_index_tree, &update_common, &common);\n \t}\n-- \ngitgitgadget\n\n"},{"id":"449034","messageId":"bdccd96947531ecc352999c764ae0968ea210011.1645469170.git.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.v3.git.git.1645469170.gitgitgadget@gmail.com","subject":"[PATCH v3 6/7] reftable: add test for length of disambiguating prefix","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-21T18:46:09Z","receivedAt":"2022-02-21T18:46:46Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThe ID => ref map is trimming object IDs to a disambiguating prefix.\nCheck that we are computing their length correctly.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n reftable/readwrite_test.c | 38 ++++++++++++++++++++++++++++++++++++++\n 1 file changed, 38 insertions(+)\n\ndiff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c\nindex 35142eb070e..a1b835785a3 100644\n--- a/reftable/readwrite_test.c\n+++ b/reftable/readwrite_test.c\n@@ -703,6 +703,43 @@ static void test_write_object_id_min_length(void)\n \tstrbuf_release(&buf);\n }\n \n+static void test_write_object_id_length(void)\n+{\n+\tstruct reftable_write_options opts = {\n+\t\t.block_size = 75,\n+\t};\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct reftable_writer *w =\n+\t\treftable_new_writer(&strbuf_add_void, &buf, &opts);\n+\tuint8_t hash[GIT_SHA1_RAWSZ] = {42};\n+\tstruct reftable_ref_record ref = {\n+\t\t.update_index = 1,\n+\t\t.value_type = REFTABLE_REF_VAL1,\n+\t\t.value.val1 = hash,\n+\t};\n+\tint err;\n+\tint i;\n+\n+\treftable_writer_set_limits(w, 1, 1);\n+\n+\t/* Write the same hash in many refs. If there is only 1 hash, the\n+\t * disambiguating prefix is length 0 */\n+\tfor (i = 0; i < 256; i++) {\n+\t\tchar name[256];\n+\t\tsnprintf(name, sizeof(name), \"ref%05d\", i);\n+\t\tref.refname = name;\n+\t\tref.value.val1[15] = i;\n+\t\terr = reftable_writer_add_ref(w, &ref);\n+\t\tEXPECT_ERR(err);\n+\t}\n+\n+\terr = reftable_writer_close(w);\n+\tEXPECT_ERR(err);\n+\tEXPECT(writer_stats(w)->object_id_len == 16);\n+\treftable_writer_free(w);\n+\tstrbuf_release(&buf);\n+}\n+\n static void test_write_empty_key(void)\n {\n \tstruct reftable_write_options opts = { 0 };\n@@ -808,6 +845,7 @@ int readwrite_test_main(int argc, const char *argv[])\n \tRUN_TEST(test_write_empty_key);\n \tRUN_TEST(test_write_empty_table);\n \tRUN_TEST(test_log_overflow);\n+\tRUN_TEST(test_write_object_id_length);\n \tRUN_TEST(test_write_object_id_min_length);\n \treturn 0;\n }\n-- \ngitgitgadget\n\n"},{"id":"449035","messageId":"72499a14e383ef81e717692002ed68959774c7da.1645469170.git.gitgitgadget@gmail.com","threadId":"57229","inReplyTo":"pull.1185.v3.git.git.1645469170.gitgitgadget@gmail.com","subject":"[PATCH v3 7/7] reftable: rename writer_stats to reftable_writer_stats","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-21T18:46:10Z","receivedAt":"2022-02-21T18:46:50Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThis function is part of the reftable API, so it should use the\nreftable_ prefix\n\nSigned-off-by: Han-Wen Nienhuys \n---\n reftable/readwrite_test.c | 10 +++++-----\n reftable/reftable-writer.h | 2 +-\n reftable/writer.c | 2 +-\n 3 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c\nindex a1b835785a3..469ab79a5ad 100644\n--- a/reftable/readwrite_test.c\n+++ b/reftable/readwrite_test.c\n@@ -100,7 +100,7 @@ static void write_table(char ***names, struct strbuf *buf, int N,\n \tn = reftable_writer_close(w);\n \tEXPECT(n == 0);\n \n-\tstats = writer_stats(w);\n+\tstats = reftable_writer_stats(w);\n \tfor (i = 0; i < stats->ref_stats.blocks; i++) {\n \t\tint off = i * opts.block_size;\n \t\tif (off == 0) {\n@@ -239,7 +239,7 @@ static void test_log_write_read(void)\n \tn = reftable_writer_close(w);\n \tEXPECT(n == 0);\n \n-\tstats = writer_stats(w);\n+\tstats = reftable_writer_stats(w);\n \tEXPECT(stats->log_stats.blocks > 0);\n \treftable_writer_free(w);\n \tw = NULL;\n@@ -330,7 +330,7 @@ static void test_log_zlib_corruption(void)\n \tn = reftable_writer_close(w);\n \tEXPECT(n == 0);\n \n-\tstats = writer_stats(w);\n+\tstats = reftable_writer_stats(w);\n \tEXPECT(stats->log_stats.blocks > 0);\n \treftable_writer_free(w);\n \tw = NULL;\n@@ -698,7 +698,7 @@ static void test_write_object_id_min_length(void)\n \n \terr = reftable_writer_close(w);\n \tEXPECT_ERR(err);\n-\tEXPECT(writer_stats(w)->object_id_len == 2);\n+\tEXPECT(reftable_writer_stats(w)->object_id_len == 2);\n \treftable_writer_free(w);\n \tstrbuf_release(&buf);\n }\n@@ -735,7 +735,7 @@ static void test_write_object_id_length(void)\n \n \terr = reftable_writer_close(w);\n \tEXPECT_ERR(err);\n-\tEXPECT(writer_stats(w)->object_id_len == 16);\n+\tEXPECT(reftable_writer_stats(w)->object_id_len == 16);\n \treftable_writer_free(w);\n \tstrbuf_release(&buf);\n }\ndiff --git a/reftable/reftable-writer.h b/reftable/reftable-writer.h\nindex a560dc17255..db8de197f6c 100644\n--- a/reftable/reftable-writer.h\n+++ b/reftable/reftable-writer.h\n@@ -143,7 +143,7 @@ int reftable_writer_close(struct reftable_writer *w);\n \n This struct becomes invalid when the writer is freed.\n */\n-const struct reftable_stats *writer_stats(struct reftable_writer *w);\n+const struct reftable_stats *reftable_writer_stats(struct reftable_writer *w);\n \n /* reftable_writer_free deallocates memory for the writer */\n void reftable_writer_free(struct reftable_writer *w);\ndiff --git a/reftable/writer.c b/reftable/writer.c\nindex 5e4e6e93416..6d979e245ff 100644\n--- a/reftable/writer.c\n+++ b/reftable/writer.c\n@@ -695,7 +695,7 @@ static int writer_flush_block(struct reftable_writer *w)\n \treturn writer_flush_nonempty_block(w);\n }\n \n-const struct reftable_stats *writer_stats(struct reftable_writer *w)\n+const struct reftable_stats *reftable_writer_stats(struct reftable_writer *w)\n {\n \treturn &w->stats;\n }\n-- \ngitgitgadget\n"},{"id":"449348","messageId":"xmqqk0dls1qx.fsf@gitster.g","threadId":"57229","inReplyTo":"pull.1185.v3.git.git.1645469170.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 0/7] reftable: avoid reading and writing empty keys","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-23T21:37:42Z","receivedAt":"2022-02-23T21:37:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Han-Wen Nienhuys via GitGitGadget\" writes:\n\n> this series makes sure that the object record does not have to consider\n> empty keys (and therefore, a NULL memcpy destination)\n>\n> while we're at it add some more tests, and fix a naming mistake.\n\nLooking good. Let's mark it for 'next' and below soonish.\n\nThanks.\n\n>\n> Han-Wen Nienhuys (7):\n> Documentation: object_id_len goes up to 31\n> reftable: reject 0 object_id_len\n> reftable: add a test that verifies that writing empty keys fails\n> reftable: avoid writing empty keys at the block layer\n> reftable: ensure that obj_id_len is >= 2 on writing\n> reftable: add test for length of disambiguating prefix\n> reftable: rename writer_stats to reftable_writer_stats\n>\n> Documentation/technical/reftable.txt | 2 +-\n> reftable/block.c | 27 ++++---\n> reftable/block_test.c | 5 ++\n> reftable/reader.c | 5 ++\n> reftable/readwrite_test.c | 105 ++++++++++++++++++++++++++-\n> reftable/reftable-writer.h | 2 +-\n> reftable/writer.c | 9 ++-\n> 7 files changed, 136 insertions(+), 19 deletions(-)\n>\n>\n> base-commit: 45fe28c951c3e70666ee4ef8379772851a8e4d32\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1185%2Fhanwen%2Fobj-id-len-v3\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1185/hanwen/obj-id-len-v3\n> Pull-Request: https://github.com/git/git/pull/1185\n>\n> Range-diff vs v2:\n>\n> 1: 80d29e8f269 = 1: 80d29e8f269 Documentation: object_id_len goes up to 31\n> 2: 4c1a19fc4ae ! 2: 68e7bc32ff8 reftable: reject 0 object_id_len\n> @@ Commit message\n> reftable: reject 0 object_id_len\n> \n> The spec says 2 <= object_id_len <= 31. We are lenient and allow 1,\n> - but we forbid 0, so we can we can be sure that we never read a\n> - 0-length key.\n> + but we forbid 0, so we can be sure that we never read a 0-length key.\n> \n> Signed-off-by: Han-Wen Nienhuys \n> \n> 3: 600b115f8b1 = 3: 8b5aebdb07a reftable: add a test that verifies that writing empty keys fails\n> 4: ba036ee8543 ! 4: a9372cacd1b reftable: avoid writing empty keys at the block layer\n> @@ reftable/block.c: int block_reader_first_key(struct block_reader *br, struct str\n> \tif (n < 0)\n> \t\treturn n;\n> +\tif (!key->len)\n> -+\t\treturn -1;\n> ++\t\treturn REFTABLE_FORMAT_ERROR;\n> \n> \treturn 0;\n> }\n> 5: 2bd3d44ba57 = 5: 0b8a42399dd reftable: ensure that obj_id_len is >= 2 on writing\n> 6: 82d36ee0e0d = 6: bdccd969475 reftable: add test for length of disambiguating prefix\n> 7: c6ffdb3471c = 7: 72499a14e38 reftable: rename writer_stats to reftable_writer_stats\n"}]}