Re: [PATCH v3 2/2] t/helper/test-delta: fix possible resource leak and ensure safe cleanup

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jul 23, 2025, 16:37 UTC
Message-ID: <CAPig+cSzYPaBGZ_muhxwcNy4xWaMF5Uc0+SN7ygnS4wNaGxEKQ@mail.gmail.com>
In-Reply-To: <20250723084624.GA580010@coredump.intra.peff.net>

On Wed, Jul 23, 2025 at 4:46 AM Jeff King <peff@peff.net> wrote:

Show 13 quoted lines

> On Wed, Jul 23, 2025 at 04:11:26AM -0400, Eric Sunshine wrote:
> > For completeness, I'll mention that I even had the thought that
> > another "fix" would be to tear out all the cleanup code entirely since
> > we _know_ that this function will be exiting immediately and the OS
> > will clean up any dangling resources.
>
> The reason we have the "cleanup" label at all is because of the memory
> leaks. And there the issue is that we build the test helpers with the
> same compiler settings as the rest of the code, so SANITIZE=leak will
> complain. So I think that is a non-starter.
>
> But if you just meant leaking descriptors, sure, I don't think any tools
> complain about that. ;)

At the time the momentary (but not really serious) thought flashed through my brain, I probably was imagining dropping all of the cleanup code; I almost certainly wasn't thinking about the SANITIZE=leak case. But immediately upon seeing your idea to use die(), I recognized it (and liked it) as a superior version of my half-baked thought.