Re: [PATCH 04/30] xfs: merge xfs_attrmulti_attr_remove into xfs_attrmulti_attr_set

From: Chandan Rajendra <chandan@linux.ibm.com>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-xfs@vger.kernel.org,
	Allison Collins <allison.henderson@oracle.com>,
	"Darrick J . Wong" <darrick.wong@oracle.com>
Subject: Re: [PATCH 04/30] xfs: merge xfs_attrmulti_attr_remove into xfs_attrmulti_attr_set
Date: Thu, 06 Feb 2020 14:51:52 +0530	[thread overview]
Message-ID: <2023754.vEBZo8tne2@localhost.localdomain> (raw)
In-Reply-To: <20200129170310.51370-5-hch@lst.de>

On Wednesday, January 29, 2020 10:32 PM Christoph Hellwig wrote: 
> Merge the ioctl handlers just like the low-level xfs_attr_set function.
>

The newly introduced changes match with the code flow that earlier existed
separately in xfs_attrmulti_attr_set() and xfs_attrmulti_attr_remove().

Reviewed-by: Chandan Rajendra <chandanrlinux@gmail.com>

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/xfs_ioctl.c   | 34 ++++++++++------------------------
>  fs/xfs/xfs_ioctl.h   |  6 ------
>  fs/xfs/xfs_ioctl32.c |  4 ++--
>  3 files changed, 12 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 79c418888e9a..b806003caacd 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -389,18 +389,20 @@ xfs_attrmulti_attr_set(
>  	uint32_t		len,
>  	uint32_t		flags)
>  {
> -	unsigned char		*kbuf;
> +	unsigned char		*kbuf = NULL;
>  	int			error;
>  	size_t			namelen;
> 
>  	if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
>  		return -EPERM;
> -	if (len > XFS_XATTR_SIZE_MAX)
> -		return -EINVAL;
> 
> -	kbuf = memdup_user(ubuf, len);
> -	if (IS_ERR(kbuf))
> -		return PTR_ERR(kbuf);
> +	if (ubuf) {
> +		if (len > XFS_XATTR_SIZE_MAX)
> +			return -EINVAL;
> +		kbuf = memdup_user(ubuf, len);
> +		if (IS_ERR(kbuf))
> +			return PTR_ERR(kbuf);
> +	}
> 
>  	namelen = strlen(name);
>  	error = xfs_attr_set(XFS_I(inode), name, namelen, kbuf, len, flags);
> @@ -410,22 +412,6 @@ xfs_attrmulti_attr_set(
>  	return error;
>  }
> 
> -int
> -xfs_attrmulti_attr_remove(
> -	struct inode		*inode,
> -	unsigned char		*name,
> -	uint32_t		flags)
> -{
> -	int			error;
> -
> -	if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
> -		return -EPERM;
> -	error = xfs_attr_set(XFS_I(inode), name, strlen(name), NULL, 0, flags);
> -	if (!error)
> -		xfs_forget_acl(inode, name, flags);
> -	return error;
> -}
> -
>  STATIC int
>  xfs_attrmulti_by_handle(
>  	struct file		*parfilp,
> @@ -504,8 +490,8 @@ xfs_attrmulti_by_handle(
>  			ops[i].am_error = mnt_want_write_file(parfilp);
>  			if (ops[i].am_error)
>  				break;
> -			ops[i].am_error = xfs_attrmulti_attr_remove(
> -					d_inode(dentry), attr_name,
> +			ops[i].am_error = xfs_attrmulti_attr_set(
> +					d_inode(dentry), attr_name, NULL, 0,
>  					ops[i].am_flags);
>  			mnt_drop_write_file(parfilp);
>  			break;
> diff --git a/fs/xfs/xfs_ioctl.h b/fs/xfs/xfs_ioctl.h
> index 420bd95dc326..819504df00ae 100644
> --- a/fs/xfs/xfs_ioctl.h
> +++ b/fs/xfs/xfs_ioctl.h
> @@ -46,12 +46,6 @@ xfs_attrmulti_attr_set(
>  	uint32_t		len,
>  	uint32_t		flags);
> 
> -extern int
> -xfs_attrmulti_attr_remove(
> -	struct inode		*inode,
> -	unsigned char		*name,
> -	uint32_t		flags);
> -
>  extern struct dentry *
>  xfs_handle_to_dentry(
>  	struct file		*parfilp,
> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> index 9705172e5410..e085f304e539 100644
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -488,8 +488,8 @@ xfs_compat_attrmulti_by_handle(
>  			ops[i].am_error = mnt_want_write_file(parfilp);
>  			if (ops[i].am_error)
>  				break;
> -			ops[i].am_error = xfs_attrmulti_attr_remove(
> -					d_inode(dentry), attr_name,
> +			ops[i].am_error = xfs_attrmulti_attr_set(
> +					d_inode(dentry), attr_name, NULL, 0,
>  					ops[i].am_flags);
>  			mnt_drop_write_file(parfilp);
>  			break;
> 

-- 
chandan