Re: [PATCH 4/5] xfs: make attr removal an explicit operation

Christoph Hellwig
2024-04-16T05:12:12Z

> + if (op != XFS_ATTRUPDATE_REMOVE || xfs_inode_hasattr(dp)) {
> error = xfs_iext_count_may_overflow(dp, XFS_ATTR_FORK,
> XFS_IEXT_ATTR_MANIP_CNT(rmt_blks));
> if (error == -EFBIG)

No need to change this in the current patch, but checking for a remove
on an inodes without attrs just to skip the extent count upgrade
and not fail the whole operation is a little silly :)

> @@ -203,7 +203,8 @@ __xfs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
> xfs_acl_to_disk(args.value, acl);
> }
>
> - error = xfs_attr_change(&args, XFS_ATTRUPDATE_UPSERT);
> + error = xfs_attr_change(&args, args.value ? XFS_ATTRUPDATE_UPSERT :
> + XFS_ATTRUPDATE_REMOVE);

Given that we have a conditional for removing vs setting right above
i'd clean this up a bit:

xfs_acl_to_disk(args.value, acl);
error = xfs_attr_change(&args, XFS_ATTRUPDATE_UPSERT)
kvfree(args.value);
} else {
error = xfs_attr_change(&args, XFS_ATTRUPDATE_REMOVE)
/*
* If the attribute didn't exist to start with that's fine.
*/
if (error == -ENOATTR)
error = 0;
}