reftable/block: make block iterators reseekable

Refactor the block iterators so that initialization and seeking are
different from one another. This makes the iterator trivially reseekable
by storing the pointer to the block at initialization time, which we can
then reuse on every seek.

This refactoring prepares the code for exposing a `reftable_iterator`
interface for blocks in a subsequent commit. Callsites are adjusted
accordingly.

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 a425dd7abf3c8b6079651804df079a7d664f3cdf..20712be7eee1339129ff698dc60fff1beae71789 100644 (file)
--- a/reftable/block.c
+++ b/reftable/block.c
@@ -381,11 +381,16 @@ 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 *block)
+void block_iter_init(struct block_iter *it, const struct reftable_block *block)
 {
        it->block = block;
+       block_iter_seek_start(it);
+}
+
+void block_iter_seek_start(struct block_iter *it)
+{
        reftable_buf_reset(&it->last_key);
-       it->next_off = block->header_off + 4;
+       it->next_off = it->block->header_off + 4;
 }
 
 struct restart_needle_less_args {
@@ -473,12 +478,11 @@ void block_iter_close(struct block_iter *it)
        reftable_buf_release(&it->scratch);
 }
 
-int block_iter_seek_key(struct block_iter *it, const struct reftable_block *block,
-                       struct reftable_buf *want)
+int block_iter_seek_key(struct block_iter *it, struct reftable_buf *want)
 {
        struct restart_needle_less_args args = {
                .needle = *want,
-               .block = block,
+               .block = it->block,
        };
        struct reftable_record rec;
        int err = 0;
@@ -496,7 +500,7 @@ int block_iter_seek_key(struct block_iter *it, const struct reftable_block *bloc
         * restart point. While that works alright, we would end up scanning
         * too many record.
         */
-       i = binsearch(block->restart_count, &restart_needle_less, &args);
+       i = binsearch(it->block->restart_count, &restart_needle_less, &args);
        if (args.error) {
                err = REFTABLE_FORMAT_ERROR;
                goto done;
@@ -521,12 +525,11 @@ int block_iter_seek_key(struct block_iter *it, const struct reftable_block *bloc
         *     starting from the preceding restart point.
         */
        if (i > 0)
-               it->next_off = block_restart_offset(block, i - 1);
+               it->next_off = block_restart_offset(it->block, i - 1);
        else
-               it->next_off = block->header_off + 4;
-       it->block = block;
+               it->next_off = it->block->header_off + 4;
 
-       err = reftable_record_init(&rec, reftable_block_type(block));
+       err = reftable_record_init(&rec, reftable_block_type(it->block));
        if (err < 0)
                goto done;
 

diff --git a/reftable/block.h b/reftable/block.h
index 268d5a1e00526e5a31eb30861dd330dc32a290de..1bfd44f56aa0b2eb7dda3522f9db17844ac7dea1 100644 (file)
--- a/reftable/block.h
+++ b/reftable/block.h
@@ -79,12 +79,23 @@ struct block_iter {
        .scratch = REFTABLE_BUF_INIT, \
 }
 
-/* Position `it` at start of the block */
-void block_iter_seek_start(struct block_iter *it, const struct reftable_block *block);
+/*
+ * Initialize the block iterator with the given block. The iterator will be
+ * positioned at the first record contained in the block. The block must remain
+ * valid until the end of the iterator's lifetime. It is valid to re-initialize
+ * iterators multiple times.
+ */
+void block_iter_init(struct block_iter *it, const struct reftable_block *block);
+
+/* Position the initialized iterator at the first record of its block. */
+void block_iter_seek_start(struct block_iter *it);
 
-/* Position `it` to the `want` key in the block */
-int block_iter_seek_key(struct block_iter *it, const struct reftable_block *block,
-                       struct reftable_buf *want);
+/*
+ * Position the initialized iterator at the desired record key. It is not an
+ * error in case the record cannot be found. If so, a subsequent call to
+ * `block_iter_next()` will indicate that the iterator is exhausted.
+ */
+int block_iter_seek_key(struct block_iter *it, struct reftable_buf *want);
 
 /* return < 0 for error, 0 for OK, > 0 for EOF. */
 int block_iter_next(struct block_iter *it, struct reftable_record *rec);

diff --git a/reftable/iter.c b/reftable/iter.c
index 9ce675510c9d497670162c9a287c25ea6db44cac..a79c90d9ba61cb934a193df015cc1c072998530a 100644 (file)
--- a/reftable/iter.c
+++ b/reftable/iter.c
@@ -139,7 +139,7 @@ static int indexed_table_ref_iter_next_block(struct indexed_table_ref_iter *it)
                /* indexed block does not exist. */
                return REFTABLE_FORMAT_ERROR;
        }
-       block_iter_seek_start(&it->cur, &it->block);
+       block_iter_init(&it->cur, &it->block);
        return 0;
 }
 

diff --git a/reftable/table.c b/reftable/table.c
index ef1f33c92fcd86ec7df468ff6e2e156d5ad292e7..50ffad7edcbabea61c767ea8d554fd52e3db1cd4 100644 (file)
--- a/reftable/table.c
+++ b/reftable/table.c
@@ -208,7 +208,7 @@ static int table_iter_next_block(struct table_iter *ti)
 
        ti->block_off = next_block_off;
        ti->is_finished = 0;
-       block_iter_seek_start(&ti->bi, &ti->block);
+       block_iter_init(&ti->bi, &ti->block);
 
        return 0;
 }
@@ -256,7 +256,7 @@ static int table_iter_seek_to(struct table_iter *ti, uint64_t off, uint8_t typ)
 
        ti->typ = reftable_block_type(&ti->block);
        ti->block_off = off;
-       block_iter_seek_start(&ti->bi, &ti->block);
+       block_iter_init(&ti->bi, &ti->block);
        ti->is_finished = 0;
        return 0;
 }
@@ -349,7 +349,8 @@ static int table_iter_seek_linear(struct table_iter *ti,
         * the wanted key inside of it. If the block does not contain our key
         * we know that the corresponding record does not exist.
         */
-       err = block_iter_seek_key(&ti->bi, &ti->block, &want_key);
+       block_iter_init(&ti->bi, &ti->block);
+       err = block_iter_seek_key(&ti->bi, &want_key);
        if (err < 0)
                goto done;
        err = 0;
@@ -417,7 +418,9 @@ static int table_iter_seek_indexed(struct table_iter *ti,
                if (err != 0)
                        goto done;
 
-               err = block_iter_seek_key(&ti->bi, &ti->block, &want_index.u.idx.last_key);
+               block_iter_init(&ti->bi, &ti->block);
+
+               err = block_iter_seek_key(&ti->bi, &want_index.u.idx.last_key);
                if (err < 0)
                        goto done;
 

diff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c
index e36ed7ac576e7838661cfde6e7ae80b38d958b3e..9f91d697f6dfe96a285bea2e0c71d86fbec1ef3a 100644 (file)
--- a/t/unit-tests/t-reftable-block.c
+++ b/t/unit-tests/t-reftable-block.c
@@ -66,7 +66,7 @@ static void t_ref_block_read_write(void)
        block_source_from_buf(&source ,&block_data);
        reftable_block_init(&block, &source, 0, header_off, block_size, REFTABLE_HASH_SIZE_SHA1);
 
-       block_iter_seek_start(&it, &block);
+       block_iter_init(&it, &block);
 
        for (i = 0; ; i++) {
                ret = block_iter_next(&it, &rec);
@@ -79,10 +79,9 @@ static void t_ref_block_read_write(void)
        }
 
        for (i = 0; i < N; i++) {
-               block_iter_reset(&it);
                reftable_record_key(&recs[i], &want);
 
-               ret = block_iter_seek_key(&it, &block, &want);
+               ret = block_iter_seek_key(&it, &want);
                check_int(ret, ==, 0);
 
                ret = block_iter_next(&it, &rec);
@@ -91,7 +90,7 @@ static void t_ref_block_read_write(void)
                check(reftable_record_equal(&recs[i], &rec, REFTABLE_HASH_SIZE_SHA1));
 
                want.len--;
-               ret = block_iter_seek_key(&it, &block, &want);
+               ret = block_iter_seek_key(&it, &want);
                check_int(ret, ==, 0);
 
                ret = block_iter_next(&it, &rec);
@@ -156,7 +155,7 @@ static void t_log_block_read_write(void)
        block_source_from_buf(&source, &block_data);
        reftable_block_init(&block, &source, 0, header_off, block_size, REFTABLE_HASH_SIZE_SHA1);
 
-       block_iter_seek_start(&it, &block);
+       block_iter_init(&it, &block);
 
        for (i = 0; ; i++) {
                ret = block_iter_next(&it, &rec);
@@ -169,11 +168,10 @@ static void t_log_block_read_write(void)
        }
 
        for (i = 0; i < N; i++) {
-               block_iter_reset(&it);
                reftable_buf_reset(&want);
                check(!reftable_buf_addstr(&want, recs[i].u.log.refname));
 
-               ret = block_iter_seek_key(&it, &block, &want);
+               ret = block_iter_seek_key(&it, &want);
                check_int(ret, ==, 0);
 
                ret = block_iter_next(&it, &rec);
@@ -182,7 +180,7 @@ static void t_log_block_read_write(void)
                check(reftable_record_equal(&recs[i], &rec, REFTABLE_HASH_SIZE_SHA1));
 
                want.len--;
-               ret = block_iter_seek_key(&it, &block, &want);
+               ret = block_iter_seek_key(&it, &want);
                check_int(ret, ==, 0);
 
                ret = block_iter_next(&it, &rec);
@@ -249,7 +247,7 @@ static void t_obj_block_read_write(void)
        block_source_from_buf(&source, &block_data);
        reftable_block_init(&block, &source, 0, header_off, block_size, REFTABLE_HASH_SIZE_SHA1);
 
-       block_iter_seek_start(&it, &block);
+       block_iter_init(&it, &block);
 
        for (i = 0; ; i++) {
                ret = block_iter_next(&it, &rec);
@@ -262,10 +260,9 @@ static void t_obj_block_read_write(void)
        }
 
        for (i = 0; i < N; i++) {
-               block_iter_reset(&it);
                reftable_record_key(&recs[i], &want);
 
-               ret = block_iter_seek_key(&it, &block, &want);
+               ret = block_iter_seek_key(&it, &want);
                check_int(ret, ==, 0);
 
                ret = block_iter_next(&it, &rec);
@@ -334,7 +331,7 @@ static void t_index_block_read_write(void)
        block_source_from_buf(&source, &block_data);
        reftable_block_init(&block, &source, 0, header_off, block_size, REFTABLE_HASH_SIZE_SHA1);
 
-       block_iter_seek_start(&it, &block);
+       block_iter_init(&it, &block);
 
        for (i = 0; ; i++) {
                ret = block_iter_next(&it, &rec);
@@ -347,10 +344,9 @@ static void t_index_block_read_write(void)
        }
 
        for (i = 0; i < N; i++) {
-               block_iter_reset(&it);
                reftable_record_key(&recs[i], &want);
 
-               ret = block_iter_seek_key(&it, &block, &want);
+               ret = block_iter_seek_key(&it, &want);
                check_int(ret, ==, 0);
 
                ret = block_iter_next(&it, &rec);
@@ -359,7 +355,7 @@ static void t_index_block_read_write(void)
                check(reftable_record_equal(&recs[i], &rec, REFTABLE_HASH_SIZE_SHA1));
 
                want.len--;
-               ret = block_iter_seek_key(&it, &block, &want);
+               ret = block_iter_seek_key(&it, &want);
                check_int(ret, ==, 0);
 
                ret = block_iter_next(&it, &rec);