On 25.06.2018 11:44, Nikolay Borisov wrote:
>
>
> On 23.06.2018 09:38, Chengguang Xu wrote:
>> Remove -ERANGE error check because there is no chance to get into
>> this condition and meanwhile avoid overriding errno to -EIO in
>> btrfs_get_acl().
>
> This is way too terse. The reason why we can't get an ERANGE error is
> because we first call btrfs_getxattr to get the length of the attribute,
> then we do a subsequent call with the size from the first call. Between
> the 2 calls the size shouldn't change.
>
> Furthermore, I see one more bad practice in this code - the first call
> to btrfs_getxattr is not using the value buffer hence it should be
> passing NULL to make this obvious, instead it's passing an empty string
> form the rodata section.
While at it, I guess you could also remove the BUG() in the default case
of the switch statement in btrfs_get_acl. A simple return
ERR_PTR(-EINVAL) should suffice.
>
>>
>> Signed-off-by: Chengguang Xu <cgxu...@gmx.com>
>> ---
>> v2:
>> - Avoid errno overriding instead of print error message in error case.
>> - Change commit log for better understanding.
>>
>> fs/btrfs/acl.c | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
>> index 15e1dfef56a5..b71a875036af 100644
>> --- a/fs/btrfs/acl.c
>> +++ b/fs/btrfs/acl.c
>> @@ -40,13 +40,13 @@ struct posix_acl *btrfs_get_acl(struct inode *inode, int
>> type)
>> return ERR_PTR(-ENOMEM);
>> size = btrfs_getxattr(inode, name, value, size);
>> }
>> - if (size > 0) {
>> + if (size > 0)
>> acl = posix_acl_from_xattr(&init_user_ns, value, size);
>> - } else if (size == -ERANGE || size == -ENODATA || size == 0) {
>> + else if (size == -ENODATA || size == 0)
>> acl = NULL;
>> - } else {
>> - acl = ERR_PTR(-EIO);
>> - }
>> + else
>> + acl = ERR_PTR(size);
>> +
>> kfree(value);
>>
>> return acl;
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
