Re: [PATCH 1/2] commit-graph: clean up leaked memory during write

[Stefan Beller <sbeller@google.com>]

>
> My preference is to avoid them in the name of simplicity. If you're
> using "make SANITIZE=leak test" to check for leaks, it will skip these
> cases. If you're using valgrind, I think these may be reported as
> "reachable". But that number already isn't useful for finding real
> leaks, because it includes cases like the "foo" above as well as
> program-lifetime globals.
>
> The only argument (IMHO) for such an UNLEAK() is that it annotates the
> location for when somebody later changes the function to "return -1"
> instead of dying. But if we are going to do such annotation, we may as
> well actually call free(), which is what the "return" version will
> ultimately have to do.

Heh, that was part of my reasoning why we'd want to have *something*.

> I'd actually be _more_ favorable to calling free() instead of UNLEAK()
> there, but I'm still mildly negative, just because it may go stale (and
> our leak-checking wouldn't usefully notice these cases). Anybody
> converting that die() to a return needs to re-analyze the function for
> what might need to be released (and that includes non-memory bits like
> descriptors, too).

Sounds reasonable, so then the consensus (between Martin, you and me)
is to drop the UNLEAK.