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

From: Jeff King <peff@peff.net>
Date: May 8, 2018, 18:25 UTC
Message-ID: <20180508182548.GD7210@sigill.intra.peff.net>
In-Reply-To: <20180506141031.30204-1-martin.agren@gmail.com>

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

Show 13 quoted lines

> 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