diff mbox

[v2,3/3] orangefs: Remove useless xattr prefix arguments

Message ID CAOg9mSQU4q4hCB2WE6TjKPyoCkYPjTm+4kO=W5L2A7TKAxFGDw@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mike Marshall June 3, 2016, 7:44 p.m. UTC 
Hi Andreas...

I'm sorry I didn't notice this right away. And that I missed the merge window
with your patch <g>... anyhow...

We use the return value in this one line you changed, our userspace code gets
ill when we send it (-ENOMEM +1) as a key length... would you change the
patch one more time, or ack my change please... I'll get these in at the next
merge window...

# git diff

On Mon, May 30, 2016 at 5:26 AM, Andreas Gruenbacher
<agruenba@redhat.com> wrote:
> Passing the attribute prefix and name to orangefs_inode_*xattr as
> separate arguments was unnecessary before, but now that the trusted
> xattr handler is gone, the prefix is even always the empty string.
> Remove it.
>
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
>  fs/orangefs/acl.c             |  9 ++---
>  fs/orangefs/file.c            |  2 --
>  fs/orangefs/orangefs-kernel.h |  2 --
>  fs/orangefs/xattr.c           | 81 ++++++++++++++-----------------------------
>  4 files changed, 29 insertions(+), 65 deletions(-)
>
> diff --git a/fs/orangefs/acl.c b/fs/orangefs/acl.c
> index df24864..28f2195 100644
> --- a/fs/orangefs/acl.c
> +++ b/fs/orangefs/acl.c
> @@ -43,11 +43,8 @@ struct posix_acl *orangefs_get_acl(struct inode *inode, int type)
>                      get_khandle_from_ino(inode),
>                      key,
>                      type);
> -       ret = orangefs_inode_getxattr(inode,
> -                                  "",
> -                                  key,
> -                                  value,
> -                                  ORANGEFS_MAX_XATTR_VALUELEN);
> +       ret = orangefs_inode_getxattr(inode, key, value,
> +                                     ORANGEFS_MAX_XATTR_VALUELEN);
>         /* if the key exists, convert it to an in-memory rep */
>         if (ret > 0) {
>                 acl = posix_acl_from_xattr(&init_user_ns, value, ret);
> @@ -131,7 +128,7 @@ int orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
>          * will xlate to a removexattr. However, we don't want removexattr
>          * complain if attributes does not exist.
>          */
> -       error = orangefs_inode_setxattr(inode, "", name, value, size, 0);
> +       error = orangefs_inode_setxattr(inode, name, value, size, 0);
>
>  out:
>         kfree(value);
> diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c
> index 5160a3f..526040e 100644
> --- a/fs/orangefs/file.c
> +++ b/fs/orangefs/file.c
> @@ -516,7 +516,6 @@ static long orangefs_ioctl(struct file *file, unsigned int cmd, unsigned long ar
>         if (cmd == FS_IOC_GETFLAGS) {
>                 val = 0;
>                 ret = orangefs_inode_getxattr(file_inode(file),
> -                                             "",
>                                               "user.pvfs2.meta_hint",
>                                               &val, sizeof(val));
>                 if (ret < 0 && ret != -ENODATA)
> @@ -549,7 +548,6 @@ static long orangefs_ioctl(struct file *file, unsigned int cmd, unsigned long ar
>                              "orangefs_ioctl: FS_IOC_SETFLAGS: %llu\n",
>                              (unsigned long long)val);
>                 ret = orangefs_inode_setxattr(file_inode(file),
> -                                             "",
>                                               "user.pvfs2.meta_hint",
>                                               &val, sizeof(val), 0);
>         }
> diff --git a/fs/orangefs/orangefs-kernel.h b/fs/orangefs/orangefs-kernel.h
> index 6503e37..7b542f1 100644
> --- a/fs/orangefs/orangefs-kernel.h
> +++ b/fs/orangefs/orangefs-kernel.h
> @@ -517,13 +517,11 @@ __s32 fsid_of_op(struct orangefs_kernel_op_s *op);
>  int orangefs_flush_inode(struct inode *inode);
>
>  ssize_t orangefs_inode_getxattr(struct inode *inode,
> -                            const char *prefix,
>                              const char *name,
>                              void *buffer,
>                              size_t size);
>
>  int orangefs_inode_setxattr(struct inode *inode,
> -                        const char *prefix,
>                          const char *name,
>                          const void *value,
>                          size_t size,
> diff --git a/fs/orangefs/xattr.c b/fs/orangefs/xattr.c
> index 640a98f..1165047 100644
> --- a/fs/orangefs/xattr.c
> +++ b/fs/orangefs/xattr.c
> @@ -59,8 +59,8 @@ static inline int convert_to_internal_xattr_flags(int setxattr_flags)
>   * unless the key does not exist for the file and/or if
>   * there were errors in fetching the attribute value.
>   */
> -ssize_t orangefs_inode_getxattr(struct inode *inode, const char *prefix,
> -               const char *name, void *buffer, size_t size)
> +ssize_t orangefs_inode_getxattr(struct inode *inode, const char *name,
> +                               void *buffer, size_t size)
>  {
>         struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
>         struct orangefs_kernel_op_s *new_op = NULL;
> @@ -70,12 +70,12 @@ ssize_t orangefs_inode_getxattr(struct inode *inode, const char *prefix,
>         int fsgid;
>
>         gossip_debug(GOSSIP_XATTR_DEBUG,
> -                    "%s: prefix %s name %s, buffer_size %zd\n",
> -                    __func__, prefix, name, size);
> +                    "%s: name %s, buffer_size %zd\n",
> +                    __func__, name, size);
>
> -       if ((strlen(name) + strlen(prefix)) >= ORANGEFS_MAX_XATTR_NAMELEN) {
> +       if (strlen(name) >= ORANGEFS_MAX_XATTR_NAMELEN) {
>                 gossip_err("Invalid key length (%d)\n",
> -                          (int)(strlen(name) + strlen(prefix)));
> +                          (int)strlen(name));
>                 return -EINVAL;
>         }
>
> @@ -97,8 +97,7 @@ ssize_t orangefs_inode_getxattr(struct inode *inode, const char *prefix,
>                 goto out_unlock;
>
>         new_op->upcall.req.getxattr.refn = orangefs_inode->refn;
> -       ret = snprintf((char *)new_op->upcall.req.getxattr.key,
> -                      ORANGEFS_MAX_XATTR_NAMELEN, "%s%s", prefix, name);
> +       strcpy(new_op->upcall.req.getxattr.key, name);
>
>         /*
>          * NOTE: Although keys are meant to be NULL terminated textual
> @@ -163,10 +162,8 @@ out_unlock:
>         return ret;
>  }
>
> -static int orangefs_inode_removexattr(struct inode *inode,
> -                           const char *prefix,
> -                           const char *name,
> -                           int flags)
> +static int orangefs_inode_removexattr(struct inode *inode, const char *name,
> +                                     int flags)
>  {
>         struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
>         struct orangefs_kernel_op_s *new_op = NULL;
> @@ -183,12 +180,8 @@ static int orangefs_inode_removexattr(struct inode *inode,
>          * textual strings, I am going to explicitly pass the
>          * length just in case we change this later on...
>          */
> -       ret = snprintf((char *)new_op->upcall.req.removexattr.key,
> -                      ORANGEFS_MAX_XATTR_NAMELEN,
> -                      "%s%s",
> -                      (prefix ? prefix : ""),
> -                      name);
> -       new_op->upcall.req.removexattr.key_sz = ret + 1;
> +       strcpy(new_op->upcall.req.removexattr.key, name);
> +       new_op->upcall.req.removexattr.key_sz = strlen(name) + 1;
>
>         gossip_debug(GOSSIP_XATTR_DEBUG,
>                      "orangefs_inode_removexattr: key %s, key_sz %d\n",
> @@ -223,8 +216,8 @@ out_unlock:
>   * Returns a -ve number on error and 0 on success.  Key is text, but value
>   * can be binary!
>   */
> -int orangefs_inode_setxattr(struct inode *inode, const char *prefix,
> -               const char *name, const void *value, size_t size, int flags)
> +int orangefs_inode_setxattr(struct inode *inode, const char *name,
> +                           const void *value, size_t size, int flags)
>  {
>         struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
>         struct orangefs_kernel_op_s *new_op;
> @@ -232,8 +225,8 @@ int orangefs_inode_setxattr(struct inode *inode, const char *prefix,
>         int ret = -ENOMEM;
>
>         gossip_debug(GOSSIP_XATTR_DEBUG,
> -                    "%s: prefix %s, name %s, buffer_size %zd\n",
> -                    __func__, prefix, name, size);
> +                    "%s: name %s, buffer_size %zd\n",
> +                    __func__, name, size);
>
>         if (size >= ORANGEFS_MAX_XATTR_VALUELEN ||
>             flags < 0) {
> @@ -245,29 +238,19 @@ int orangefs_inode_setxattr(struct inode *inode, const char *prefix,
>
>         internal_flag = convert_to_internal_xattr_flags(flags);
>
> -       if (prefix) {
> -               if (strlen(name) + strlen(prefix) >= ORANGEFS_MAX_XATTR_NAMELEN) {
> -                       gossip_err
> -                           ("orangefs_inode_setxattr: bogus key size (%d)\n",
> -                            (int)(strlen(name) + strlen(prefix)));
> -                       return -EINVAL;
> -               }
> -       } else {
> -               if (strlen(name) >= ORANGEFS_MAX_XATTR_NAMELEN) {
> -                       gossip_err
> -                           ("orangefs_inode_setxattr: bogus key size (%d)\n",
> -                            (int)(strlen(name)));
> -                       return -EINVAL;
> -               }
> +       if (strlen(name) >= ORANGEFS_MAX_XATTR_NAMELEN) {
> +               gossip_err
> +                   ("orangefs_inode_setxattr: bogus key size (%d)\n",
> +                    (int)(strlen(name)));
> +               return -EINVAL;
>         }
>
>         /* This is equivalent to a removexattr */
>         if (size == 0 && value == NULL) {
>                 gossip_debug(GOSSIP_XATTR_DEBUG,
> -                            "removing xattr (%s%s)\n",
> -                            prefix,
> +                            "removing xattr (%s)\n",
>                              name);
> -               return orangefs_inode_removexattr(inode, prefix, name, flags);
> +               return orangefs_inode_removexattr(inode, name, flags);
>         }
>
>         gossip_debug(GOSSIP_XATTR_DEBUG,
> @@ -288,11 +271,8 @@ int orangefs_inode_setxattr(struct inode *inode, const char *prefix,
>          * strings, I am going to explicitly pass the length just in
>          * case we change this later on...
>          */
> -       ret = snprintf((char *)new_op->upcall.req.setxattr.keyval.key,
> -                      ORANGEFS_MAX_XATTR_NAMELEN,
> -                      "%s%s",
> -                      prefix, name);
> -       new_op->upcall.req.setxattr.keyval.key_sz = ret + 1;
> +       strcpy(new_op->upcall.req.setxattr.keyval.key, name);
> +       new_op->upcall.req.setxattr.keyval.key_sz = strlen(name) + 1;
>         memcpy(new_op->upcall.req.setxattr.keyval.val, value, size);
>         new_op->upcall.req.setxattr.keyval.val_sz = size;
>
> @@ -455,12 +435,7 @@ static int orangefs_xattr_set_default(const struct xattr_handler *handler,
>                                       size_t size,
>                                       int flags)
>  {
> -       return orangefs_inode_setxattr(inode,
> -                                   "",
> -                                   name,
> -                                   buffer,
> -                                   size,
> -                                   flags);
> +       return orangefs_inode_setxattr(inode, name, buffer, size, flags);
>  }
>
>  static int orangefs_xattr_get_default(const struct xattr_handler *handler,
> @@ -470,11 +445,7 @@ static int orangefs_xattr_get_default(const struct xattr_handler *handler,
>                                       void *buffer,
>                                       size_t size)
>  {
> -       return orangefs_inode_getxattr(inode,
> -                                   "",
> -                                   name,
> -                                   buffer,
> -                                   size);
> +       return orangefs_inode_getxattr(inode, name, buffer, size);
>
>  }
>
> --
> 2.5.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/orangefs/xattr.c b/fs/orangefs/xattr.c
index 1165047..bdb407e 100644
--- a/fs/orangefs/xattr.c
+++ b/fs/orangefs/xattr.c
@@ -97,7 +97,11 @@  ssize_t orangefs_inode_getxattr(struct inode *inode, const ch
                goto out_unlock;

        new_op->upcall.req.getxattr.refn = orangefs_inode->refn;
-       strcpy(new_op->upcall.req.getxattr.key, name);
+
+       ret = snprintf((char *)new_op->upcall.req.getxattr.key,
+                      ORANGEFS_MAX_XATTR_NAMELEN,
+                      "%s",
+                      name);

        /*
         * NOTE: Although keys are meant to be NULL terminated textual