reftable/record: improve semantics when initializing records

According to our usual coding style, the `reftable_new_record()` function would indicate that it is allocating a new record. This is not the case though as the function merely initializes records without allocating any memory. Replace `reftable_new_record()` with a new `reftable_record_init()` function that takes a record pointer as input and initializes it accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Feb 6, 2024 at 07:35 UTC 3ddef475d008b8a705a53e6bb1405bb773ffdc50
6 files changed +33 -54
reftable/block.c
+9 -9
@@ -382,23 +382,23 @@ int block_reader_seek(struct block_reader *br, struct block_iter *it,
382 .key = *want,
383 .r = br,
384 };
385 - struct reftable_record rec = reftable_new_record(block_reader_type(br));
386 - int err = 0;
385 struct block_iter next = BLOCK_ITER_INIT;
386 + struct reftable_record rec;
387 + int err = 0, i;
388
389 - int i = binsearch(br->restart_count, &restart_key_less, &args);
389 if (args.error) {
390 err = REFTABLE_FORMAT_ERROR;
391 goto done;
392 }
393
395 - it->br = br;
396 - if (i > 0) {
397 - i--;
398 - it->next_off = block_reader_restart_offset(br, i);
399 - } else {
394 + i = binsearch(br->restart_count, &restart_key_less, &args);
395 + if (i > 0)
396 + it->next_off = block_reader_restart_offset(br, i - 1);
397 + else
398 it->next_off = br->header_off + 4;
401 - }
399 + it->br = br;
400 +
401 + reftable_record_init(&rec, block_reader_type(br));
402
403 /* We're looking for the last entry less/equal than the wanted key, so
404 we have to go one entry too far and then back up.
reftable/merged.c
+5 -3
@@ -21,11 +21,11 @@ static int merged_iter_init(struct merged_iter *mi)
21 {
22 for (size_t i = 0; i < mi->stack_len; i++) {
23 struct pq_entry e = {
24 - .rec = reftable_new_record(mi->typ),
24 .index = i,
25 };
26 int err;
27
28 + reftable_record_init(&e.rec, mi->typ);
29 err = iterator_next(&mi->stack[i], &e.rec);
30 if (err < 0)
31 return err;
@@ -57,10 +57,12 @@ static int merged_iter_advance_nonnull_subiter(struct merged_iter *mi,
57 size_t idx)
58 {
59 struct pq_entry e = {
60 - .rec = reftable_new_record(mi->typ),
60 .index = idx,
61 };
63 - int err = iterator_next(&mi->stack[idx], &e.rec);
62 + int err;
63 +
64 + reftable_record_init(&e.rec, mi->typ);
65 + err = iterator_next(&mi->stack[idx], &e.rec);
66 if (err < 0)
67 return err;
68
reftable/reader.c
+2 -2
@@ -444,13 +444,13 @@ static int reader_start(struct reftable_reader *r, struct table_iter *ti,
444 static int reader_seek_linear(struct table_iter *ti,
445 struct reftable_record *want)
446 {
447 - struct reftable_record rec =
448 - reftable_new_record(reftable_record_type(want));
447 struct strbuf want_key = STRBUF_INIT;
448 struct strbuf got_key = STRBUF_INIT;
449 struct table_iter next = TABLE_ITER_INIT;
450 + struct reftable_record rec;
451 int err = -1;
452
453 + reftable_record_init(&rec, reftable_record_type(want));
454 reftable_record_key(want, &want_key);
455
456 while (1) {
reftable/record.c
+10 -33
@@ -1259,45 +1259,22 @@ reftable_record_vtable(struct reftable_record *rec)
1259 abort();
1260 }
1261
1262 -struct reftable_record reftable_new_record(uint8_t typ)
1262 +void reftable_record_init(struct reftable_record *rec, uint8_t typ)
1263 {
1264 - struct reftable_record clean = {
1265 - .type = typ,
1266 - };
1264 + memset(rec, 0, sizeof(*rec));
1265 + rec->type = typ;
1266
1268 - /* the following is involved, but the naive solution (just return
1269 - * `clean` as is, except for BLOCK_TYPE_INDEX), returns a garbage
1270 - * clean.u.obj.offsets pointer on Windows VS CI. Go figure.
1271 - */
1267 switch (typ) {
1273 - case BLOCK_TYPE_OBJ:
1274 - {
1275 - struct reftable_obj_record obj = { 0 };
1276 - clean.u.obj = obj;
1277 - break;
1278 - }
1279 - case BLOCK_TYPE_INDEX:
1280 - {
1281 - struct reftable_index_record idx = {
1282 - .last_key = STRBUF_INIT,
1283 - };
1284 - clean.u.idx = idx;
1285 - break;
1286 - }
1268 case BLOCK_TYPE_REF:
1288 - {
1289 - struct reftable_ref_record ref = { 0 };
1290 - clean.u.ref = ref;
1291 - break;
1292 - }
1269 case BLOCK_TYPE_LOG:
1294 - {
1295 - struct reftable_log_record log = { 0 };
1296 - clean.u.log = log;
1297 - break;
1298 - }
1270 + case BLOCK_TYPE_OBJ:
1271 + return;
1272 + case BLOCK_TYPE_INDEX:
1273 + strbuf_init(&rec->u.idx.last_key, 0);
1274 + return;
1275 + default:
1276 + BUG("unhandled record type");
1277 }
1300 - return clean;
1278 }
1279
1280 void reftable_record_print(struct reftable_record *rec, int hash_size)
reftable/record.h
+5 -5
@@ -69,9 +69,6 @@ struct reftable_record_vtable {
69 /* returns true for recognized block types. Block start with the block type. */
70 int reftable_is_block_type(uint8_t typ);
71
72 -/* return an initialized record for the given type */
73 -struct reftable_record reftable_new_record(uint8_t typ);
74 -
72 /* Encode `key` into `dest`. Sets `is_restart` to indicate a restart. Returns
73 * number of bytes written. */
74 int reftable_encode_key(int *is_restart, struct string_view dest,
@@ -100,8 +97,8 @@ struct reftable_obj_record {
97 /* record is a generic wrapper for different types of records. It is normally
98 * created on the stack, or embedded within another struct. If the type is
99 * known, a fresh instance can be initialized explicitly. Otherwise, use
103 - * reftable_new_record() to initialize generically (as the index_record is not
104 - * valid as 0-initialized structure)
100 + * `reftable_record_init()` to initialize generically (as the index_record is
101 + * not valid as 0-initialized structure)
102 */
103 struct reftable_record {
104 uint8_t type;
@@ -113,6 +110,9 @@ struct reftable_record {
110 } u;
111 };
112
113 +/* Initialize the reftable record for the given type */
114 +void reftable_record_init(struct reftable_record *rec, uint8_t typ);
115 +
116 /* see struct record_vtable */
117 int reftable_record_equal(struct reftable_record *a, struct reftable_record *b, int hash_size);
118 void reftable_record_print(struct reftable_record *rec, int hash_size);
reftable/record_test.c
+2 -2
@@ -16,11 +16,11 @@
16
17 static void test_copy(struct reftable_record *rec)
18 {
19 - struct reftable_record copy = { 0 };
19 + struct reftable_record copy;
20 uint8_t typ;
21
22 typ = reftable_record_type(rec);
23 - copy = reftable_new_record(typ);
23 + reftable_record_init(&copy, typ);
24 reftable_record_copy_from(&copy, rec, GIT_SHA1_RAWSZ);
25 /* do it twice to catch memory leaks */
26 reftable_record_copy_from(&copy, rec, GIT_SHA1_RAWSZ);