Re: [PATCH] format-patch: unleak "-v <num>"

From: Jeff King <peff@peff.net>
Date: Jan 16, 2023, 17:30 UTC
Message-ID: <Y8WJnGHs5nM5GwBM@coredump.intra.peff.net>
In-Reply-To: <xmqqv8l8gr6s.fsf@gitster.g>

On Sun, Jan 15, 2023 at 12:03:39AM -0800, Junio C Hamano wrote:

Show 11 quoted lines

> The "subject_prefix" member of "struct revision" usually is always
> set to a borrowed string (either a string literal like "PATCH" that
> appear in the program text as a hardcoded default, or the value of
> "format.subjectprefix") and is never freed when the containing
> revision structure is released.  The "-v <num>" codepath however
> violates this rule and stores a pointer to an allocated string to
> this member, relinquishing the responsibility to free it when it is
> done using the revision structure, leading to a small one-time leak.
> 
> Instead, keep track of the string it allocates to let the revision
> structure borrow, and clean it up when it is done.

FWIW, this looks obviously correct to me.

The word "unleak" in the subject made me think about UNLEAK(), so this is a small tangent. This is exactly the kind of case that I designed UNLEAK() for, because the solution really is "while you are assigning to X, also keep a copy of the pointer in Y to be freed later".

And UNLEAK() is just "keep a copy of the pointer in Y to know that we _could_ free it later". And of course "do nothing if we are not leak-detecting". But since we seem to be moving away from UNLEAK(), and since it would not even save any lines here, I'm perfectly happy with this solution.

-Peff