Re: [PATCH v7 03/19] xfs: Add xfs_has_attr and subroutines

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2/25/20 3:15 AM, Chandan Rajendra wrote:
On Tuesday, February 25, 2020 3:19 PM Chandan Rajendra wrote:
On Sunday, February 23, 2020 7:35 AM Allison Collins wrote:
From: Allison Henderson <allison.henderson@xxxxxxxxxx>

This patch adds a new functions to check for the existence of an attribute.
Subroutines are also added to handle the cases of leaf blocks, nodes or shortform.
Common code that appears in existing attr add and remove functions have been
factored out to help reduce the appearance of duplicated code.  We will need these
routines later for delayed attributes since delayed operations cannot return error
codes.

Signed-off-by: Allison Collins <allison.henderson@xxxxxxxxxx>
---
  fs/xfs/libxfs/xfs_attr.c      | 171 ++++++++++++++++++++++++++++--------------
  fs/xfs/libxfs/xfs_attr.h      |   1 +
  fs/xfs/libxfs/xfs_attr_leaf.c | 111 +++++++++++++++++----------
  fs/xfs/libxfs/xfs_attr_leaf.h |   3 +
  4 files changed, 188 insertions(+), 98 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 9acdb23..2255060 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -46,6 +46,7 @@ STATIC int xfs_attr_shortform_addname(xfs_da_args_t *args);
  STATIC int xfs_attr_leaf_get(xfs_da_args_t *args);
  STATIC int xfs_attr_leaf_addname(xfs_da_args_t *args);
  STATIC int xfs_attr_leaf_removename(xfs_da_args_t *args);
+STATIC int xfs_attr_leaf_hasname(struct xfs_da_args *args, struct xfs_buf **bp);
/*
   * Internal routines when attribute list is more than one block.
@@ -53,6 +54,8 @@ STATIC int xfs_attr_leaf_removename(xfs_da_args_t *args);
  STATIC int xfs_attr_node_get(xfs_da_args_t *args);
  STATIC int xfs_attr_node_addname(xfs_da_args_t *args);
  STATIC int xfs_attr_node_removename(xfs_da_args_t *args);
+STATIC int xfs_attr_node_hasname(xfs_da_args_t *args,
+				 struct xfs_da_state **state);
  STATIC int xfs_attr_fillstate(xfs_da_state_t *state);
  STATIC int xfs_attr_refillstate(xfs_da_state_t *state);
@@ -310,6 +313,37 @@ xfs_attr_set_args(
  }
/*
+ * Return EEXIST if attr is found, or ENOATTR if not
+ */
+int
+xfs_has_attr(
+	struct xfs_da_args      *args)
+{
+	struct xfs_inode	*dp = args->dp;
+	struct xfs_buf		*bp = NULL;
+	int			error;
+
+	if (!xfs_inode_hasattr(dp))
+		return -ENOATTR;
+
+	if (dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) {
+		ASSERT(dp->i_afp->if_flags & XFS_IFINLINE);
+		return xfs_attr_sf_findname(args, NULL, NULL);
+	}
+
+	if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) {
+		error = xfs_attr_leaf_hasname(args, &bp);
+
+		if (bp)
+			xfs_trans_brelse(args->trans, bp);
+
+		return error;
+	}
+
+	return xfs_attr_node_hasname(args, NULL);
+}
+
+/*
   * Remove the attribute specified in @args.
   */
  int
@@ -583,26 +617,20 @@ STATIC int
  xfs_attr_leaf_addname(
  	struct xfs_da_args	*args)
  {
-	struct xfs_inode	*dp;
  	struct xfs_buf		*bp;
  	int			retval, error, forkoff;
+	struct xfs_inode	*dp = args->dp;
trace_xfs_attr_leaf_addname(args); /*
-	 * Read the (only) block in the attribute list in.
-	 */
-	dp = args->dp;
-	args->blkno = 0;
-	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, &bp);
-	if (error)
-		return error;
-
-	/*
  	 * Look up the given attribute in the leaf block.  Figure out if
  	 * the given flags produce an error or call for an atomic rename.
  	 */
-	retval = xfs_attr3_leaf_lookup_int(bp, args);
+	retval = xfs_attr_leaf_hasname(args, &bp);
+	if (retval != -ENOATTR && retval != -EEXIST)
+		return retval;
+
  	if ((args->name.type & ATTR_REPLACE) && (retval == -ENOATTR)) {
  		xfs_trans_brelse(args->trans, bp);
  		return retval;
@@ -754,6 +782,27 @@ xfs_attr_leaf_addname(
  }
/*
+ * Return EEXIST if attr is found, or ENOATTR if not
+ */
+STATIC int
+xfs_attr_leaf_hasname(
+	struct xfs_da_args      *args,
+	struct xfs_buf		**bp)
+{
+	int                     error = 0;
+
+	error = xfs_attr3_leaf_read(args->trans, args->dp, 0, bp);
+	if (error)
+		return error;
+
+	error = xfs_attr3_leaf_lookup_int(*bp, args);
+	if (error != -ENOATTR && error != -EEXIST)
+		xfs_trans_brelse(args->trans, *bp);
+
+	return error;
+}
+
+/*
   * Remove a name from the leaf attribute list structure
   *
   * This leaf block cannot have a "remote" value, we only call this routine
@@ -773,12 +822,11 @@ xfs_attr_leaf_removename(
  	 * Remove the attribute.
  	 */
  	dp = args->dp;
-	args->blkno = 0;
-	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, &bp);
-	if (error)
+
+	error = xfs_attr_leaf_hasname(args, &bp);
+	if (error != -ENOATTR && error != -EEXIST)
  		return error;
- error = xfs_attr3_leaf_lookup_int(bp, args);
  	if (error == -ENOATTR) {
  		xfs_trans_brelse(args->trans, bp);
  		return error;
@@ -817,12 +865,10 @@ xfs_attr_leaf_get(xfs_da_args_t *args)
trace_xfs_attr_leaf_get(args); - args->blkno = 0;
-	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, &bp);
-	if (error)
+	error = xfs_attr_leaf_hasname(args, &bp);
+	if (error != -ENOATTR && error != -EEXIST)
  		return error;
- error = xfs_attr3_leaf_lookup_int(bp, args);
  	if (error != -EEXIST)  {
  		xfs_trans_brelse(args->trans, bp);
  		return error;
@@ -832,6 +878,41 @@ xfs_attr_leaf_get(xfs_da_args_t *args)
  	return error;
  }
+/*
+ * Return EEXIST if attr is found, or ENOATTR if not
+ * statep: If not null is set to point at the found state.  Caller will
+ *         be responsible for freeing the state in this case.
+ */
+STATIC int
+xfs_attr_node_hasname(
+	struct xfs_da_args	*args,
+	struct xfs_da_state	**statep)
+{
+	struct xfs_da_state	*state;
+	int			retval, error;
+
+	state = xfs_da_state_alloc();
+	state->args = args;
+	state->mp = args->dp->i_mount;
+
+	if (statep != NULL)
+		*statep = NULL;
+
+	/*
+	 * Search to see if name exists, and get back a pointer to it.
+	 */
+	error = xfs_da3_node_lookup_int(state, &retval);
+	if (error == 0) {
+		if (statep != NULL)
+			*statep = state;
+		return retval;
+	}

If 'statep' were NULL, then this would leak the memory pointed to by 'state'
right?

Apart from the above, the remaining changes look good to me.

Reviewed-by: Chandan Rajendra <chandanrlinux@xxxxxxxxx>

Alrighty, will fix.  Thanks!

Allison



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux