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>

diff --git a/reftable/block.c b/reftable/block.c
index 844da397833f5c2520439be2931941f705eb9391..a425dd7abf3c8b6079651804df079a7d664f3cdf 100644 (file)
--- a/reftable/block.c
+++ b/reftable/block.c
@@ -381,13 +381,11 @@ static uint32_t block_restart_offset(const struct reftable_block *b, size_t idx)
        return reftable_get_be24(b->block_data.data + b->restart_off + 3 * idx);
 }
 
-void block_iter_seek_start(struct block_iter *it, const struct reftable_block *b)
+void block_iter_seek_start(struct block_iter *it, const struct reftable_block *block)
 {
-       it->block = b->block_data.data;
-       it->block_len = b->restart_off;
-       it->hash_size = b->hash_size;
+       it->block = block;
        reftable_buf_reset(&it->last_key);
-       it->next_off = b->header_off + 4;
+       it->next_off = block->header_off + 4;
 }
 
 struct restart_needle_less_args {
@@ -435,14 +433,14 @@ static int restart_needle_less(size_t idx, void *_args)
 int block_iter_next(struct block_iter *it, struct reftable_record *rec)
 {
        struct string_view in = {
-               .buf = (unsigned char *) it->block + it->next_off,
-               .len = it->block_len - it->next_off,
+               .buf = (unsigned char *) it->block->block_data.data + it->next_off,
+               .len = it->block->restart_off - it->next_off,
        };
        struct string_view start = in;
        uint8_t extra = 0;
        int n = 0;
 
-       if (it->next_off >= it->block_len)
+       if (it->next_off >= it->block->restart_off)
                return 1;
 
        n = reftable_decode_key(&it->last_key, &extra, in);
@@ -452,7 +450,7 @@ int block_iter_next(struct block_iter *it, struct reftable_record *rec)
                return REFTABLE_FORMAT_ERROR;
 
        string_view_consume(&in, n);
-       n = reftable_record_decode(rec, it->last_key, extra, in, it->hash_size,
+       n = reftable_record_decode(rec, it->last_key, extra, in, it->block->hash_size,
                                   &it->scratch);
        if (n < 0)
                return -1;
@@ -467,8 +465,6 @@ void block_iter_reset(struct block_iter *it)
        reftable_buf_reset(&it->last_key);
        it->next_off = 0;
        it->block = NULL;
-       it->block_len = 0;
-       it->hash_size = 0;
 }
 
 void block_iter_close(struct block_iter *it)
@@ -528,9 +524,7 @@ int block_iter_seek_key(struct block_iter *it, const struct reftable_block *bloc
                it->next_off = block_restart_offset(block, i - 1);
        else
                it->next_off = block->header_off + 4;
-       it->block = block->block_data.data;
-       it->block_len = block->restart_off;
-       it->hash_size = block->hash_size;
+       it->block = block;
 
        err = reftable_record_init(&rec, reftable_block_type(block));
        if (err < 0)

diff --git a/reftable/block.h b/reftable/block.h
index 4f7f29028c4602bf431a5807f13a4b331dd05b22..268d5a1e00526e5a31eb30861dd330dc32a290de 100644 (file)
--- a/reftable/block.h
+++ b/reftable/block.h
@@ -67,9 +67,7 @@ void block_writer_release(struct block_writer *bw);
 struct block_iter {
        /* offset within the block of the next entry to read. */
        uint32_t next_off;
-       const unsigned char *block;
-       size_t block_len;
-       uint32_t hash_size;
+       const struct reftable_block *block;
 
        /* key for last entry we read. */
        struct reftable_buf last_key;