From c983374035bcba0f70d8908d735d17dfef4e0edf Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Thu, 20 Jan 2022 15:12:11 +0000 Subject: reftable: implement record equality generically This simplifies unittests a little, and provides further coverage for reftable_record_copy(). Signed-off-by: Han-Wen Nienhuys Signed-off-by: Junio C Hamano --- reftable/record_test.c | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) (limited to 'reftable/record_test.c') diff --git a/reftable/record_test.c b/reftable/record_test.c index f4ad7cace4..9268084815 100644 --- a/reftable/record_test.c +++ b/reftable/record_test.c @@ -21,18 +21,7 @@ static void test_copy(struct reftable_record *rec) reftable_record_copy_from(©, rec, GIT_SHA1_RAWSZ); /* do it twice to catch memory leaks */ reftable_record_copy_from(©, rec, GIT_SHA1_RAWSZ); - switch (reftable_record_type(©)) { - case BLOCK_TYPE_REF: - EXPECT(reftable_ref_record_equal(reftable_record_as_ref(©), - reftable_record_as_ref(rec), - GIT_SHA1_RAWSZ)); - break; - case BLOCK_TYPE_LOG: - EXPECT(reftable_log_record_equal(reftable_record_as_log(©), - reftable_record_as_log(rec), - GIT_SHA1_RAWSZ)); - break; - } + EXPECT(reftable_record_equal(rec, ©, GIT_SHA1_RAWSZ)); reftable_record_destroy(©); } @@ -346,13 +335,7 @@ static void test_reftable_obj_record_roundtrip(void) GIT_SHA1_RAWSZ); EXPECT(n == m); - EXPECT(in.hash_prefix_len == out.hash_prefix_len); - EXPECT(in.offset_len == out.offset_len); - - EXPECT(!memcmp(in.hash_prefix, out.hash_prefix, - in.hash_prefix_len)); - EXPECT(0 == memcmp(in.offsets, out.offsets, - sizeof(uint64_t) * in.offset_len)); + EXPECT(reftable_record_equal(&rec, &rec_out, GIT_SHA1_RAWSZ)); strbuf_release(&key); reftable_record_release(&rec_out); } @@ -390,7 +373,7 @@ static void test_reftable_index_record_roundtrip(void) m = reftable_record_decode(&out_rec, key, extra, dest, GIT_SHA1_RAWSZ); EXPECT(m == n); - EXPECT(in.offset == out.offset); + EXPECT(reftable_record_equal(&rec, &out_rec, GIT_SHA1_RAWSZ)); reftable_record_release(&out_rec); strbuf_release(&key); -- cgit v1.2.3 From 66c0dabab5e15f78d0505be36cac4a383e14cf88 Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Thu, 20 Jan 2022 15:12:13 +0000 Subject: reftable: make reftable_record a tagged union This reduces the amount of glue code, because we don't need a void pointer or vtable within the structure. The only snag is that reftable_index_record contain a strbuf, so it cannot be zero-initialized. To address this, use reftable_new_record() to return fresh instance, given a record type. Since reftable_new_record() doesn't cause heap allocation anymore, it should be balanced with reftable_record_release() rather than reftable_record_destroy(). Thanks to Peff for the suggestion. Helped-by: Jeff King Signed-off-by: Han-Wen Nienhuys Signed-off-by: Junio C Hamano --- reftable/record_test.c | 157 +++++++++++++++++++++++++------------------------ 1 file changed, 80 insertions(+), 77 deletions(-) (limited to 'reftable/record_test.c') diff --git a/reftable/record_test.c b/reftable/record_test.c index 9268084815..c6fdd1925a 100644 --- a/reftable/record_test.c +++ b/reftable/record_test.c @@ -16,13 +16,16 @@ static void test_copy(struct reftable_record *rec) { - struct reftable_record copy = - reftable_new_record(reftable_record_type(rec)); + struct reftable_record copy = { 0 }; + uint8_t typ; + + typ = reftable_record_type(rec); + copy = reftable_new_record(typ); reftable_record_copy_from(©, rec, GIT_SHA1_RAWSZ); /* do it twice to catch memory leaks */ reftable_record_copy_from(©, rec, GIT_SHA1_RAWSZ); EXPECT(reftable_record_equal(rec, ©, GIT_SHA1_RAWSZ)); - reftable_record_destroy(©); + reftable_record_release(©); } static void test_varint_roundtrip(void) @@ -95,61 +98,58 @@ static void test_reftable_ref_record_roundtrip(void) int i = 0; for (i = REFTABLE_REF_DELETION; i < REFTABLE_NR_REF_VALUETYPES; i++) { - struct reftable_ref_record in = { NULL }; - struct reftable_ref_record out = { NULL }; - struct reftable_record rec_out = { NULL }; + struct reftable_record in = { + .type = BLOCK_TYPE_REF, + }; + struct reftable_record out = { .type = BLOCK_TYPE_REF }; struct strbuf key = STRBUF_INIT; - struct reftable_record rec = { NULL }; uint8_t buffer[1024] = { 0 }; struct string_view dest = { .buf = buffer, .len = sizeof(buffer), }; - int n, m; - in.value_type = i; + in.u.ref.value_type = i; switch (i) { case REFTABLE_REF_DELETION: break; case REFTABLE_REF_VAL1: - in.value.val1 = reftable_malloc(GIT_SHA1_RAWSZ); - set_hash(in.value.val1, 1); + in.u.ref.value.val1 = reftable_malloc(GIT_SHA1_RAWSZ); + set_hash(in.u.ref.value.val1, 1); break; case REFTABLE_REF_VAL2: - in.value.val2.value = reftable_malloc(GIT_SHA1_RAWSZ); - set_hash(in.value.val2.value, 1); - in.value.val2.target_value = + in.u.ref.value.val2.value = reftable_malloc(GIT_SHA1_RAWSZ); - set_hash(in.value.val2.target_value, 2); + set_hash(in.u.ref.value.val2.value, 1); + in.u.ref.value.val2.target_value = + reftable_malloc(GIT_SHA1_RAWSZ); + set_hash(in.u.ref.value.val2.target_value, 2); break; case REFTABLE_REF_SYMREF: - in.value.symref = xstrdup("target"); + in.u.ref.value.symref = xstrdup("target"); break; } - in.refname = xstrdup("refs/heads/master"); + in.u.ref.refname = xstrdup("refs/heads/master"); - reftable_record_from_ref(&rec, &in); - test_copy(&rec); + test_copy(&in); - EXPECT(reftable_record_val_type(&rec) == i); + EXPECT(reftable_record_val_type(&in) == i); - reftable_record_key(&rec, &key); - n = reftable_record_encode(&rec, dest, GIT_SHA1_RAWSZ); + reftable_record_key(&in, &key); + n = reftable_record_encode(&in, dest, GIT_SHA1_RAWSZ); EXPECT(n > 0); /* decode into a non-zero reftable_record to test for leaks. */ - - reftable_record_from_ref(&rec_out, &out); - m = reftable_record_decode(&rec_out, key, i, dest, - GIT_SHA1_RAWSZ); + m = reftable_record_decode(&out, key, i, dest, GIT_SHA1_RAWSZ); EXPECT(n == m); - EXPECT(reftable_ref_record_equal(&in, &out, GIT_SHA1_RAWSZ)); - reftable_record_release(&rec_out); + EXPECT(reftable_ref_record_equal(&in.u.ref, &out.u.ref, + GIT_SHA1_RAWSZ)); + reftable_record_release(&in); strbuf_release(&key); - reftable_ref_record_release(&in); + reftable_record_release(&out); } } @@ -202,7 +202,7 @@ static void test_reftable_log_record_roundtrip(void) set_test_hash(in[0].value.update.new_hash, 1); set_test_hash(in[0].value.update.old_hash, 2); for (i = 0; i < ARRAY_SIZE(in); i++) { - struct reftable_record rec = { NULL }; + struct reftable_record rec = { .type = BLOCK_TYPE_LOG }; struct strbuf key = STRBUF_INIT; uint8_t buffer[1024] = { 0 }; struct string_view dest = { @@ -210,23 +210,25 @@ static void test_reftable_log_record_roundtrip(void) .len = sizeof(buffer), }; /* populate out, to check for leaks. */ - struct reftable_log_record out = { - .refname = xstrdup("old name"), - .value_type = REFTABLE_LOG_UPDATE, - .value = { - .update = { - .new_hash = reftable_calloc(GIT_SHA1_RAWSZ), - .old_hash = reftable_calloc(GIT_SHA1_RAWSZ), - .name = xstrdup("old name"), - .email = xstrdup("old@email"), - .message = xstrdup("old message"), + struct reftable_record out = { + .type = BLOCK_TYPE_LOG, + .u.log = { + .refname = xstrdup("old name"), + .value_type = REFTABLE_LOG_UPDATE, + .value = { + .update = { + .new_hash = reftable_calloc(GIT_SHA1_RAWSZ), + .old_hash = reftable_calloc(GIT_SHA1_RAWSZ), + .name = xstrdup("old name"), + .email = xstrdup("old@email"), + .message = xstrdup("old message"), + }, }, }, }; - struct reftable_record rec_out = { NULL }; int n, m, valtype; - reftable_record_from_log(&rec, &in[i]); + rec.u.log = in[i]; test_copy(&rec); @@ -234,16 +236,16 @@ static void test_reftable_log_record_roundtrip(void) n = reftable_record_encode(&rec, dest, GIT_SHA1_RAWSZ); EXPECT(n >= 0); - reftable_record_from_log(&rec_out, &out); valtype = reftable_record_val_type(&rec); - m = reftable_record_decode(&rec_out, key, valtype, dest, + m = reftable_record_decode(&out, key, valtype, dest, GIT_SHA1_RAWSZ); EXPECT(n == m); - EXPECT(reftable_log_record_equal(&in[i], &out, GIT_SHA1_RAWSZ)); + EXPECT(reftable_log_record_equal(&in[i], &out.u.log, + GIT_SHA1_RAWSZ)); reftable_log_record_release(&in[i]); strbuf_release(&key); - reftable_record_release(&rec_out); + reftable_record_release(&out); } } @@ -311,41 +313,43 @@ static void test_reftable_obj_record_roundtrip(void) } }; int i = 0; for (i = 0; i < ARRAY_SIZE(recs); i++) { - struct reftable_obj_record in = recs[i]; uint8_t buffer[1024] = { 0 }; struct string_view dest = { .buf = buffer, .len = sizeof(buffer), }; - struct reftable_record rec = { NULL }; + struct reftable_record in = { + .type = BLOCK_TYPE_OBJ, + .u.obj = recs[i], + }; struct strbuf key = STRBUF_INIT; - struct reftable_obj_record out = { NULL }; - struct reftable_record rec_out = { NULL }; + struct reftable_record out = { .type = BLOCK_TYPE_OBJ }; int n, m; uint8_t extra; - reftable_record_from_obj(&rec, &in); - test_copy(&rec); - reftable_record_key(&rec, &key); - n = reftable_record_encode(&rec, dest, GIT_SHA1_RAWSZ); + test_copy(&in); + reftable_record_key(&in, &key); + n = reftable_record_encode(&in, dest, GIT_SHA1_RAWSZ); EXPECT(n > 0); - extra = reftable_record_val_type(&rec); - reftable_record_from_obj(&rec_out, &out); - m = reftable_record_decode(&rec_out, key, extra, dest, + extra = reftable_record_val_type(&in); + m = reftable_record_decode(&out, key, extra, dest, GIT_SHA1_RAWSZ); EXPECT(n == m); - EXPECT(reftable_record_equal(&rec, &rec_out, GIT_SHA1_RAWSZ)); + EXPECT(reftable_record_equal(&in, &out, GIT_SHA1_RAWSZ)); strbuf_release(&key); - reftable_record_release(&rec_out); + reftable_record_release(&out); } } static void test_reftable_index_record_roundtrip(void) { - struct reftable_index_record in = { - .offset = 42, - .last_key = STRBUF_INIT, + struct reftable_record in = { + .type = BLOCK_TYPE_INDEX, + .u.idx = { + .offset = 42, + .last_key = STRBUF_INIT, + }, }; uint8_t buffer[1024] = { 0 }; struct string_view dest = { @@ -353,31 +357,30 @@ static void test_reftable_index_record_roundtrip(void) .len = sizeof(buffer), }; struct strbuf key = STRBUF_INIT; - struct reftable_record rec = { NULL }; - struct reftable_index_record out = { .last_key = STRBUF_INIT }; - struct reftable_record out_rec = { NULL }; + struct reftable_record out = { + .type = BLOCK_TYPE_INDEX, + .u.idx = { .last_key = STRBUF_INIT }, + }; int n, m; uint8_t extra; - strbuf_addstr(&in.last_key, "refs/heads/master"); - reftable_record_from_index(&rec, &in); - reftable_record_key(&rec, &key); - test_copy(&rec); + strbuf_addstr(&in.u.idx.last_key, "refs/heads/master"); + reftable_record_key(&in, &key); + test_copy(&in); - EXPECT(0 == strbuf_cmp(&key, &in.last_key)); - n = reftable_record_encode(&rec, dest, GIT_SHA1_RAWSZ); + EXPECT(0 == strbuf_cmp(&key, &in.u.idx.last_key)); + n = reftable_record_encode(&in, dest, GIT_SHA1_RAWSZ); EXPECT(n > 0); - extra = reftable_record_val_type(&rec); - reftable_record_from_index(&out_rec, &out); - m = reftable_record_decode(&out_rec, key, extra, dest, GIT_SHA1_RAWSZ); + extra = reftable_record_val_type(&in); + m = reftable_record_decode(&out, key, extra, dest, GIT_SHA1_RAWSZ); EXPECT(m == n); - EXPECT(reftable_record_equal(&rec, &out_rec, GIT_SHA1_RAWSZ)); + EXPECT(reftable_record_equal(&in, &out, GIT_SHA1_RAWSZ)); - reftable_record_release(&out_rec); + reftable_record_release(&out); strbuf_release(&key); - strbuf_release(&in.last_key); + strbuf_release(&in.u.idx.last_key); } int record_test_main(int argc, const char *argv[]) -- cgit v1.2.3 From 01033de49f26b75afd1e868d56c332c60b141faa Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Thu, 20 Jan 2022 15:12:14 +0000 Subject: reftable: add print functions to the record types This isn't used per se, but it is useful for debugging, especially Windows CI failures. Signed-off-by: Han-Wen Nienhuys Signed-off-by: Junio C Hamano --- reftable/record_test.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) (limited to 'reftable/record_test.c') diff --git a/reftable/record_test.c b/reftable/record_test.c index c6fdd1925a..f91ea5e883 100644 --- a/reftable/record_test.c +++ b/reftable/record_test.c @@ -25,6 +25,10 @@ static void test_copy(struct reftable_record *rec) /* do it twice to catch memory leaks */ reftable_record_copy_from(©, rec, GIT_SHA1_RAWSZ); EXPECT(reftable_record_equal(rec, ©, GIT_SHA1_RAWSZ)); + + puts("testing print coverage:\n"); + reftable_record_print(©, GIT_SHA1_RAWSZ); + reftable_record_release(©); } @@ -176,7 +180,8 @@ static void test_reftable_log_record_equal(void) static void test_reftable_log_record_roundtrip(void) { int i; - struct reftable_log_record in[2] = { + + struct reftable_log_record in[] = { { .refname = xstrdup("refs/heads/master"), .update_index = 42, @@ -197,10 +202,24 @@ static void test_reftable_log_record_roundtrip(void) .refname = xstrdup("refs/heads/master"), .update_index = 22, .value_type = REFTABLE_LOG_DELETION, + }, + { + .refname = xstrdup("branch"), + .update_index = 33, + .value_type = REFTABLE_LOG_UPDATE, + .value = { + .update = { + .old_hash = reftable_malloc(GIT_SHA1_RAWSZ), + .new_hash = reftable_malloc(GIT_SHA1_RAWSZ), + /* rest of fields left empty. */ + }, + }, } }; set_test_hash(in[0].value.update.new_hash, 1); set_test_hash(in[0].value.update.old_hash, 2); + set_test_hash(in[2].value.update.new_hash, 3); + set_test_hash(in[2].value.update.old_hash, 4); for (i = 0; i < ARRAY_SIZE(in); i++) { struct reftable_record rec = { .type = BLOCK_TYPE_LOG }; struct strbuf key = STRBUF_INIT; -- cgit v1.2.3