On Tue, Jun 26, 2018 at 02:08:27PM +0800, Chengguang Xu wrote:
> There is no chance to get into -ERANGE error condition 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. So remove the
> unnecessary -ERANGE error check and meanwhile fix some other
> bad practices as well.
>
> Detail fixes in this patch.
> - return ERR_PTR(-EINVAL) instead of crashing kernel in default
> case of switch.
> - return original error code when failing from btrfs_getxattr().
> - remove unnecessary bracket.
> - remove unnecessary -ERANGE check.
Hm, that should be really 3 patches, but Nikolay is to blame as he
should have mentioned that when asking for the other things to fix.
> Signed-off-by: Chengguang Xu <cgxu519@xxxxxxx>
> ---
> v3:
> - Fix some other bad practices.
> - Add more information to commit log.
>
> v2:
> - Avoid errno overriding instead of print error message in error case.
> - Change commit log for better understanding.
>
> fs/btrfs/acl.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
> index 15e1dfef56a5..3b66c957ea6f 100644
> --- a/fs/btrfs/acl.c
> +++ b/fs/btrfs/acl.c
> @@ -30,23 +30,22 @@ struct posix_acl *btrfs_get_acl(struct inode *inode, int type)
> name = XATTR_NAME_POSIX_ACL_DEFAULT;
> break;
> default:
> - BUG();
> + return ERR_PTR(-EINVAL);
First patch
> }
>
> - size = btrfs_getxattr(inode, name, "", 0);
> + size = btrfs_getxattr(inode, name, NULL, 0);
Second patch
> if (size > 0) {
> value = kzalloc(size, GFP_KERNEL);
> if (!value)
> 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);
And third patch.
>From current changelog only the 3rd is covered, otherwise 1st and 2nd
are under the "and other bad things", but this deserves a more specific
explanation.
So please split and resend. Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
