| Message ID | 9a1253649a4dc993da7caced2f15839d988905d9.1711519925.git.ps@pks.im (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | reftable: optimize table and block iterators | expand |
Patrick Steinhardt <ps@pks.im> writes: > > diff --git a/reftable/reader.c b/reftable/reader.c > index f70efa2b7c..f925570bf3 100644 > --- a/reftable/reader.c > +++ b/reftable/reader.c > @@ -253,7 +253,7 @@ static void table_iter_block_done(struct table_iter *ti) > if (!ti->bi.br) { > return; > } > - reftable_block_done(&ti->bi.br->block); > + block_reader_release(ti->bi.br); > FREE_AND_NULL(ti->bi.br); > > ti->bi.last_key.len = 0; I would expect `FREE_AND_NULL(ti->bi.br)` to also be within `block_reader_release`, but then you'd have to pass `table_iter` or `**`. So I guess this is okay.
On Wed, Apr 03, 2024 at 06:16:35AM -0700, Karthik Nayak wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > > diff --git a/reftable/reader.c b/reftable/reader.c > > index f70efa2b7c..f925570bf3 100644 > > --- a/reftable/reader.c > > +++ b/reftable/reader.c > > @@ -253,7 +253,7 @@ static void table_iter_block_done(struct table_iter *ti) > > if (!ti->bi.br) { > > return; > > } > > - reftable_block_done(&ti->bi.br->block); > > + block_reader_release(ti->bi.br); > > FREE_AND_NULL(ti->bi.br); > > > > ti->bi.last_key.len = 0; > > I would expect `FREE_AND_NULL(ti->bi.br)` to also be within > `block_reader_release`, but then you'd have to pass `table_iter` or > `**`. So I guess this is okay. It is customary in the Git codebase to discern `release` and `free` functions. `release` will release all memory hosted by a structure, but will not release the structure itself. `free` on the other hand is expected to free both the memory hosted by a structure, and the structure itself. `block_reader_release()` thus shouldn't free the `struct block_reader`. Patrick
diff --git a/reftable/block.c b/reftable/block.c index 6b78dd3424..013849f028 100644 --- a/reftable/block.c +++ b/reftable/block.c @@ -256,6 +256,11 @@ int block_reader_init(struct block_reader *br, struct reftable_block *block, return err; } +void block_reader_release(struct block_reader *br) +{ + reftable_block_done(&br->block); +} + uint8_t block_reader_type(struct block_reader *r) { return r->block.data[r->header_off]; diff --git a/reftable/block.h b/reftable/block.h index d73ed73549..601a1e0e89 100644 --- a/reftable/block.h +++ b/reftable/block.h @@ -81,6 +81,8 @@ int block_reader_init(struct block_reader *br, struct reftable_block *bl, uint32_t header_off, uint32_t table_block_size, int hash_size); +void block_reader_release(struct block_reader *br); + /* Returns the block type (eg. 'r' for refs) */ uint8_t block_reader_type(struct block_reader *r); diff --git a/reftable/reader.c b/reftable/reader.c index f70efa2b7c..f925570bf3 100644 --- a/reftable/reader.c +++ b/reftable/reader.c @@ -253,7 +253,7 @@ static void table_iter_block_done(struct table_iter *ti) if (!ti->bi.br) { return; } - reftable_block_done(&ti->bi.br->block); + block_reader_release(ti->bi.br); FREE_AND_NULL(ti->bi.br); ti->bi.last_key.len = 0;
Introduce a new function `block_reader_release()` that releases resources acquired by the block reader. This function will be extended in a subsequent commit. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- reftable/block.c | 5 +++++ reftable/block.h | 2 ++ reftable/reader.c | 2 +- 3 files changed, 8 insertions(+), 1 deletion(-)