Re: [PATCH v2 10/16] overlayfs/file: Convert to cred_guard()

[Vinicius Costa Gomes <vinicius.gomes@intel.com>]

Amir Goldstein <amir73il@gmail.com> writes:
>> -       old_cred = ovl_override_creds_light(file_inode(file)->i_sb);
>> -       ret = vfs_fallocate(real.file, mode, offset, len);
>> -       revert_creds_light(old_cred);
>> +       cred_scoped_guard(ovl_creds(file_inode(file)->i_sb))
>> +               ret = vfs_fallocate(real.file, mode, offset, len);
>>
>
> I find this syntax confusing. Even though it is a valid syntax,
> I prefer that if there is a scope we use explicit brackets for it even
> if the scope is
> a single line.
>

Will add the brackets.

> How about using:
>        {
>                cred_guard(ovl_creds(file_inode(file)->i_sb));
>                ret = vfs_fallocate(real.file, mode, offset, len);
>        }
>
> It is more clear and helps averting the compiler bug(?).

I prefer the scoped_cred_guard() idiom, having it spelled out sounds
better to me. But a new block should avoid the bug as well.

>
> Maybe we should just place cred_guard(ovl_creds(file_inode(file_out)->i_sb))
> in ovl_copy_file_range()?
>
> I don't think that the order of ovl_override_creds() vs. inode_lock()
> really matters?
>

Most probably the order should not matter. Will change this.

> Thanks,
> Amir.


Cheers,
-- 
Vinicius