Very nice fix, thanks again for doing it!
Jeff King <peff@peff.net> 于2025年3月4日周二 14:33写道：
Show 45 quoted lines
>
> On Sat, Mar 01, 2025 at 12:31:33PM +0100, René Scharfe wrote:
>
> > --- >8 ---
> > Subject: [PATCH] reftable: release name on reftable_reader_new() error
> >
> > If block_source_read_block() or parse_footer() fail, we leak the "name"
> > member of struct reftable_reader in reftable_reader_new(). Release it.
> >
> > Reported by: H Z <shiyuyuranzh@gmail.com>
> > Signed-off-by: René Scharfe <l.s.r@web.de>
> > ---
> > reftable/reader.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/reftable/reader.c b/reftable/reader.c
> > index 3f2e4b2800..f38c83f140 100644
> > --- a/reftable/reader.c
> > +++ b/reftable/reader.c
> > @@ -666,6 +666,7 @@ int reftable_reader_new(struct reftable_reader **out,
> > reftable_block_done(&footer);
> > reftable_block_done(&header);
> > if (err) {
> > + reftable_free(r->name);
> > reftable_free(r);
> > block_source_close(source);
> > }
>
> Coverity complains that "r" might be NULL here. At the top of the
> function we do:
>
> REFTABLE_CALLOC_ARRAY(r, 1);
> if (!r) {
> err = REFTABLE_OUT_OF_MEMORY_ERROR;
> goto done;
> }
>
> and then the done label hits your new line (the "done:" is right above
> the context in your patch). And err of course is non-zero.
>
> So this probably needs an "if (r)", or multiple layered out-labels. Or
> alternatively we could return directly when the first allocation fails,
> since there is nothing to clean up at that point.
>
> -Peff