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@xxxxxxxxx> 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
