On Sun, Feb 23, 2020 at 4:07 AM Allison Collins
<allison.henderson@xxxxxxxxxx> wrote:
>
> This patch replaces the attribute name and length parameters with a single struct
> xfs_name parameter. This helps to clean up the numbers of parameters being passed
> around and pre-simplifies the code some.
>
> Signed-off-by: Allison Collins <allison.henderson@xxxxxxxxxx>
> ---
I realize I am joining very late with lots of reviewed-by already applied,
so unless I find anything big, please regard my comments and possible
future improvements for the extended series rather than objections to this
pretty much baked patch set.
[...]
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index d42de92..28c07c9 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -357,7 +357,9 @@ xfs_attrmulti_attr_get(
> {
> unsigned char *kbuf;
> int error = -EFAULT;
> - size_t namelen;
> + struct xfs_name xname;
> +
> + xfs_name_init(&xname, name);
>
> if (*len > XFS_XATTR_SIZE_MAX)
> return -EINVAL;
> @@ -365,9 +367,7 @@ xfs_attrmulti_attr_get(
> if (!kbuf)
> return -ENOMEM;
>
> - namelen = strlen(name);
> - error = xfs_attr_get(XFS_I(inode), name, namelen, &kbuf, (int *)len,
> - flags);
> + error = xfs_attr_get(XFS_I(inode), &xname, &kbuf, (int *)len, flags);
> if (error)
> goto out_kfree;
>
> @@ -389,7 +389,9 @@ xfs_attrmulti_attr_set(
> {
> unsigned char *kbuf;
> int error;
> - size_t namelen;
> + struct xfs_name xname;
> +
> + xfs_name_init(&xname, name);
>
> if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
> return -EPERM;
> @@ -400,8 +402,7 @@ xfs_attrmulti_attr_set(
> if (IS_ERR(kbuf))
> return PTR_ERR(kbuf);
>
> - namelen = strlen(name);
> - error = xfs_attr_set(XFS_I(inode), name, namelen, kbuf, len, flags);
> + error = xfs_attr_set(XFS_I(inode), &xname, kbuf, len, flags);
> if (!error)
> xfs_forget_acl(inode, name, flags);
> kfree(kbuf);
> @@ -415,12 +416,14 @@ xfs_attrmulti_attr_remove(
> uint32_t flags)
> {
> int error;
> - size_t namelen;
> + struct xfs_name xname;
> +
> + xfs_name_init(&xname, name);
>
> if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
> return -EPERM;
> - namelen = strlen(name);
> - error = xfs_attr_remove(XFS_I(inode), name, namelen, flags);
> +
> + error = xfs_attr_remove(XFS_I(inode), &xname, flags);
> if (!error)
> xfs_forget_acl(inode, name, flags);
> return error;
A struct inititializer macro would have been nice, so code like this:
+ struct xfs_name xname;
+
+ xfs_name_init(&xname, name);
Would become:
+ struct xfs_name xname = XFS_NAME_STR_INIT(name);
As a matter of fact, in most of the cases a named local variable is
not needed at
all and the code could be written with an anonymous local struct variable macro:
+ error = xfs_attr_remove(XFS_I(inode), XFS_NAME_STR(name), flags);
Thanks,
Amir.
