Web lists-archives.com

Re: [PATCH] pkt-line: re-'static'-ify buffer in packet_write_fmt_1()




On Mon, Aug 28, 2017 at 10:52:51AM -0700, Stefan Beller wrote:

> >> I'm curious, too. I don't think the valgrind setup in our test suite is
> >> great for finding leaks right now.
> 
> This discussion comes up every once in a while,
> the last time I was involved in this discussion I proposed
> to have an "optional_free(void *)", which only frees memory
> in e.g. the developer build/debug build.
> 
> That way we can have a strict "no leaks in developer build"
> policy (as it is easy to detect!), but it doesn't slow down the
> widely distributed version of Git.

Personally I am not that worried about slowing down program-exit with
some free() calls (though I would reserve judgement to see how slow it
actually is).

It is that I do not want to deal with the complexity and maintenance
headache of dropping values which are program-lifetime caches. If we
introduce a free-all-object-structs function, now everybody needs to
remember to call it (even if they didn't realize they were allocating
object structs in the first place, as it may have happened under the
hood of a sub-function). And we have to wonder what might happen when
somebody calls that function _not_ at program exit.

If we can declare "reachable things are not leaks" (and certainly
valgrind is aware of that distinction), then all of that goes away. And
you're just left with local variables in main() (or our cmd_*
equivalents) that appear as leaks. And then we can solve that either by
freeing them, or just calling them program-lifetime and telling the
compiler that it's so by declaring them "static".

-Peff