Re: [PATCH 4/9] reftable/block: introduce `block_reader_release()`

From: Patrick Steinhardt <ps@pks.im>
To: Karthik Nayak <karthik.188@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 4/9] reftable/block: introduce `block_reader_release()`
Date: Mon, 8 Apr 2024 14:10:52 +0200	[thread overview]
Message-ID: <ZhPezL_8Ozf375FI@tanuki> (raw)
In-Reply-To: <CAOLa=ZQ4Vq7YfUsh4mky8HsEurp3RyUEXsZS7tDU0XMc+6Gv0w@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1107 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]