freshlybakedca.ke / git
Git fork
fork atom

reftable/block: store block pointer in the block iterator

The block iterator requires access to a bunch of data from the
underlying `reftable_block` that it is iterating over. This data is
stored by copying over relevant data into a separate set of variables.
This has multiple downsides:

- We require more storage space than necessary. This is more of a
theoretical issue as we shouldn't ever have many blocks.

- We have to perform more bookkeeping, and the variable names are
inconsistent across the two data structures. This can lead to some
confusion.

- The lifetime of the block iterator is tied to the block anyway, but
we hide that a bit by only storing pointers pointing into the block.

There isn't really any good reason why we rip out parts of the block
instead of storing a pointer to the block itself.

Refactor the code to do so. Despite being simpler, it also allows us to
decouple the lifetime of the block iterator from seeking in a subsequent
commit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

authored by

Patrick Steinhardt and committed by
Junio C Hamano 156d79ce 655e18d6

+9 -17
unified split
+8 -14
reftable/block.c
··· 381 381 return reftable_get_be24(b->block_data.data + b->restart_off + 3 * idx); 382 382 } 383 383 384 - void block_iter_seek_start(struct block_iter *it, const struct reftable_block *b) 384 + void block_iter_seek_start(struct block_iter *it, const struct reftable_block *block) 385 385 { 386 - it->block = b->block_data.data; 387 - it->block_len = b->restart_off; 388 - it->hash_size = b->hash_size; 386 + it->block = block; 389 387 reftable_buf_reset(&it->last_key); 390 - it->next_off = b->header_off + 4; 388 + it->next_off = block->header_off + 4; 391 389 } 392 390 393 391 struct restart_needle_less_args { ··· 435 433 int block_iter_next(struct block_iter *it, struct reftable_record *rec) 436 434 { 437 435 struct string_view in = { 438 - .buf = (unsigned char *) it->block + it->next_off, 439 - .len = it->block_len - it->next_off, 436 + .buf = (unsigned char *) it->block->block_data.data + it->next_off, 437 + .len = it->block->restart_off - it->next_off, 440 438 }; 441 439 struct string_view start = in; 442 440 uint8_t extra = 0; 443 441 int n = 0; 444 442 445 - if (it->next_off >= it->block_len) 443 + if (it->next_off >= it->block->restart_off) 446 444 return 1; 447 445 448 446 n = reftable_decode_key(&it->last_key, &extra, in); ··· 452 450 return REFTABLE_FORMAT_ERROR; 453 451 454 452 string_view_consume(&in, n); 455 - n = reftable_record_decode(rec, it->last_key, extra, in, it->hash_size, 453 + n = reftable_record_decode(rec, it->last_key, extra, in, it->block->hash_size, 456 454 &it->scratch); 457 455 if (n < 0) 458 456 return -1; ··· 467 465 reftable_buf_reset(&it->last_key); 468 466 it->next_off = 0; 469 467 it->block = NULL; 470 - it->block_len = 0; 471 - it->hash_size = 0; 472 468 } 473 469 474 470 void block_iter_close(struct block_iter *it) ··· 528 524 it->next_off = block_restart_offset(block, i - 1); 529 525 else 530 526 it->next_off = block->header_off + 4; 531 - it->block = block->block_data.data; 532 - it->block_len = block->restart_off; 533 - it->hash_size = block->hash_size; 527 + it->block = block; 534 528 535 529 err = reftable_record_init(&rec, reftable_block_type(block)); 536 530 if (err < 0)
+1 -3
reftable/block.h
··· 67 67 struct block_iter { 68 68 /* offset within the block of the next entry to read. */ 69 69 uint32_t next_off; 70 - const unsigned char *block; 71 - size_t block_len; 72 - uint32_t hash_size; 70 + const struct reftable_block *block; 73 71 74 72 /* key for last entry we read. */ 75 73 struct reftable_buf last_key;