Re: [PATCH v3 18/25] struct lock_file: declare some fields volatile

> Am 4/14/2014 15:54, schrieb Michael Haggerty:
>> The function remove_lock_file_on_signal() is used as a signal handler.
>> It is not realistic to make the signal handler conform strictly to the
>> C standard, which is very restrictive about what a signal handler is
>> allowed to do.  But let's increase the likelihood that it will work:
>>
>> The lock_file_list global variable and several fields from struct
>> lock_file are used by the signal handler.  Declare those values
>> "volatile" to increase the chance that the signal handler will see a
>> valid object state.
> 
> Yes, it's important that the signal handler sees a valid object state, and
> "volatile" can help here. But I think the reason why it helps is not
> obvious, and it should be mentioned in the commit message:
> 
> It is not so much that "volatile" forces the compiler to lay down each
> access of the variable coded in C in the assembly code, but more
> importantly, that "volatile" disallows any re-ordering of these accesses.
> Then:
> 
> - 'lock->active = 1' must be the last assignment during setup
> 
> - 'lock->active = 0' must be the first assignment during tear-down.
> 
> - Ideally, all members of struct lock_file should be "volatile".
> 
> The last point is important because the compiler is allowed to re-order
> accesses to non-"volatile" variables across "volatile" accesses. I say
> "ideally" because if filename were defined "volatile filename[PATH_MAX]",
> strcpy() could not be used anymore. OTOH, it is unlikely that a compiler
> re-orders a strcpy() call across other expressions, and we can get away
> without "volatile" in the "filename" case in practice.

>> Suggested-by: Johannes Sixt <j.sixt@viscovery.net>
> 
> Not a big deal, but just in case you re-roll again and you do not forget:
> 
>   Johannes Sixt <j6t@kdbg.org>
> 
> is preferred.