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 <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>

Author: pks-t

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;
 

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);

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;
 }
 

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;
 

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);