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

[Jeff King <peff@peff.net>]

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

> 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