On 09/18/2014 06:32 AM, Torsten Bögershausen wrote:
> On 09/16/2014 09:33 PM, Michael Haggerty wrote:
> []
>>
>> diff --git a/lockfile.c b/lockfile.c
>> index 983c3ec..00c972c 100644
>> --- a/lockfile.c
>> +++ b/lockfile.c
>> @@ -129,6 +129,22 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
>> */
>> static const size_t max_path_len = sizeof(lk->filename) - 5;
>>
>> + if (!lock_file_list) {
>> + /* One-time initialization */
>> + sigchain_push_common(remove_lock_file_on_signal);
>> + atexit(remove_lock_file);
>> + }
>> +
>> + if (!lk->on_list) {
>> + /* Initialize *lk and add it to lock_file_list: */
>> + lk->fd = -1;
>> + lk->owner = 0;
>> + lk->on_list = 1;
>> + lk->filename[0] = 0;
> Does it makes sense to change the order here:
>
> Do the full initialization, and once that is completed, set on_list = 1
> + lk->filename[0] = 0;
> + lk->on_list = 1;
>From a functional standpoint, it doesn't matter. This function is the
only place that uses on_list, and it is basically only used to make sure
that each lock_file structure is initialized and registered in
lock_file_list exactly once. In particular, the signal handling code
doesn't care about the on_list field.
So the only important timing requirement WRT on_list is that it be set
before this function is called again with the same lock_file object. But
any code that would call this function twice, simultaneously, with the
same lock_file argument would be broken far more seriously than could be
fixed by changing the order that the fields are initialized.
But I guess you are right that it looks more natural to set this field
only after all of the initialization is done. I will make the change.
Michael
--
Michael Haggerty
mhagger@xxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html