On Mon, Feb 17, 2020 at 01:59:39PM +0100, Christoph Hellwig wrote:
> The inode can easily be derived from the args structure. Also
> don't bother with else statements after early returns.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> Reviewed-by: Chandan Rajendra <chandanrlinux@xxxxxxxxx>
> Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_attr.c | 15 +++++++--------
> fs/xfs/libxfs/xfs_attr.h | 2 +-
> fs/xfs/scrub/attr.c | 2 +-
> 3 files changed, 9 insertions(+), 10 deletions(-)
Looks ok, but....
>
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 288b39e81efd..fd095e3d4a9a 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -77,19 +77,18 @@ xfs_inode_hasattr(
> */
> int
> xfs_attr_get_ilocked(
> - struct xfs_inode *ip,
> struct xfs_da_args *args)
> {
> - ASSERT(xfs_isilocked(ip, XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
> + ASSERT(xfs_isilocked(args->dp, XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
>
> - if (!xfs_inode_hasattr(ip))
> + if (!xfs_inode_hasattr(args->dp))
> return -ENOATTR;
> - else if (ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL)
> +
> + if (args->dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL)
> return xfs_attr_shortform_getvalue(args);
> - else if (xfs_bmap_one_block(ip, XFS_ATTR_FORK))
> + if (xfs_bmap_one_block(args->dp, XFS_ATTR_FORK))
> return xfs_attr_leaf_get(args);
> - else
> - return xfs_attr_node_get(args);
> + return xfs_attr_node_get(args);
> }
>
> /*
> @@ -133,7 +132,7 @@ xfs_attr_get(
> args->op_flags |= XFS_DA_OP_ALLOCVAL;
>
> lock_mode = xfs_ilock_attr_map_shared(args->dp);
> - error = xfs_attr_get_ilocked(args->dp, args);
> + error = xfs_attr_get_ilocked(args);
> xfs_iunlock(args->dp, lock_mode);
... at this point I really would like to see the "args->dp" pointer
get renamed. "dp" was originally short for "directory inode
pointer", but it's clear that it hasn't meant that for a long time.
It's just an inode pointer.
That's out of scope for this patch set, though, so maybe the next
cleanup patchset?
Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
--
Dave Chinner
david@xxxxxxxxxxxxx
