Re: [PATCH 07/32] xfs: allow xattr matching on name and value for local/sf attrs

Christoph Hellwig
2024-04-10T05:17:00Z

On Tue, Apr 09, 2024 at 05:55:20PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> Add a new XFS_DA_OP_PARENT flag to signal that the caller wants to look

The flag doesn't actually exist, the match is done on the
XFS_ATTR_PARENT namespaces.

>
> @@ -2444,14 +2477,17 @@ xfs_attr3_leaf_lookup_int(
> name_loc = xfs_attr3_leaf_name_local(leaf, probe);
> if (!xfs_attr_match(args, entry->flags,
> name_loc->nameval,
> - name_loc->namelen))
> + name_loc->namelen,
> + &name_loc->nameval[name_loc->namelen],
> + be16_to_cpu(name_loc->valuelen)))

If we'd switch from the odd pre-existing three-tab indent to the normal
two-tab indent we'd avoid the overly long line here.

> continue;
> args->index = probe;
> return -EEXIST;
> } else {
> name_rmt = xfs_attr3_leaf_name_remote(leaf, probe);
> if (!xfs_attr_match(args, entry->flags, name_rmt->name,
> - name_rmt->namelen))
> + name_rmt->namelen, NULL,
> + be32_to_cpu(name_rmt->valuelen)))

... and here.

The remote side might also benefit from a local variable to store the
endian swapped version of the valuelen instead of calculating it twice.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>