Junio C Hamano <gitster@xxxxxxxxx> writes: > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > >> Convert the UNLEAK() added in 0e5bba53af7 (add UNLEAK annotation for >> reducing leak false positives, 2017-09-08) to release the memory using >> strbuf_release() instead. >> >> The tests being marked as passing with >> "TEST_PASSES_SANITIZE_LEAK=true" already passed before due to the >> UNLEAK(), but now they really don't leak memory, so let's mark them as >> such. > > That smells like a brave move. > > Specifically, the cited commit turned an existing strbuf_release() > on &err into UNLEAK(). If that and the other strbuf (sb) were so > easily releasable, why didn't we do so back then already? I suspect that the answer to the above question is because these allocations are in the top-level cmd_commit() function, which is never called recursively or repeatedly as a subroutine. The only significant thing that happens after we return from it is to exit. In such a code path, marking a variable as UNLEAK() is a better thing to do than calling strbuf_release(). Both will work as a way to squelch sanitizers from reporting a leak that does not matter, but calling strbuf_release() means we'd spend extra cycles to return pieces of memory to the pool, even though we know that the pool itself will be cleaned immediately later at exit. We already have UNLEAK to tell sanitizers not to distract us from spotting and plugging real leaks by reporting these apparent leaks that do not matter. It is of somewhat dubious value to do a "we care too much about pleasing sanitizer and spend extra cycles at runtime while real users are doing real work" change.
