Re: [PATCH v7 02/19] xfs: Embed struct xfs_name in xfs_da_args

Allison Collins
2020-02-25T02:02:41Z

On 2/24/20 5:57 PM, Dave Chinner wrote:
> On Sat, Feb 22, 2020 at 07:05:54PM -0700, Allison Collins wrote:
>> This patch embeds an xfs_name in xfs_da_args, replacing the name, namelen, and flags
>> members. This helps to clean up the xfs_da_args structure and make it more uniform
>> with the new xfs_name parameter being passed around.
>
> Commit message should wrap at 68-72 columns.
>
>>
>> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
>> Reviewed-by: Brian Foster <bfoster@redhat.com>
>> ---
>> fs/xfs/libxfs/xfs_attr.c | 37 +++++++-------
>> fs/xfs/libxfs/xfs_attr_leaf.c | 104 +++++++++++++++++++++-------------------
>> fs/xfs/libxfs/xfs_attr_remote.c | 2 +-
>> fs/xfs/libxfs/xfs_da_btree.c | 6 ++-
>> fs/xfs/libxfs/xfs_da_btree.h | 4 +-
>> fs/xfs/libxfs/xfs_dir2.c | 18 +++----
>> fs/xfs/libxfs/xfs_dir2_block.c | 6 +--
>> fs/xfs/libxfs/xfs_dir2_leaf.c | 6 +--
>> fs/xfs/libxfs/xfs_dir2_node.c | 8 ++--
>> fs/xfs/libxfs/xfs_dir2_sf.c | 30 ++++++------
>> fs/xfs/scrub/attr.c | 12 ++---
>> fs/xfs/xfs_trace.h | 20 ++++----
>> 12 files changed, 130 insertions(+), 123 deletions(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
>> index 6717f47..9acdb23 100644
>> --- a/fs/xfs/libxfs/xfs_attr.c
>> +++ b/fs/xfs/libxfs/xfs_attr.c
>> @@ -72,13 +72,12 @@ xfs_attr_args_init(
>> args->geo = dp->i_mount->m_attr_geo;
>> args->whichfork = XFS_ATTR_FORK;
>> args->dp = dp;
>> - args->flags = flags;
>> - args->name = name->name;
>> - args->namelen = name->len;
>> - if (args->namelen >= MAXNAMELEN)
>> + memcpy(&args->name, name, sizeof(struct xfs_name));
>> + args->name.type = flags;
>
> This doesn't play well with Christoph's cleanup series which fixes
> up all the confusion with internal versus API flags. I guess the
> namespace is part of the attribute name, but I think this would be a
> much clearer conversion when placed on top of the way Christoph
> cleaned all this up...
>
> Have you looked at rebasing this on top of that cleanup series?
>
> Cheers,
>
Yes, there is some conflict between the sets here and there, but I think
folks wanted to keep them separate for now. Are you referring to
"[780d29057781] xfs: fix misuse of the XFS_ATTR_INCOMPLETE flag"? I'm
pretty sure this set is already seated on top of that one. This one is
based on the latest for-next.

Allison