reftable/reader: inline `init_reader()`

Most users use an allocated version of the `reftable_reader`, except for some tests. We are about to convert the reader to become refcounted though, and providing the ability to keep a reader on the stack makes this conversion harder than necessary. Update the tests to use `reftable_reader_new()` instead to prepare for this change. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Aug 23, 2024 at 16:12 UTC 2de3c0d34555685a0502e81a37436a7db41a2ddf
3 files changed +98 -99
reftable/reader.c
+61 -61
@@ -162,58 +162,6 @@ done:
162 return err;
163 }
164
165 -int init_reader(struct reftable_reader *r, struct reftable_block_source *source,
166 - const char *name)
167 -{
168 - struct reftable_block footer = { NULL };
169 - struct reftable_block header = { NULL };
170 - int err = 0;
171 - uint64_t file_size = block_source_size(source);
172 -
173 - /* Need +1 to read type of first block. */
174 - uint32_t read_size = header_size(2) + 1; /* read v2 because it's larger. */
175 - memset(r, 0, sizeof(struct reftable_reader));
176 -
177 - if (read_size > file_size) {
178 - err = REFTABLE_FORMAT_ERROR;
179 - goto done;
180 - }
181 -
182 - err = block_source_read_block(source, &header, 0, read_size);
183 - if (err != read_size) {
184 - err = REFTABLE_IO_ERROR;
185 - goto done;
186 - }
187 -
188 - if (memcmp(header.data, "REFT", 4)) {
189 - err = REFTABLE_FORMAT_ERROR;
190 - goto done;
191 - }
192 - r->version = header.data[4];
193 - if (r->version != 1 && r->version != 2) {
194 - err = REFTABLE_FORMAT_ERROR;
195 - goto done;
196 - }
197 -
198 - r->size = file_size - footer_size(r->version);
199 - r->source = *source;
200 - r->name = xstrdup(name);
201 - r->hash_id = 0;
202 -
203 - err = block_source_read_block(source, &footer, r->size,
204 - footer_size(r->version));
205 - if (err != footer_size(r->version)) {
206 - err = REFTABLE_IO_ERROR;
207 - goto done;
208 - }
209 -
210 - err = parse_footer(r, footer.data, header.data);
211 -done:
212 - reftable_block_done(&footer);
213 - reftable_block_done(&header);
214 - return err;
215 -}
216 -
165 struct table_iter {
166 struct reftable_reader *r;
167 uint8_t typ;
@@ -637,16 +585,68 @@ void reader_close(struct reftable_reader *r)
585 FREE_AND_NULL(r->name);
586 }
587
640 -int reftable_reader_new(struct reftable_reader **p,
641 - struct reftable_block_source *src, char const *name)
588 +int reftable_reader_new(struct reftable_reader **out,
589 + struct reftable_block_source *source, char const *name)
590 {
643 - struct reftable_reader *rd = reftable_calloc(1, sizeof(*rd));
644 - int err = init_reader(rd, src, name);
645 - if (err == 0) {
646 - *p = rd;
647 - } else {
648 - block_source_close(src);
649 - reftable_free(rd);
591 + struct reftable_block footer = { 0 };
592 + struct reftable_block header = { 0 };
593 + struct reftable_reader *r;
594 + uint64_t file_size = block_source_size(source);
595 + uint32_t read_size;
596 + int err;
597 +
598 + REFTABLE_CALLOC_ARRAY(r, 1);
599 +
600 + /*
601 + * We need one extra byte to read the type of first block. We also
602 + * pretend to always be reading v2 of the format because it is larger.
603 + */
604 + read_size = header_size(2) + 1;
605 + if (read_size > file_size) {
606 + err = REFTABLE_FORMAT_ERROR;
607 + goto done;
608 + }
609 +
610 + err = block_source_read_block(source, &header, 0, read_size);
611 + if (err != read_size) {
612 + err = REFTABLE_IO_ERROR;
613 + goto done;
614 + }
615 +
616 + if (memcmp(header.data, "REFT", 4)) {
617 + err = REFTABLE_FORMAT_ERROR;
618 + goto done;
619 + }
620 + r->version = header.data[4];
621 + if (r->version != 1 && r->version != 2) {
622 + err = REFTABLE_FORMAT_ERROR;
623 + goto done;
624 + }
625 +
626 + r->size = file_size - footer_size(r->version);
627 + r->source = *source;
628 + r->name = xstrdup(name);
629 + r->hash_id = 0;
630 +
631 + err = block_source_read_block(source, &footer, r->size,
632 + footer_size(r->version));
633 + if (err != footer_size(r->version)) {
634 + err = REFTABLE_IO_ERROR;
635 + goto done;
636 + }
637 +
638 + err = parse_footer(r, footer.data, header.data);
639 + if (err)
640 + goto done;
641 +
642 + *out = r;
643 +
644 +done:
645 + reftable_block_done(&footer);
646 + reftable_block_done(&header);
647 + if (err) {
648 + reftable_free(r);
649 + block_source_close(source);
650 }
651 return err;
652 }
reftable/reader.h
-2
@@ -52,8 +52,6 @@ struct reftable_reader {
52 struct reftable_reader_offsets log_offsets;
53 };
54
55 -int init_reader(struct reftable_reader *r, struct reftable_block_source *source,
56 - const char *name);
55 void reader_close(struct reftable_reader *r);
56 const char *reader_name(struct reftable_reader *r);
57
reftable/readwrite_test.c
+37 -36
@@ -195,7 +195,7 @@ static void test_log_write_read(void)
195 struct reftable_log_record log = { NULL };
196 int n;
197 struct reftable_iterator it = { NULL };
198 - struct reftable_reader rd = { NULL };
198 + struct reftable_reader *reader;
199 struct reftable_block_source source = { NULL };
200 struct strbuf buf = STRBUF_INIT;
201 struct reftable_writer *w =
@@ -236,10 +236,10 @@ static void test_log_write_read(void)
236
237 block_source_from_strbuf(&source, &buf);
238
239 - err = init_reader(&rd, &source, "file.log");
239 + err = reftable_reader_new(&reader, &source, "file.log");
240 EXPECT_ERR(err);
241
242 - reftable_reader_init_ref_iterator(&rd, &it);
242 + reftable_reader_init_ref_iterator(reader, &it);
243
244 err = reftable_iterator_seek_ref(&it, names[N - 1]);
245 EXPECT_ERR(err);
@@ -254,7 +254,7 @@ static void test_log_write_read(void)
254 reftable_iterator_destroy(&it);
255 reftable_ref_record_release(&ref);
256
257 - reftable_reader_init_log_iterator(&rd, &it);
257 + reftable_reader_init_log_iterator(reader, &it);
258
259 err = reftable_iterator_seek_log(&it, "");
260 EXPECT_ERR(err);
@@ -279,7 +279,7 @@ static void test_log_write_read(void)
279 /* cleanup. */
280 strbuf_release(&buf);
281 free_names(names);
282 - reader_close(&rd);
282 + reftable_reader_free(reader);
283 }
284
285 static void test_log_zlib_corruption(void)
@@ -288,7 +288,7 @@ static void test_log_zlib_corruption(void)
288 .block_size = 256,
289 };
290 struct reftable_iterator it = { 0 };
291 - struct reftable_reader rd = { 0 };
291 + struct reftable_reader *reader;
292 struct reftable_block_source source = { 0 };
293 struct strbuf buf = STRBUF_INIT;
294 struct reftable_writer *w =
@@ -331,18 +331,18 @@ static void test_log_zlib_corruption(void)
331
332 block_source_from_strbuf(&source, &buf);
333
334 - err = init_reader(&rd, &source, "file.log");
334 + err = reftable_reader_new(&reader, &source, "file.log");
335 EXPECT_ERR(err);
336
337 - reftable_reader_init_log_iterator(&rd, &it);
337 + reftable_reader_init_log_iterator(reader, &it);
338 err = reftable_iterator_seek_log(&it, "refname");
339 EXPECT(err == REFTABLE_ZLIB_ERROR);
340
341 reftable_iterator_destroy(&it);
342
343 /* cleanup. */
344 + reftable_reader_free(reader);
345 strbuf_release(&buf);
345 - reader_close(&rd);
346 }
347
348 static void test_table_read_write_sequential(void)
@@ -352,7 +352,7 @@ static void test_table_read_write_sequential(void)
352 int N = 50;
353 struct reftable_iterator it = { NULL };
354 struct reftable_block_source source = { NULL };
355 - struct reftable_reader rd = { NULL };
355 + struct reftable_reader *reader;
356 int err = 0;
357 int j = 0;
358
@@ -360,10 +360,10 @@ static void test_table_read_write_sequential(void)
360
361 block_source_from_strbuf(&source, &buf);
362
363 - err = init_reader(&rd, &source, "file.ref");
363 + err = reftable_reader_new(&reader, &source, "file.ref");
364 EXPECT_ERR(err);
365
366 - reftable_reader_init_ref_iterator(&rd, &it);
366 + reftable_reader_init_ref_iterator(reader, &it);
367 err = reftable_iterator_seek_ref(&it, "");
368 EXPECT_ERR(err);
369
@@ -381,11 +381,11 @@ static void test_table_read_write_sequential(void)
381 reftable_ref_record_release(&ref);
382 }
383 EXPECT(j == N);
384 +
385 reftable_iterator_destroy(&it);
386 + reftable_reader_free(reader);
387 strbuf_release(&buf);
388 free_names(names);
387 -
388 - reader_close(&rd);
389 }
390
391 static void test_table_write_small_table(void)
@@ -404,7 +404,7 @@ static void test_table_read_api(void)
404 char **names;
405 struct strbuf buf = STRBUF_INIT;
406 int N = 50;
407 - struct reftable_reader rd = { NULL };
407 + struct reftable_reader *reader;
408 struct reftable_block_source source = { NULL };
409 int err;
410 int i;
@@ -415,10 +415,10 @@ static void test_table_read_api(void)
415
416 block_source_from_strbuf(&source, &buf);
417
418 - err = init_reader(&rd, &source, "file.ref");
418 + err = reftable_reader_new(&reader, &source, "file.ref");
419 EXPECT_ERR(err);
420
421 - reftable_reader_init_ref_iterator(&rd, &it);
421 + reftable_reader_init_ref_iterator(reader, &it);
422 err = reftable_iterator_seek_ref(&it, names[0]);
423 EXPECT_ERR(err);
424
@@ -431,7 +431,7 @@ static void test_table_read_api(void)
431 }
432 reftable_iterator_destroy(&it);
433 reftable_free(names);
434 - reader_close(&rd);
434 + reftable_reader_free(reader);
435 strbuf_release(&buf);
436 }
437
@@ -440,7 +440,7 @@ static void test_table_read_write_seek(int index, int hash_id)
440 char **names;
441 struct strbuf buf = STRBUF_INIT;
442 int N = 50;
443 - struct reftable_reader rd = { NULL };
443 + struct reftable_reader *reader;
444 struct reftable_block_source source = { NULL };
445 int err;
446 int i = 0;
@@ -453,18 +453,18 @@ static void test_table_read_write_seek(int index, int hash_id)
453
454 block_source_from_strbuf(&source, &buf);
455
456 - err = init_reader(&rd, &source, "file.ref");
456 + err = reftable_reader_new(&reader, &source, "file.ref");
457 EXPECT_ERR(err);
458 - EXPECT(hash_id == reftable_reader_hash_id(&rd));
458 + EXPECT(hash_id == reftable_reader_hash_id(reader));
459
460 if (!index) {
461 - rd.ref_offsets.index_offset = 0;
461 + reader->ref_offsets.index_offset = 0;
462 } else {
463 - EXPECT(rd.ref_offsets.index_offset > 0);
463 + EXPECT(reader->ref_offsets.index_offset > 0);
464 }
465
466 for (i = 1; i < N; i++) {
467 - reftable_reader_init_ref_iterator(&rd, &it);
467 + reftable_reader_init_ref_iterator(reader, &it);
468 err = reftable_iterator_seek_ref(&it, names[i]);
469 EXPECT_ERR(err);
470 err = reftable_iterator_next_ref(&it, &ref);
@@ -480,7 +480,7 @@ static void test_table_read_write_seek(int index, int hash_id)
480 strbuf_addstr(&pastLast, names[N - 1]);
481 strbuf_addstr(&pastLast, "/");
482
483 - reftable_reader_init_ref_iterator(&rd, &it);
483 + reftable_reader_init_ref_iterator(reader, &it);
484 err = reftable_iterator_seek_ref(&it, pastLast.buf);
485 if (err == 0) {
486 struct reftable_ref_record ref = { NULL };
@@ -498,7 +498,7 @@ static void test_table_read_write_seek(int index, int hash_id)
498 reftable_free(names[i]);
499 }
500 reftable_free(names);
501 - reader_close(&rd);
501 + reftable_reader_free(reader);
502 }
503
504 static void test_table_read_write_seek_linear(void)
@@ -530,7 +530,7 @@ static void test_table_refs_for(int indexed)
530 int i = 0;
531 int n;
532 int err;
533 - struct reftable_reader rd;
533 + struct reftable_reader *reader;
534 struct reftable_block_source source = { NULL };
535
536 struct strbuf buf = STRBUF_INIT;
@@ -579,18 +579,18 @@ static void test_table_refs_for(int indexed)
579
580 block_source_from_strbuf(&source, &buf);
581
582 - err = init_reader(&rd, &source, "file.ref");
582 + err = reftable_reader_new(&reader, &source, "file.ref");
583 EXPECT_ERR(err);
584 if (!indexed) {
585 - rd.obj_offsets.is_present = 0;
585 + reader->obj_offsets.is_present = 0;
586 }
587
588 - reftable_reader_init_ref_iterator(&rd, &it);
588 + reftable_reader_init_ref_iterator(reader, &it);
589 err = reftable_iterator_seek_ref(&it, "");
590 EXPECT_ERR(err);
591 reftable_iterator_destroy(&it);
592
593 - err = reftable_reader_refs_for(&rd, &it, want_hash);
593 + err = reftable_reader_refs_for(reader, &it, want_hash);
594 EXPECT_ERR(err);
595
596 j = 0;
@@ -611,7 +611,7 @@ static void test_table_refs_for(int indexed)
611 strbuf_release(&buf);
612 free_names(want_names);
613 reftable_iterator_destroy(&it);
614 - reader_close(&rd);
614 + reftable_reader_free(reader);
615 }
616
617 static void test_table_refs_for_no_index(void)
@@ -928,11 +928,11 @@ static void test_corrupt_table_empty(void)
928 {
929 struct strbuf buf = STRBUF_INIT;
930 struct reftable_block_source source = { NULL };
931 - struct reftable_reader rd = { NULL };
931 + struct reftable_reader *reader;
932 int err;
933
934 block_source_from_strbuf(&source, &buf);
935 - err = init_reader(&rd, &source, "file.log");
935 + err = reftable_reader_new(&reader, &source, "file.log");
936 EXPECT(err == REFTABLE_FORMAT_ERROR);
937 }
938
@@ -941,13 +941,14 @@ static void test_corrupt_table(void)
941 uint8_t zeros[1024] = { 0 };
942 struct strbuf buf = STRBUF_INIT;
943 struct reftable_block_source source = { NULL };
944 - struct reftable_reader rd = { NULL };
944 + struct reftable_reader *reader;
945 int err;
946 strbuf_add(&buf, zeros, sizeof(zeros));
947
948 block_source_from_strbuf(&source, &buf);
949 - err = init_reader(&rd, &source, "file.log");
949 + err = reftable_reader_new(&reader, &source, "file.log");
950 EXPECT(err == REFTABLE_FORMAT_ERROR);
951 +
952 strbuf_release(&buf);
953 }
954