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 <[email protected]> > --- > 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
