Web lists-archives.com

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




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

> 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