Re: [PATCH 00/11] midx: clean up t5319 under 'SANITIZE=leak'

From: Taylor Blau <me@ttaylorr.com>
Date: Oct 22, 2021, 04:39 UTC
Message-ID: <YXJAfICQN8s5Gm7s@nand.local>
In-Reply-To: <211021.861r4emv7b.gmgdl@evledraar.gmail.com>

On Thu, Oct 21, 2021 at 01:50:55PM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 13 quoted lines

>
> On Wed, Oct 20 2021, Taylor Blau wrote:
>
> > I tried to separate these out based on their respective areas. The last 10% are
> > all from leaking memory in the rev_info structure, which I punted on for now by
> > just UNLEAK()-ing it. That's all done in the last patch. If we choose to take
> > that last patch, then t5319 passes even under SANITIZE=leak builds. But it's a
> > little gross, so I'm happy to leave it out if others would prefer.
>
> I'll defer to you, but I've got a mild preference for keeping it out. An
> upcoming series of patches I'm submitting (I'm waiting on the current
> leak fixes to land) will fix most of those rev_info leaks. So not having
> UNLEAK() markings in-flight makes things a bit easier to manage.

If you don't mind, I think keeping this in (as a way to verify that we really did make t5319 leak-free) may be good for reviewers. When you clean these areas up, you'll have to remember to remove those UNLEAK()s, but hopefully they produce conflicts with your in-flight work that serve as not-too-annoying reminders.

I would certainly rather not have to UNLEAK() those at all, so I am very excited to see a series from you which handles freeing these resources appropriately. It was just too big a bite for me to chew off when preparing this quick series.

Thanks, Taylor