Re: [PATCH] config: use a static lock_file struct

> On Wed, Aug 30, 2017 at 12:55:55AM -0400, Jeff King wrote:
> [...]
> The patch below demonstrates how this could be used to turn some
> "xcalloc" lock_files into stack variables that are allowed to go out of
> scope after we commit or rollback. This solves the three lock-related
> leaks reported by valgrind when running t0000.
> 
> _But_ it also demonstrates an interesting downside of this approach.
> Some functions are lazy in their error paths. For instance, look at
> write_index_as_tree(). We take the lock early in the function, but may
> return before a commit or rollback if we hit an error.  With the current
> code this "leaks" the tempfile, which is wrong. But nobody notices
> because in practice the program exits soon after and we clean up the
> tempfile then.
> 
> But with a stack variable lock_file, that minor problem becomes a major
> one: our stack variable got added to a global linked list and the
> atexit() handler will try to read it. But now of course it will contain
> garbage, since the variable went out of scope.
> 
> So it's probably safer to just let tempfile.c handle the whole lifetime,
> and have it put all live tempfiles on the heap, and free them when
> they're deactivated. That means our creation signature becomes more
> like:
> 
>   struct tempfile *create_tempfile(const char *path);
> 
> and delete_tempfile() actually calls free() on it (though we'd probably
> want to skip the free() from a signal handler for the usual reasons).