On Tue, Feb 25, 2020 at 03:09:53PM -0800, Christoph Hellwig wrote:
> Instead of converting from one style of arguments to another in
> xfs_attr_set, pass the structure from higher up in the call chain.
>
> 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 | 80 ++++++++++++----------------------------
> fs/xfs/libxfs/xfs_attr.h | 4 +-
> fs/xfs/xfs_acl.c | 35 ++++++++----------
> fs/xfs/xfs_ioctl.c | 25 ++++++++-----
> fs/xfs/xfs_xattr.c | 24 ++++++------
> 5 files changed, 68 insertions(+), 100 deletions(-)
Overall looks good.
Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index fed4f359ef20..13de4bcb8307 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -359,27 +359,32 @@ xfs_attrmulti_attr_get(
> uint32_t *len,
> uint32_t flags)
> {
> - unsigned char *kbuf;
> - int error = -EFAULT;
> - size_t namelen;
> + struct xfs_da_args args = {
> + .dp = XFS_I(inode),
> + .flags = flags,
> + .name = name,
> + .namelen = strlen(name),
> + .valuelen = *len,
> + };
> + int error;
>
> if (*len > XFS_XATTR_SIZE_MAX)
> return -EINVAL;
> - kbuf = kmem_zalloc_large(*len, 0);
> - if (!kbuf)
> +
> + args.value = kmem_zalloc_large(*len, 0);
> + if (!args.value)
> return -ENOMEM;
>
> - namelen = strlen(name);
> - error = xfs_attr_get(XFS_I(inode), name, namelen, &kbuf, (int *)len,
> - flags);
> + error = xfs_attr_get(&args);
> if (error)
> goto out_kfree;
>
> - if (copy_to_user(ubuf, kbuf, *len))
> + *len = args.valuelen;
> + if (copy_to_user(ubuf, args.value, args.valuelen))
> error = -EFAULT;
>
> out_kfree:
> - kmem_free(kbuf);
> + kmem_free(args.value);
> return error;
This would seem like a prime candidate for ATTR_ALLOC conversion?
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
