Re: [PATCH v3 0/6] usage.c: add die_message() & plug memory leaks in refs.c & config.c

[Jonathan Tan <jonathantanmy@google.com>]

> What started as a set of small memory leak fixes is now adding and
> usinge a die_message() function. This non-fatal-die() is useful to
> various callers that want to print "fatal: " before exiting, but don't
> want to call die() for whatever reason.
> 
> I wasn't planning to submit that now, but these were incomplete
> patches I had lying around, and make the 5th and 6th patch much nicer,
> in response to comments on v1 and v2 to the effect that managing
> free()-ing around die() functions was rather nasty.
> 
> This doesn't conflict with anything in-flight, and the changes
> themselves are rather simple.

Is this mainly to make a CI work, or just so that a certain set of tools
work? (You mention an old version of GCC in patch 5.) If for CI, I think
that there might be a sufficient version to fix this, but if not, I
would think that something less intrusive would be better (e.g. a
comment that certain versions do not work).

As for patches 5 and 6, I think that any leak detection we use has to
consider any pointers still on the stack as "live" - if not we wouldn't
be able to die without returning back to the topmost function since any
intermediate function could have allocations (unless I'm mistaking
something). (Unless die() is somehow overwriting the stackframe through
some tail call optimization or something - in which case maybe what we
should do is to disable the tail call optimization when we are checking
for leaks.)