On 2/23/20 2:34 AM, Amir Goldstein wrote:
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.
Sure, no worries, it has a lot of history to keep track of.
[...]
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);
The macro does look nice. I would be the third iteration of
initializers that this patch has been through though. Can I get a
consensus of how many people like the macro?
Allison
Thanks,
Amir.
