* [PATCH v2 0/3] Minor orangefs xattr cleanups
@ 2016-05-30 9:25 Andreas Gruenbacher
2016-05-30 9:25 ` [PATCH v2 1/3] orangefs: Remove useless defines Andreas Gruenbacher
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Andreas Gruenbacher @ 2016-05-30 9:25 UTC (permalink / raw)
To: Mike Marshall; +Cc: Andreas Gruenbacher, linux-fsdevel
Rebased onto current mainline.
Andreas
Andreas Gruenbacher (3):
orangefs: Remove useless defines
orangefs: Remove redundant "trusted." xattr handler
orangefs: Remove useless xattr prefix arguments
fs/orangefs/acl.c | 17 +++---
fs/orangefs/file.c | 2 -
fs/orangefs/orangefs-kernel.h | 13 -----
fs/orangefs/xattr.c | 125 +++++++++---------------------------------
4 files changed, 34 insertions(+), 123 deletions(-)
--
2.5.5
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 1/3] orangefs: Remove useless defines 2016-05-30 9:25 [PATCH v2 0/3] Minor orangefs xattr cleanups Andreas Gruenbacher @ 2016-05-30 9:25 ` Andreas Gruenbacher 2016-05-30 9:26 ` [PATCH v2 2/3] orangefs: Remove redundant "trusted." xattr handler Andreas Gruenbacher 2016-05-30 9:26 ` [PATCH v2 3/3] orangefs: Remove useless xattr prefix arguments Andreas Gruenbacher 2 siblings, 0 replies; 6+ messages in thread From: Andreas Gruenbacher @ 2016-05-30 9:25 UTC (permalink / raw) To: Mike Marshall; +Cc: Andreas Gruenbacher, linux-fsdevel The ORANGEFS_XATTR_INDEX_ defines are unused; the ORANGEFS_XATTR_NAME_ defines only obfuscate the code. Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> --- fs/orangefs/acl.c | 8 ++++---- fs/orangefs/file.c | 4 ++-- fs/orangefs/orangefs-kernel.h | 11 ----------- fs/orangefs/xattr.c | 17 ++++++----------- 4 files changed, 12 insertions(+), 28 deletions(-) diff --git a/fs/orangefs/acl.c b/fs/orangefs/acl.c index 03f89db..df24864 100644 --- a/fs/orangefs/acl.c +++ b/fs/orangefs/acl.c @@ -18,10 +18,10 @@ struct posix_acl *orangefs_get_acl(struct inode *inode, int type) switch (type) { case ACL_TYPE_ACCESS: - key = ORANGEFS_XATTR_NAME_ACL_ACCESS; + key = XATTR_NAME_POSIX_ACL_ACCESS; break; case ACL_TYPE_DEFAULT: - key = ORANGEFS_XATTR_NAME_ACL_DEFAULT; + key = XATTR_NAME_POSIX_ACL_DEFAULT; break; default: gossip_err("orangefs_get_acl: bogus value of type %d\n", type); @@ -74,7 +74,7 @@ int orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type) switch (type) { case ACL_TYPE_ACCESS: - name = ORANGEFS_XATTR_NAME_ACL_ACCESS; + name = XATTR_NAME_POSIX_ACL_ACCESS; if (acl) { umode_t mode = inode->i_mode; /* @@ -98,7 +98,7 @@ int orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type) } break; case ACL_TYPE_DEFAULT: - name = ORANGEFS_XATTR_NAME_ACL_DEFAULT; + name = XATTR_NAME_POSIX_ACL_DEFAULT; break; default: gossip_err("%s: invalid type %d!\n", __func__, type); diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c index 491e82c..5160a3f 100644 --- a/fs/orangefs/file.c +++ b/fs/orangefs/file.c @@ -516,7 +516,7 @@ 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), - ORANGEFS_XATTR_NAME_DEFAULT_PREFIX, + "", "user.pvfs2.meta_hint", &val, sizeof(val)); if (ret < 0 && ret != -ENODATA) @@ -549,7 +549,7 @@ 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), - ORANGEFS_XATTR_NAME_DEFAULT_PREFIX, + "", "user.pvfs2.meta_hint", &val, sizeof(val), 0); } diff --git a/fs/orangefs/orangefs-kernel.h b/fs/orangefs/orangefs-kernel.h index 2281882..6503e37 100644 --- a/fs/orangefs/orangefs-kernel.h +++ b/fs/orangefs/orangefs-kernel.h @@ -119,17 +119,6 @@ struct client_debug_mask { #define ORANGEFS_CACHE_CREATE_FLAGS 0 #endif /* ((defined ORANGEFS_KERNEL_DEBUG) && (defined CONFIG_DEBUG_SLAB)) */ -/* orangefs xattr and acl related defines */ -#define ORANGEFS_XATTR_INDEX_POSIX_ACL_ACCESS 1 -#define ORANGEFS_XATTR_INDEX_POSIX_ACL_DEFAULT 2 -#define ORANGEFS_XATTR_INDEX_TRUSTED 3 -#define ORANGEFS_XATTR_INDEX_DEFAULT 4 - -#define ORANGEFS_XATTR_NAME_ACL_ACCESS XATTR_NAME_POSIX_ACL_ACCESS -#define ORANGEFS_XATTR_NAME_ACL_DEFAULT XATTR_NAME_POSIX_ACL_DEFAULT -#define ORANGEFS_XATTR_NAME_TRUSTED_PREFIX "trusted." -#define ORANGEFS_XATTR_NAME_DEFAULT_PREFIX "" - /* these functions are defined in orangefs-utils.c */ int orangefs_prepare_cdm_array(char *debug_array_string); int orangefs_prepare_debugfs_help_string(int); diff --git a/fs/orangefs/xattr.c b/fs/orangefs/xattr.c index 5893ddd..f387e8a 100644 --- a/fs/orangefs/xattr.c +++ b/fs/orangefs/xattr.c @@ -456,7 +456,7 @@ static int orangefs_xattr_set_default(const struct xattr_handler *handler, int flags) { return orangefs_inode_setxattr(inode, - ORANGEFS_XATTR_NAME_DEFAULT_PREFIX, + "", name, buffer, size, @@ -471,7 +471,7 @@ static int orangefs_xattr_get_default(const struct xattr_handler *handler, size_t size) { return orangefs_inode_getxattr(inode, - ORANGEFS_XATTR_NAME_DEFAULT_PREFIX, + "", name, buffer, size); @@ -487,7 +487,7 @@ static int orangefs_xattr_set_trusted(const struct xattr_handler *handler, int flags) { return orangefs_inode_setxattr(inode, - ORANGEFS_XATTR_NAME_TRUSTED_PREFIX, + XATTR_TRUSTED_PREFIX, name, buffer, size, @@ -502,25 +502,20 @@ static int orangefs_xattr_get_trusted(const struct xattr_handler *handler, size_t size) { return orangefs_inode_getxattr(inode, - ORANGEFS_XATTR_NAME_TRUSTED_PREFIX, + XATTR_TRUSTED_PREFIX, name, buffer, size); } static struct xattr_handler orangefs_xattr_trusted_handler = { - .prefix = ORANGEFS_XATTR_NAME_TRUSTED_PREFIX, + .prefix = XATTR_TRUSTED_PREFIX, .get = orangefs_xattr_get_trusted, .set = orangefs_xattr_set_trusted, }; static struct xattr_handler orangefs_xattr_default_handler = { - /* - * NOTE: this is set to be the empty string. - * so that all un-prefixed xattrs keys get caught - * here! - */ - .prefix = ORANGEFS_XATTR_NAME_DEFAULT_PREFIX, + .prefix = "", /* match any name => handlers called with full name */ .get = orangefs_xattr_get_default, .set = orangefs_xattr_set_default, }; -- 2.5.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/3] orangefs: Remove redundant "trusted." xattr handler 2016-05-30 9:25 [PATCH v2 0/3] Minor orangefs xattr cleanups Andreas Gruenbacher 2016-05-30 9:25 ` [PATCH v2 1/3] orangefs: Remove useless defines Andreas Gruenbacher @ 2016-05-30 9:26 ` Andreas Gruenbacher 2016-05-30 9:26 ` [PATCH v2 3/3] orangefs: Remove useless xattr prefix arguments Andreas Gruenbacher 2 siblings, 0 replies; 6+ messages in thread From: Andreas Gruenbacher @ 2016-05-30 9:26 UTC (permalink / raw) To: Mike Marshall; +Cc: Andreas Gruenbacher, linux-fsdevel Orangefs has a catch-all xattr handler that effectively does what the trusted handler does already. Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> --- fs/orangefs/xattr.c | 37 ------------------------------------- 1 file changed, 37 deletions(-) diff --git a/fs/orangefs/xattr.c b/fs/orangefs/xattr.c index f387e8a..640a98f 100644 --- a/fs/orangefs/xattr.c +++ b/fs/orangefs/xattr.c @@ -478,42 +478,6 @@ static int orangefs_xattr_get_default(const struct xattr_handler *handler, } -static int orangefs_xattr_set_trusted(const struct xattr_handler *handler, - struct dentry *unused, - struct inode *inode, - const char *name, - const void *buffer, - size_t size, - int flags) -{ - return orangefs_inode_setxattr(inode, - XATTR_TRUSTED_PREFIX, - name, - buffer, - size, - flags); -} - -static int orangefs_xattr_get_trusted(const struct xattr_handler *handler, - struct dentry *unused, - struct inode *inode, - const char *name, - void *buffer, - size_t size) -{ - return orangefs_inode_getxattr(inode, - XATTR_TRUSTED_PREFIX, - name, - buffer, - size); -} - -static struct xattr_handler orangefs_xattr_trusted_handler = { - .prefix = XATTR_TRUSTED_PREFIX, - .get = orangefs_xattr_get_trusted, - .set = orangefs_xattr_set_trusted, -}; - static struct xattr_handler orangefs_xattr_default_handler = { .prefix = "", /* match any name => handlers called with full name */ .get = orangefs_xattr_get_default, @@ -523,7 +487,6 @@ static struct xattr_handler orangefs_xattr_default_handler = { const struct xattr_handler *orangefs_xattr_handlers[] = { &posix_acl_access_xattr_handler, &posix_acl_default_xattr_handler, - &orangefs_xattr_trusted_handler, &orangefs_xattr_default_handler, NULL }; -- 2.5.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] orangefs: Remove useless xattr prefix arguments 2016-05-30 9:25 [PATCH v2 0/3] Minor orangefs xattr cleanups Andreas Gruenbacher 2016-05-30 9:25 ` [PATCH v2 1/3] orangefs: Remove useless defines Andreas Gruenbacher 2016-05-30 9:26 ` [PATCH v2 2/3] orangefs: Remove redundant "trusted." xattr handler Andreas Gruenbacher @ 2016-05-30 9:26 ` Andreas Gruenbacher 2016-06-03 19:44 ` Mike Marshall 2 siblings, 1 reply; 6+ messages in thread From: Andreas Gruenbacher @ 2016-05-30 9:26 UTC (permalink / raw) To: Mike Marshall; +Cc: Andreas Gruenbacher, linux-fsdevel 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 3/3] orangefs: Remove useless xattr prefix arguments 2016-05-30 9:26 ` [PATCH v2 3/3] orangefs: Remove useless xattr prefix arguments Andreas Gruenbacher @ 2016-06-03 19:44 ` Mike Marshall 2016-06-04 9:02 ` Andreas Gruenbacher 0 siblings, 1 reply; 6+ messages in thread From: Mike Marshall @ 2016-06-03 19:44 UTC (permalink / raw) To: Andreas Gruenbacher; +Cc: linux-fsdevel, Mike Marshall 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 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 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 > ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 3/3] orangefs: Remove useless xattr prefix arguments 2016-06-03 19:44 ` Mike Marshall @ 2016-06-04 9:02 ` Andreas Gruenbacher 0 siblings, 0 replies; 6+ messages in thread From: Andreas Gruenbacher @ 2016-06-04 9:02 UTC (permalink / raw) To: Mike Marshall; +Cc: Andreas Gruenbacher, linux-fsdevel Mike, On Fri, Jun 3, 2016 at 9:44 PM, Mike Marshall <hubcap@omnibond.com> wrote: > 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... ah, my mistake. Here's a fixed version. Thanks, Andreas 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 | 83 ++++++++++++++----------------------------- 4 files changed, 30 insertions(+), 66 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..73a0c34 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,15 +97,14 @@ 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 * strings, I am going to explicitly pass the length just in case * we change this later on... */ - new_op->upcall.req.getxattr.key_sz = ret + 1; + new_op->upcall.req.getxattr.key_sz = strlen(name) + 1; ret = service_operation(new_op, "orangefs_inode_getxattr", get_interruptible_flag(inode)); @@ -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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-06-04 9:02 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-05-30 9:25 [PATCH v2 0/3] Minor orangefs xattr cleanups Andreas Gruenbacher 2016-05-30 9:25 ` [PATCH v2 1/3] orangefs: Remove useless defines Andreas Gruenbacher 2016-05-30 9:26 ` [PATCH v2 2/3] orangefs: Remove redundant "trusted." xattr handler Andreas Gruenbacher 2016-05-30 9:26 ` [PATCH v2 3/3] orangefs: Remove useless xattr prefix arguments Andreas Gruenbacher 2016-06-03 19:44 ` Mike Marshall 2016-06-04 9:02 ` Andreas Gruenbacher
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).