Re: [PATCH v1 15/17] xfs: Add helper function xfs_attr_list_context_init

["Darrick J. Wong" <djwong@kernel.org>]

On Sat, Jun 11, 2022 at 02:41:58AM -0700, Allison Henderson wrote:
> This patch adds a helper function xfs_attr_list_context_init used by
> xfs_attr_list. This function initializes the xfs_attr_list_context
> structure passed to xfs_attr_list_int. We will need this later to call
> xfs_attr_list_int_ilocked when the node is already locked.

Since you've mentioned the xattr userspace functions -- does our current
codebase hide the parent pointer xattrs from regular
getxattr/setxattr/listxattr system calls?

> Signed-off-by: Allison Henderson <allison.henderson@oracle.com>

The change itself looks pretty straightfoward,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_file.c  |  1 +
>  fs/xfs/xfs_ioctl.c | 54 ++++++++++++++++++++++++++++++++--------------
>  fs/xfs/xfs_ioctl.h |  2 ++
>  3 files changed, 41 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index e2f2a3a94634..884827f024fd 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -17,6 +17,7 @@
>  #include "xfs_bmap_util.h"
>  #include "xfs_dir2.h"
>  #include "xfs_dir2_priv.h"
> +#include "xfs_attr.h"
>  #include "xfs_ioctl.h"
>  #include "xfs_trace.h"
>  #include "xfs_log.h"
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 5a364a7d58fd..e1612e99e0c5 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -369,6 +369,40 @@ xfs_attr_flags(
>  	return 0;
>  }
>  
> +/*
> + * Initializes an xfs_attr_list_context suitable for
> + * use by xfs_attr_list
> + */
> +int
> +xfs_ioc_attr_list_context_init(
> +	struct xfs_inode		*dp,
> +	char				*buffer,
> +	int				bufsize,
> +	int				flags,
> +	struct xfs_attr_list_context	*context)
> +{
> +	struct xfs_attrlist		*alist;
> +
> +	/*
> +	 * Initialize the output buffer.
> +	 */
> +	context->dp = dp;
> +	context->resynch = 1;
> +	context->attr_filter = xfs_attr_filter(flags);
> +	context->buffer = buffer;
> +	context->bufsize = round_down(bufsize, sizeof(uint32_t));
> +	context->firstu = context->bufsize;
> +	context->put_listent = xfs_ioc_attr_put_listent;
> +
> +	alist = context->buffer;
> +	alist->al_count = 0;
> +	alist->al_more = 0;
> +	alist->al_offset[0] = context->bufsize;
> +
> +	return 0;
> +}
> +
> +
>  int
>  xfs_ioc_attr_list(
>  	struct xfs_inode		*dp,
> @@ -378,7 +412,6 @@ xfs_ioc_attr_list(
>  	struct xfs_attrlist_cursor __user *ucursor)
>  {
>  	struct xfs_attr_list_context	context = { };
> -	struct xfs_attrlist		*alist;
>  	void				*buffer;
>  	int				error;
>  
> @@ -410,21 +443,10 @@ xfs_ioc_attr_list(
>  	if (!buffer)
>  		return -ENOMEM;
>  
> -	/*
> -	 * Initialize the output buffer.
> -	 */
> -	context.dp = dp;
> -	context.resynch = 1;
> -	context.attr_filter = xfs_attr_filter(flags);
> -	context.buffer = buffer;
> -	context.bufsize = round_down(bufsize, sizeof(uint32_t));
> -	context.firstu = context.bufsize;
> -	context.put_listent = xfs_ioc_attr_put_listent;
> -
> -	alist = context.buffer;
> -	alist->al_count = 0;
> -	alist->al_more = 0;
> -	alist->al_offset[0] = context.bufsize;
> +	error = xfs_ioc_attr_list_context_init(dp, buffer, bufsize, flags,
> +			&context);
> +	if (error)
> +		return error;
>  
>  	error = xfs_attr_list(&context);
>  	if (error)
> diff --git a/fs/xfs/xfs_ioctl.h b/fs/xfs/xfs_ioctl.h
> index d4abba2c13c1..ca60e1c427a3 100644
> --- a/fs/xfs/xfs_ioctl.h
> +++ b/fs/xfs/xfs_ioctl.h
> @@ -35,6 +35,8 @@ int xfs_ioc_attrmulti_one(struct file *parfilp, struct inode *inode,
>  int xfs_ioc_attr_list(struct xfs_inode *dp, void __user *ubuf,
>  		      size_t bufsize, int flags,
>  		      struct xfs_attrlist_cursor __user *ucursor);
> +int xfs_ioc_attr_list_context_init(struct xfs_inode *dp, char *buffer,
> +		int bufsize, int flags, struct xfs_attr_list_context *context);
>  
>  extern struct dentry *
>  xfs_handle_to_dentry(
> -- 
> 2.25.1
>