From: Taylor Blau <me@ttaylorr.com>
Date: Tue, 09 Dec 2025 02:16:34 GMT
Subject: Re: [PATCH 15/17] t/helper/test-read-midx.c: plug memory leak when selecting layer
Message-ID: <aTeGgqxVO4xcuk6y@nand.local>
In-Reply-To: <aTcYhKOIu7ebJ_xV@pks.im>

On Mon, Dec 08, 2025 at 07:27:16PM +0100, Patrick Steinhardt wrote:
> > @@ -36,8 +37,11 @@ static int read_midx_file(const char *object_dir, const char *checksum,
> >  	if (checksum) {
> >  		while (m && strcmp(get_midx_checksum(m), checksum))
> >  			m = m->base_midx;
> > -		if (!m)
> > -			return 1;
> > +		if (!m) {
> > +			ret = error(_("could not find MIDX with checksum %s"),
> > +				    checksum);
> > +			goto out;
> > +		}
> >  	}
> >
> >  	printf("header: %08x %d %d %d %d\n",
>
> We change the return code from 1 to -1, but that ultimately shouldn't
> matter much.

Yeah; I think that returning negative values here makes more sense, and
use of error() encourages that pattern, hence the change here.

> I'll stop reviewing here and will have a look at the remaining two
> patches with some fresh eyes. But so far this was a nice read, thanks!

Thanks for the review thus far! I look forward to your thoughts on the
remainder of the series. In related news, I owe you some review on your
'pks/skip-noop-rewrite' patches, which I hope to get to tomorrow.

Thanks,
Taylor