Patrick Steinhardt <ps@xxxxxx> writes:
> 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.
>
So 'reftable/blocksource.c' is an existing file and we're moving related
content into this file. Makes sense.
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
> 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(-)
>
> 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);
> }
>
> --
> 2.49.0.604.gff1f9ca942.dirty
The changes look good to me!
Attachment:
signature.asc
Description: PGP signature
