Re: [PATCH 0/5] getting rid of most "static struct lock_file"s

[Jeff King <peff@peff.net>]

On Sun, May 06, 2018 at 04:10:26PM +0200, Martin Ågren wrote:

> This series addresses two classes of "static struct lock_file", removing
> the staticness: Those locks that already live inside a function, and
> those that can simply be moved into the function they are used from.
> 
> The first three patches are some cleanups I noticed along the way, where
> we first take a lock using LOCK_DIE_ON_ERROR, then check whether we got
> it.
> 
> After this series, we have a small number of "static struct lock_file"
> left, namely those locks that are used from within more than one
> function. Thus, after this series, when one stumbles upon a static
> lock_file, it should be a clear warning that the lock is being
> used by more than one function.

These all look fine to me. The commit messages all made perfect sense to
me, but it sounds like some people weren't aware of the new
post-076aa2cbda rules. So maybe it makes sense to reference that even in
the earlier commits, and to explicitly say that it's safe to convert
even in the case where the lock_file goes out of scope while still
active.

The only dangerous thing left to check for is anybody holding onto a
pointer-to-lockfile. The only such pointer declared outside of a
parameter list is in create_reflock(), and there it's just to
temporarily coerce a void pointer. So unless somebody is doing something
really tricky (putting a pointer-to-lock in a "void *"), I think these
conversions all have to be trivially correct (famous last words...).

-Peff