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

[Alli <allison.henderson@oracle.com>]

On Wed, 2022-06-29 at 11:42 -0700, Darrick J. Wong wrote:
> 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?
I don't think it's hidden, but it has the XFS_ATTR_PARENT flag set so
that the caller can know to handle it differently than a normal string
attr. 

> 
> > Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
> 
> The change itself looks pretty straightfoward,
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> 
Great, thanks!
Allison

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