diff options
author | Andreas Gruenbacher <agruenba@redhat.com> | 2016-06-04 05:02:33 -0400 |
---|---|---|
committer | Mike Marshall <hubcap@omnibond.com> | 2016-07-05 15:47:27 -0400 |
commit | d373a712c1142a4e119e359df63c192afa9bb2fb (patch) | |
tree | b313d3fea4ed4b5adb71afa044cdf1341f4ff1bc | |
parent | 2ce8272a1014d9d0d2f859ffba9a815f9ce12f99 (diff) |
orangefs: Remove useless xattr prefix arguments
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>
Signed-off-by: Mike Marshall <hubcap@omnibond.com>
-rw-r--r-- | fs/orangefs/acl.c | 9 | ||||
-rw-r--r-- | fs/orangefs/file.c | 2 | ||||
-rw-r--r-- | fs/orangefs/orangefs-kernel.h | 2 | ||||
-rw-r--r-- | 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 df2486402dc1..28f2195cd798 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) | |||
43 | get_khandle_from_ino(inode), | 43 | get_khandle_from_ino(inode), |
44 | key, | 44 | key, |
45 | type); | 45 | type); |
46 | ret = orangefs_inode_getxattr(inode, | 46 | ret = orangefs_inode_getxattr(inode, key, value, |
47 | "", | 47 | ORANGEFS_MAX_XATTR_VALUELEN); |
48 | key, | ||
49 | value, | ||
50 | ORANGEFS_MAX_XATTR_VALUELEN); | ||
51 | /* if the key exists, convert it to an in-memory rep */ | 48 | /* if the key exists, convert it to an in-memory rep */ |
52 | if (ret > 0) { | 49 | if (ret > 0) { |
53 | acl = posix_acl_from_xattr(&init_user_ns, value, ret); | 50 | 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) | |||
131 | * will xlate to a removexattr. However, we don't want removexattr | 128 | * will xlate to a removexattr. However, we don't want removexattr |
132 | * complain if attributes does not exist. | 129 | * complain if attributes does not exist. |
133 | */ | 130 | */ |
134 | error = orangefs_inode_setxattr(inode, "", name, value, size, 0); | 131 | error = orangefs_inode_setxattr(inode, name, value, size, 0); |
135 | 132 | ||
136 | out: | 133 | out: |
137 | kfree(value); | 134 | kfree(value); |
diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c index 5160a3f27e71..526040e09f78 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 | |||
516 | if (cmd == FS_IOC_GETFLAGS) { | 516 | if (cmd == FS_IOC_GETFLAGS) { |
517 | val = 0; | 517 | val = 0; |
518 | ret = orangefs_inode_getxattr(file_inode(file), | 518 | ret = orangefs_inode_getxattr(file_inode(file), |
519 | "", | ||
520 | "user.pvfs2.meta_hint", | 519 | "user.pvfs2.meta_hint", |
521 | &val, sizeof(val)); | 520 | &val, sizeof(val)); |
522 | if (ret < 0 && ret != -ENODATA) | 521 | if (ret < 0 && ret != -ENODATA) |
@@ -549,7 +548,6 @@ static long orangefs_ioctl(struct file *file, unsigned int cmd, unsigned long ar | |||
549 | "orangefs_ioctl: FS_IOC_SETFLAGS: %llu\n", | 548 | "orangefs_ioctl: FS_IOC_SETFLAGS: %llu\n", |
550 | (unsigned long long)val); | 549 | (unsigned long long)val); |
551 | ret = orangefs_inode_setxattr(file_inode(file), | 550 | ret = orangefs_inode_setxattr(file_inode(file), |
552 | "", | ||
553 | "user.pvfs2.meta_hint", | 551 | "user.pvfs2.meta_hint", |
554 | &val, sizeof(val), 0); | 552 | &val, sizeof(val), 0); |
555 | } | 553 | } |
diff --git a/fs/orangefs/orangefs-kernel.h b/fs/orangefs/orangefs-kernel.h index 6503e376047e..7b542f168d44 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); | |||
517 | int orangefs_flush_inode(struct inode *inode); | 517 | int orangefs_flush_inode(struct inode *inode); |
518 | 518 | ||
519 | ssize_t orangefs_inode_getxattr(struct inode *inode, | 519 | ssize_t orangefs_inode_getxattr(struct inode *inode, |
520 | const char *prefix, | ||
521 | const char *name, | 520 | const char *name, |
522 | void *buffer, | 521 | void *buffer, |
523 | size_t size); | 522 | size_t size); |
524 | 523 | ||
525 | int orangefs_inode_setxattr(struct inode *inode, | 524 | int orangefs_inode_setxattr(struct inode *inode, |
526 | const char *prefix, | ||
527 | const char *name, | 525 | const char *name, |
528 | const void *value, | 526 | const void *value, |
529 | size_t size, | 527 | size_t size, |
diff --git a/fs/orangefs/xattr.c b/fs/orangefs/xattr.c index 640a98f9144d..73a0c3411d4b 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) | |||
59 | * unless the key does not exist for the file and/or if | 59 | * unless the key does not exist for the file and/or if |
60 | * there were errors in fetching the attribute value. | 60 | * there were errors in fetching the attribute value. |
61 | */ | 61 | */ |
62 | ssize_t orangefs_inode_getxattr(struct inode *inode, const char *prefix, | 62 | ssize_t orangefs_inode_getxattr(struct inode *inode, const char *name, |
63 | const char *name, void *buffer, size_t size) | 63 | void *buffer, size_t size) |
64 | { | 64 | { |
65 | struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode); | 65 | struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode); |
66 | struct orangefs_kernel_op_s *new_op = NULL; | 66 | struct orangefs_kernel_op_s *new_op = NULL; |
@@ -70,12 +70,12 @@ ssize_t orangefs_inode_getxattr(struct inode *inode, const char *prefix, | |||
70 | int fsgid; | 70 | int fsgid; |
71 | 71 | ||
72 | gossip_debug(GOSSIP_XATTR_DEBUG, | 72 | gossip_debug(GOSSIP_XATTR_DEBUG, |
73 | "%s: prefix %s name %s, buffer_size %zd\n", | 73 | "%s: name %s, buffer_size %zd\n", |
74 | __func__, prefix, name, size); | 74 | __func__, name, size); |
75 | 75 | ||
76 | if ((strlen(name) + strlen(prefix)) >= ORANGEFS_MAX_XATTR_NAMELEN) { | 76 | if (strlen(name) >= ORANGEFS_MAX_XATTR_NAMELEN) { |
77 | gossip_err("Invalid key length (%d)\n", | 77 | gossip_err("Invalid key length (%d)\n", |
78 | (int)(strlen(name) + strlen(prefix))); | 78 | (int)strlen(name)); |
79 | return -EINVAL; | 79 | return -EINVAL; |
80 | } | 80 | } |
81 | 81 | ||
@@ -97,15 +97,14 @@ ssize_t orangefs_inode_getxattr(struct inode *inode, const char *prefix, | |||
97 | goto out_unlock; | 97 | goto out_unlock; |
98 | 98 | ||
99 | new_op->upcall.req.getxattr.refn = orangefs_inode->refn; | 99 | new_op->upcall.req.getxattr.refn = orangefs_inode->refn; |
100 | ret = snprintf((char *)new_op->upcall.req.getxattr.key, | 100 | strcpy(new_op->upcall.req.getxattr.key, name); |
101 | ORANGEFS_MAX_XATTR_NAMELEN, "%s%s", prefix, name); | ||
102 | 101 | ||
103 | /* | 102 | /* |
104 | * NOTE: Although keys are meant to be NULL terminated textual | 103 | * NOTE: Although keys are meant to be NULL terminated textual |
105 | * strings, I am going to explicitly pass the length just in case | 104 | * strings, I am going to explicitly pass the length just in case |
106 | * we change this later on... | 105 | * we change this later on... |
107 | */ | 106 | */ |
108 | new_op->upcall.req.getxattr.key_sz = ret + 1; | 107 | new_op->upcall.req.getxattr.key_sz = strlen(name) + 1; |
109 | 108 | ||
110 | ret = service_operation(new_op, "orangefs_inode_getxattr", | 109 | ret = service_operation(new_op, "orangefs_inode_getxattr", |
111 | get_interruptible_flag(inode)); | 110 | get_interruptible_flag(inode)); |
@@ -163,10 +162,8 @@ out_unlock: | |||
163 | return ret; | 162 | return ret; |
164 | } | 163 | } |
165 | 164 | ||
166 | static int orangefs_inode_removexattr(struct inode *inode, | 165 | static int orangefs_inode_removexattr(struct inode *inode, const char *name, |
167 | const char *prefix, | 166 | int flags) |
168 | const char *name, | ||
169 | int flags) | ||
170 | { | 167 | { |
171 | struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode); | 168 | struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode); |
172 | struct orangefs_kernel_op_s *new_op = NULL; | 169 | struct orangefs_kernel_op_s *new_op = NULL; |
@@ -183,12 +180,8 @@ static int orangefs_inode_removexattr(struct inode *inode, | |||
183 | * textual strings, I am going to explicitly pass the | 180 | * textual strings, I am going to explicitly pass the |
184 | * length just in case we change this later on... | 181 | * length just in case we change this later on... |
185 | */ | 182 | */ |
186 | ret = snprintf((char *)new_op->upcall.req.removexattr.key, | 183 | strcpy(new_op->upcall.req.removexattr.key, name); |
187 | ORANGEFS_MAX_XATTR_NAMELEN, | 184 | new_op->upcall.req.removexattr.key_sz = strlen(name) + 1; |
188 | "%s%s", | ||
189 | (prefix ? prefix : ""), | ||
190 | name); | ||
191 | new_op->upcall.req.removexattr.key_sz = ret + 1; | ||
192 | 185 | ||
193 | gossip_debug(GOSSIP_XATTR_DEBUG, | 186 | gossip_debug(GOSSIP_XATTR_DEBUG, |
194 | "orangefs_inode_removexattr: key %s, key_sz %d\n", | 187 | "orangefs_inode_removexattr: key %s, key_sz %d\n", |
@@ -223,8 +216,8 @@ out_unlock: | |||
223 | * Returns a -ve number on error and 0 on success. Key is text, but value | 216 | * Returns a -ve number on error and 0 on success. Key is text, but value |
224 | * can be binary! | 217 | * can be binary! |
225 | */ | 218 | */ |
226 | int orangefs_inode_setxattr(struct inode *inode, const char *prefix, | 219 | int orangefs_inode_setxattr(struct inode *inode, const char *name, |
227 | const char *name, const void *value, size_t size, int flags) | 220 | const void *value, size_t size, int flags) |
228 | { | 221 | { |
229 | struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode); | 222 | struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode); |
230 | struct orangefs_kernel_op_s *new_op; | 223 | struct orangefs_kernel_op_s *new_op; |
@@ -232,8 +225,8 @@ int orangefs_inode_setxattr(struct inode *inode, const char *prefix, | |||
232 | int ret = -ENOMEM; | 225 | int ret = -ENOMEM; |
233 | 226 | ||
234 | gossip_debug(GOSSIP_XATTR_DEBUG, | 227 | gossip_debug(GOSSIP_XATTR_DEBUG, |
235 | "%s: prefix %s, name %s, buffer_size %zd\n", | 228 | "%s: name %s, buffer_size %zd\n", |
236 | __func__, prefix, name, size); | 229 | __func__, name, size); |
237 | 230 | ||
238 | if (size >= ORANGEFS_MAX_XATTR_VALUELEN || | 231 | if (size >= ORANGEFS_MAX_XATTR_VALUELEN || |
239 | flags < 0) { | 232 | flags < 0) { |
@@ -245,29 +238,19 @@ int orangefs_inode_setxattr(struct inode *inode, const char *prefix, | |||
245 | 238 | ||
246 | internal_flag = convert_to_internal_xattr_flags(flags); | 239 | internal_flag = convert_to_internal_xattr_flags(flags); |
247 | 240 | ||
248 | if (prefix) { | 241 | if (strlen(name) >= ORANGEFS_MAX_XATTR_NAMELEN) { |
249 | if (strlen(name) + strlen(prefix) >= ORANGEFS_MAX_XATTR_NAMELEN) { | 242 | gossip_err |
250 | gossip_err | 243 | ("orangefs_inode_setxattr: bogus key size (%d)\n", |
251 | ("orangefs_inode_setxattr: bogus key size (%d)\n", | 244 | (int)(strlen(name))); |
252 | (int)(strlen(name) + strlen(prefix))); | 245 | return -EINVAL; |
253 | return -EINVAL; | ||
254 | } | ||
255 | } else { | ||
256 | if (strlen(name) >= ORANGEFS_MAX_XATTR_NAMELEN) { | ||
257 | gossip_err | ||
258 | ("orangefs_inode_setxattr: bogus key size (%d)\n", | ||
259 | (int)(strlen(name))); | ||
260 | return -EINVAL; | ||
261 | } | ||
262 | } | 246 | } |
263 | 247 | ||
264 | /* This is equivalent to a removexattr */ | 248 | /* This is equivalent to a removexattr */ |
265 | if (size == 0 && value == NULL) { | 249 | if (size == 0 && value == NULL) { |
266 | gossip_debug(GOSSIP_XATTR_DEBUG, | 250 | gossip_debug(GOSSIP_XATTR_DEBUG, |
267 | "removing xattr (%s%s)\n", | 251 | "removing xattr (%s)\n", |
268 | prefix, | ||
269 | name); | 252 | name); |
270 | return orangefs_inode_removexattr(inode, prefix, name, flags); | 253 | return orangefs_inode_removexattr(inode, name, flags); |
271 | } | 254 | } |
272 | 255 | ||
273 | gossip_debug(GOSSIP_XATTR_DEBUG, | 256 | gossip_debug(GOSSIP_XATTR_DEBUG, |
@@ -288,11 +271,8 @@ int orangefs_inode_setxattr(struct inode *inode, const char *prefix, | |||
288 | * strings, I am going to explicitly pass the length just in | 271 | * strings, I am going to explicitly pass the length just in |
289 | * case we change this later on... | 272 | * case we change this later on... |
290 | */ | 273 | */ |
291 | ret = snprintf((char *)new_op->upcall.req.setxattr.keyval.key, | 274 | strcpy(new_op->upcall.req.setxattr.keyval.key, name); |
292 | ORANGEFS_MAX_XATTR_NAMELEN, | 275 | new_op->upcall.req.setxattr.keyval.key_sz = strlen(name) + 1; |
293 | "%s%s", | ||
294 | prefix, name); | ||
295 | new_op->upcall.req.setxattr.keyval.key_sz = ret + 1; | ||
296 | memcpy(new_op->upcall.req.setxattr.keyval.val, value, size); | 276 | memcpy(new_op->upcall.req.setxattr.keyval.val, value, size); |
297 | new_op->upcall.req.setxattr.keyval.val_sz = size; | 277 | new_op->upcall.req.setxattr.keyval.val_sz = size; |
298 | 278 | ||
@@ -455,12 +435,7 @@ static int orangefs_xattr_set_default(const struct xattr_handler *handler, | |||
455 | size_t size, | 435 | size_t size, |
456 | int flags) | 436 | int flags) |
457 | { | 437 | { |
458 | return orangefs_inode_setxattr(inode, | 438 | return orangefs_inode_setxattr(inode, name, buffer, size, flags); |
459 | "", | ||
460 | name, | ||
461 | buffer, | ||
462 | size, | ||
463 | flags); | ||
464 | } | 439 | } |
465 | 440 | ||
466 | static int orangefs_xattr_get_default(const struct xattr_handler *handler, | 441 | 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, | |||
470 | void *buffer, | 445 | void *buffer, |
471 | size_t size) | 446 | size_t size) |
472 | { | 447 | { |
473 | return orangefs_inode_getxattr(inode, | 448 | return orangefs_inode_getxattr(inode, name, buffer, size); |
474 | "", | ||
475 | name, | ||
476 | buffer, | ||
477 | size); | ||
478 | 449 | ||
479 | } | 450 | } |
480 | 451 | ||