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.
>
> 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
