ab/commit-plug-leaks (was: What's cooking in git.git (Mar 2022, #05; Wed, 23))

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, Mar 23 2022, Junio C Hamano wrote:

> * ab/commit-plug-leaks (2022-02-16) 2 commits
>  - commit: use strbuf_release() instead of UNLEAK()
>  - commit: fix "author_ident" leak
>
>  Leakfixes in the top-level called-once function.
>
>  Expecting a reroll.
>  I think UNLEAK->strbuf_release() is a regression.
>  source: <cover-0.2-00000000000-20220216T081844Z-avarab@gmail.com>

Re our earlier exchange ending in
https://lore.kernel.org/git/xmqqczjbj0nf.fsf@gitster.g/ I still think it
makes sense to get rid of the UNLEAK() there.

One reason not mentioned there (but which I do find useful) is that if
we actually free the memory then you don't need to build with
-DSUPPRESS_ANNOTATED_LEAKS to suppress these, which is useful e.g. when
looking at valgrind output, as opposed to SANITIZE=leak where we do add
-DSUPPRESS_ANNOTATED_LEAKS.

And since I submitted this topic our number of UNLEAK() in builtin/ is
down from 29 to 23 with the queued release_revisions() topic.

But you seem to feel strongly that we should keep these specific
UNLEAK() for reasons I don't really understand despite re-reading that
exchange. I.e. the cost of doing the actual release is minuscule, and we
have a lot of such strbuf_release() in builtin/*.c already.

But if you'd just like to drop this topic I understand, but I think it's
ready to advance as is, but either way it's probably good to get it out
of the current status