diff mbox series

[03/16] reftable/blocksource: consolidate code into a single file

Message ID 20250331-pks-reftable-polishing-v1-3-ebed5247434c@pks.im (mailing list archive)
State New
Headers show
Series reftable: overhaul the API to expose access to blocks | expand

Commit Message

Patrick Steinhardt March 31, 2025, 8:41 a.m. UTC
The code that implements block sources is distributed across a couple of
files even though. Consolidate all of it into "reftable/blocksource.c"
and its accompanying header so that it is easier to locate and more self
contained.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/block.c                    | 17 +++--------------
 reftable/block.h                    |  3 ---
 reftable/blocksource.c              | 35 +++++++++++++++++++++++++++++++++++
 reftable/blocksource.h              | 27 ++++++++++++++++++++++++++-
 reftable/iter.c                     |  5 +++--
 reftable/reftable-blocksource.h     |  3 ++-
 reftable/table.c                    | 33 +++++----------------------------
 reftable/table.h                    |  7 -------
 t/unit-tests/t-reftable-block.c     |  8 ++++----
 t/unit-tests/t-reftable-readwrite.c |  4 ++--
 10 files changed, 80 insertions(+), 62 deletions(-)

Comments

Justin Tobler April 2, 2025, 5:42 p.m. UTC | #1
On 25/03/31 10:41AM, Patrick Steinhardt wrote:
> The code that implements block sources is distributed across a couple of
> files even though. Consolidate all of it into "reftable/blocksource.c"

even though... ? I think you meant to followup with something here.

> and its accompanying header so that it is easier to locate and more self
> contained.

It looks like some of the block source function get renamed in the
process. It might we worth mentioning that in the commit message too.
Other than that, everything else in this patch seems to be a simple
reorganization. Looks good!

-Justin
diff mbox series

Patch

diff --git a/reftable/block.c b/reftable/block.c
index a5734d44415..97740187259 100644
--- a/reftable/block.c
+++ b/reftable/block.c
@@ -221,7 +221,7 @@  int block_reader_init(struct block_reader *br, struct reftable_block *block,
 	uint32_t restart_start = 0;
 	uint8_t *restart_bytes = NULL;
 
-	reftable_block_done(&br->block);
+	block_source_return_block(&br->block);
 
 	if (!reftable_is_block_type(typ)) {
 		err =  REFTABLE_FORMAT_ERROR;
@@ -285,7 +285,7 @@  int block_reader_init(struct block_reader *br, struct reftable_block *block,
 		}
 
 		/* We're done with the input data. */
-		reftable_block_done(block);
+		block_source_return_block(block);
 		block->data = br->uncompressed_data;
 		block->len = sz;
 		full_block_size = src_len + block_header_skip - br->zstream->avail_in;
@@ -324,7 +324,7 @@  void block_reader_release(struct block_reader *br)
 	inflateEnd(br->zstream);
 	reftable_free(br->zstream);
 	reftable_free(br->uncompressed_data);
-	reftable_block_done(&br->block);
+	block_source_return_block(&br->block);
 }
 
 uint8_t block_reader_type(const struct block_reader *r)
@@ -570,14 +570,3 @@  void block_writer_release(struct block_writer *bw)
 	reftable_buf_release(&bw->last_key);
 	/* the block is not owned. */
 }
-
-void reftable_block_done(struct reftable_block *blockp)
-{
-	struct reftable_block_source source = blockp->source;
-	if (blockp && source.ops)
-		source.ops->return_block(source.arg, blockp);
-	blockp->data = NULL;
-	blockp->len = 0;
-	blockp->source.ops = NULL;
-	blockp->source.arg = NULL;
-}
diff --git a/reftable/block.h b/reftable/block.h
index eaeffdffc90..203b07d9a44 100644
--- a/reftable/block.h
+++ b/reftable/block.h
@@ -142,7 +142,4 @@  size_t header_size(int version);
 /* size of file footer, depending on format version */
 size_t footer_size(int version);
 
-/* returns a block to its source. */
-void reftable_block_done(struct reftable_block *ret);
-
 #endif
diff --git a/reftable/blocksource.c b/reftable/blocksource.c
index 1397cbe7800..bc785506fb1 100644
--- a/reftable/blocksource.c
+++ b/reftable/blocksource.c
@@ -13,6 +13,41 @@ 
 #include "reftable-blocksource.h"
 #include "reftable-error.h"
 
+void block_source_return_block(struct reftable_block *block)
+{
+	struct reftable_block_source source = block->source;
+	if (block && source.ops)
+		source.ops->return_block(source.arg, block);
+	block->data = NULL;
+	block->len = 0;
+	block->source.ops = NULL;
+	block->source.arg = NULL;
+}
+
+void block_source_close(struct reftable_block_source *source)
+{
+	if (!source->ops) {
+		return;
+	}
+
+	source->ops->close(source->arg);
+	source->ops = NULL;
+}
+
+ssize_t block_source_read_block(struct reftable_block_source *source,
+				struct reftable_block *dest, uint64_t off,
+				uint32_t size)
+{
+	ssize_t result = source->ops->read_block(source->arg, dest, off, size);
+	dest->source = *source;
+	return result;
+}
+
+uint64_t block_source_size(struct reftable_block_source *source)
+{
+	return source->ops->size(source->arg);
+}
+
 static void reftable_buf_return_block(void *b REFTABLE_UNUSED, struct reftable_block *dest)
 {
 	if (dest->len)
diff --git a/reftable/blocksource.h b/reftable/blocksource.h
index 7b67898ae22..639b9a1a3c5 100644
--- a/reftable/blocksource.h
+++ b/reftable/blocksource.h
@@ -12,9 +12,34 @@ 
 #include "system.h"
 
 struct reftable_block_source;
+struct reftable_block;
 struct reftable_buf;
 
-/* Create an in-memory block source for reading reftables */
+/*
+ * Close the block source and the underlying resource. This is a no-op in case
+ * the block source is zero-initialized.
+ */
+void block_source_close(struct reftable_block_source *source);
+
+/*
+ * Read a block of length `size` from the source at the given `off`.
+ */
+ssize_t block_source_read_block(struct reftable_block_source *source,
+				struct reftable_block *dest, uint64_t off,
+				uint32_t size);
+
+/*
+ * Return the total length of the underlying resource.
+ */
+uint64_t block_source_size(struct reftable_block_source *source);
+
+/*
+ * Return a block to its original source, releasing any resources associated
+ * with it.
+ */
+void block_source_return_block(struct reftable_block *block);
+
+/* Create an in-memory block source for reading reftables. */
 void block_source_from_buf(struct reftable_block_source *bs,
 			   struct reftable_buf *buf);
 
diff --git a/reftable/iter.c b/reftable/iter.c
index 7376f263c99..6af6eb49396 100644
--- a/reftable/iter.c
+++ b/reftable/iter.c
@@ -11,6 +11,7 @@ 
 #include "system.h"
 
 #include "block.h"
+#include "blocksource.h"
 #include "constants.h"
 #include "reftable-error.h"
 #include "table.h"
@@ -113,7 +114,7 @@  static void indexed_table_ref_iter_close(void *p)
 {
 	struct indexed_table_ref_iter *it = p;
 	block_iter_close(&it->cur);
-	reftable_block_done(&it->block_reader.block);
+	block_source_return_block(&it->block_reader.block);
 	reftable_free(it->offsets);
 	reftable_buf_release(&it->oid);
 }
@@ -127,7 +128,7 @@  static int indexed_table_ref_iter_next_block(struct indexed_table_ref_iter *it)
 		return 1;
 	}
 
-	reftable_block_done(&it->block_reader.block);
+	block_source_return_block(&it->block_reader.block);
 
 	off = it->offsets[it->offset_idx++];
 	err = table_init_block_reader(it->table, &it->block_reader, off,
diff --git a/reftable/reftable-blocksource.h b/reftable/reftable-blocksource.h
index 8692cd017e9..96430b629e4 100644
--- a/reftable/reftable-blocksource.h
+++ b/reftable/reftable-blocksource.h
@@ -11,7 +11,8 @@ 
 
 #include <stdint.h>
 
-/* block_source is a generic wrapper for a seekable readable file.
+/*
+ * Generic wrapper for a seekable readable file.
  */
 struct reftable_block_source {
 	struct reftable_block_source_vtable *ops;
diff --git a/reftable/table.c b/reftable/table.c
index 440fb559ad1..d18e17b0d44 100644
--- a/reftable/table.c
+++ b/reftable/table.c
@@ -10,35 +10,12 @@ 
 
 #include "system.h"
 #include "block.h"
+#include "blocksource.h"
 #include "constants.h"
 #include "iter.h"
 #include "record.h"
 #include "reftable-error.h"
 
-uint64_t block_source_size(struct reftable_block_source *source)
-{
-	return source->ops->size(source->arg);
-}
-
-ssize_t block_source_read_block(struct reftable_block_source *source,
-				struct reftable_block *dest, uint64_t off,
-				uint32_t size)
-{
-	ssize_t result = source->ops->read_block(source->arg, dest, off, size);
-	dest->source = *source;
-	return result;
-}
-
-void block_source_close(struct reftable_block_source *source)
-{
-	if (!source->ops) {
-		return;
-	}
-
-	source->ops->close(source->arg);
-	source->ops = NULL;
-}
-
 static struct reftable_table_offsets *
 table_offsets_for(struct reftable_table *t, uint8_t typ)
 {
@@ -249,7 +226,7 @@  int table_init_block_reader(struct reftable_table *t, struct block_reader *br,
 	}
 
 	if (block_size > guess_block_size) {
-		reftable_block_done(&block);
+		block_source_return_block(&block);
 		err = table_get_block(t, &block, next_off, block_size);
 		if (err < 0) {
 			goto done;
@@ -259,7 +236,7 @@  int table_init_block_reader(struct reftable_table *t, struct block_reader *br,
 	err = block_reader_init(br, &block, header_off, t->block_size,
 				hash_size(t->hash_id));
 done:
-	reftable_block_done(&block);
+	block_source_return_block(&block);
 
 	return err;
 }
@@ -666,8 +643,8 @@  int reftable_table_new(struct reftable_table **out,
 	*out = t;
 
 done:
-	reftable_block_done(&footer);
-	reftable_block_done(&header);
+	block_source_return_block(&footer);
+	block_source_return_block(&header);
 	if (err) {
 		if (t)
 			reftable_free(t->name);
diff --git a/reftable/table.h b/reftable/table.h
index 9cd8f80a207..8d8dd2b413d 100644
--- a/reftable/table.h
+++ b/reftable/table.h
@@ -14,13 +14,6 @@ 
 #include "reftable-iterator.h"
 #include "reftable-table.h"
 
-uint64_t block_source_size(struct reftable_block_source *source);
-
-ssize_t block_source_read_block(struct reftable_block_source *source,
-				struct reftable_block *dest, uint64_t off,
-				uint32_t size);
-void block_source_close(struct reftable_block_source *source);
-
 /* metadata for a block type */
 struct reftable_table_offsets {
 	int is_present;
diff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c
index 22040aeefa5..8bb40482347 100644
--- a/t/unit-tests/t-reftable-block.c
+++ b/t/unit-tests/t-reftable-block.c
@@ -100,7 +100,7 @@  static void t_ref_block_read_write(void)
 	block_reader_release(&br);
 	block_iter_close(&it);
 	reftable_record_release(&rec);
-	reftable_block_done(&br.block);
+	block_source_return_block(&br.block);
 	reftable_buf_release(&want);
 	reftable_buf_release(&buf);
 	for (i = 0; i < N; i++)
@@ -190,7 +190,7 @@  static void t_log_block_read_write(void)
 	block_reader_release(&br);
 	block_iter_close(&it);
 	reftable_record_release(&rec);
-	reftable_block_done(&br.block);
+	block_source_return_block(&br.block);
 	reftable_buf_release(&want);
 	reftable_buf_release(&buf);
 	for (i = 0; i < N; i++)
@@ -273,7 +273,7 @@  static void t_obj_block_read_write(void)
 	block_reader_release(&br);
 	block_iter_close(&it);
 	reftable_record_release(&rec);
-	reftable_block_done(&br.block);
+	block_source_return_block(&br.block);
 	reftable_buf_release(&want);
 	reftable_buf_release(&buf);
 	for (i = 0; i < N; i++)
@@ -365,7 +365,7 @@  static void t_index_block_read_write(void)
 	block_reader_release(&br);
 	block_iter_close(&it);
 	reftable_record_release(&rec);
-	reftable_block_done(&br.block);
+	block_source_return_block(&br.block);
 	reftable_buf_release(&want);
 	reftable_buf_release(&buf);
 	for (i = 0; i < N; i++)
diff --git a/t/unit-tests/t-reftable-readwrite.c b/t/unit-tests/t-reftable-readwrite.c
index c4c27242ba9..3fba888cdaa 100644
--- a/t/unit-tests/t-reftable-readwrite.c
+++ b/t/unit-tests/t-reftable-readwrite.c
@@ -32,13 +32,13 @@  static void t_buffer(void)
 	n = block_source_read_block(&source, &out, 0, sizeof(in));
 	check_int(n, ==, sizeof(in));
 	check(!memcmp(in, out.data, n));
-	reftable_block_done(&out);
+	block_source_return_block(&out);
 
 	n = block_source_read_block(&source, &out, 1, 2);
 	check_int(n, ==, 2);
 	check(!memcmp(out.data, "el", 2));
 
-	reftable_block_done(&out);
+	block_source_return_block(&out);
 	block_source_close(&source);
 	reftable_buf_release(&buf);
 }