Subject: Re: [PATCH v2 21/27] global: drop `UNLEAK()` annotation

From: Patrick Steinhardt <[email-masked]>
Date: Tue, 12 Nov 2024 09:53:28 +0100

On Tue, Nov 12, 2024 at 03:26:09AM -0500, Jeff King wrote:
> On Mon, Nov 11, 2024 at 11:38:50AM +0100, Patrick Steinhardt wrote:
> > This neatly demonstrates one of the issues with `UNLEAK()`: it is quite
> > easy for the annotation to become stale. A second issue is that its
> > whole intent is to paper over leaks. And while that has been a necessary
> > evil in the past, because Git was leaking left and right, it isn't
> > really much of an issue nowadays where our test suite has no known leaks
> > anymore.
> 
> I do agree that stale annotations are a weakness (they do not hurt the
> leak-checker if they exist, but they are an eye-sore).
> 
> I'm not sure I would agree that the intent was to paper over leaks. The
> point was to avoid noise from the leak-checker about memory that was
> intentionally held until program exit and released by returning from
> main(). I think the main thing that made it obsolete is that we decided
> it was worth it to spend the cycles freeing that memory rather than
> ignoring it.
> 
> But it's possible I'm just splitting hairs. :)

Yeah, I know that this was also used to mark memory that intentionally
leaks because we're about to exit anyway. I basically consider that as
some form of "papering over" it, but I get your comment that this may be
a bit too strongly worded.

Do you want me to reformulate this, or do we just go with the current
description?

Patrick