diff mbox series

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

Message ID 20220611094200.129502-16-allison.henderson@oracle.com (mailing list archive)
State Superseded
Headers show
Series Return of the Parent Pointers | expand

Commit Message

Allison Henderson June 11, 2022, 9:41 a.m. UTC 
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.

Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
 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(-)

Comments

Darrick J. Wong June 29, 2022, 6:42 p.m. UTC | #1 
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
>
Allison Henderson June 30, 2022, 1:30 a.m. UTC | #2 
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
> >
diff mbox series

Patch

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(