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

From: Jeff King <peff@peff.net>
Date: Sep 2, 2017, 13:50 UTC
Message-ID: <20170902135015.2hf4b3o4xivdd5x2@sigill.intra.peff.net>
In-Reply-To: <20170902084451.h5pmbtolqcl2j3w4@sigill.intra.peff.net>

On Sat, Sep 02, 2017 at 04:44:51AM -0400, Jeff King wrote:

Show 7 quoted lines

> There are actually very few callers of create_tempfile (I count two).
> Everybody just uses lock_file(). So I think we could get away with just
> modifying the internals of the lock struct to hold a pointer, and then
> teaching commit_lock_file() et al to reset it to NULL after performing
> an operation that frees the tempfile. We could even modify
> rename_tempfile() and friends to take a pointer-to-pointer and NULL it
> itself. That would make it harder to get wrong.

I take it back. It's true that there are very few create_tempfile() callers, but we'd want to have a similar change for mks_tempfile_*, and there are a lot more of those.

One solution would be to add an extra level of indirection, like:

  struct tempfile {
	struct the_part_that_we_put_on_our_global_list *data;
  };

That lets us keep normal value semantics for "struct tempfile", which is nice. And existing callers wouldn't have to be modified at all for the creation/deletion bits. But I suspect it would also result in a lot of replacements like s/temp->/temp.data->/ for any callers that look at the underlying fields that have moved into the sub-struct.

-Peff