From 45dc0caa0a2c69c87d3d2ea41c724ecf0754c2d5 Mon Sep 17 00:00:00 2001 From: Yaniv Michael Kaul Date: Fri, 24 May 2024 23:04:23 +0300 Subject: [PATCH 1/2] Traverse xattrs using a pointer to xattr instead of snprintf() its value The key can be a pointer, pointing to the xattr, instead of copying it. Updates: #1000 Signed-off-by: Yaniv Kaul --- xlators/storage/posix/src/posix-gfid-path.c | 8 +++--- xlators/storage/posix/src/posix-helpers.c | 8 +++--- .../storage/posix/src/posix-inode-fd-ops.c | 27 ++++++++++++------- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/xlators/storage/posix/src/posix-gfid-path.c b/xlators/storage/posix/src/posix-gfid-path.c index eb6f74cf29b..26d8c3ae502 100644 --- a/xlators/storage/posix/src/posix-gfid-path.c +++ b/xlators/storage/posix/src/posix-gfid-path.c @@ -47,9 +47,7 @@ posix_get_gfid2path(xlator_t *this, inode_t *inode, const char *real_path, char *value = NULL; size_t remaining_size = 0; size_t bytes = 0; - char keybuffer[4096] = { - 0, - }; + char *keybuffer = NULL; uuid_t pargfid = { 0, @@ -137,8 +135,8 @@ posix_get_gfid2path(xlator_t *this, inode_t *inode, const char *real_path, remaining_size = size; list_offset = 0; while (remaining_size > 0) { - len = snprintf(keybuffer, sizeof(keybuffer), "%s", - list + list_offset); + keybuffer = list + list_offset; + len = strlen(keybuffer); if (!posix_is_gfid2path_xattr(keybuffer)) { goto ignore; diff --git a/xlators/storage/posix/src/posix-helpers.c b/xlators/storage/posix/src/posix-helpers.c index 08bb4acd3e2..cac0979c3be 100644 --- a/xlators/storage/posix/src/posix-helpers.c +++ b/xlators/storage/posix/src/posix-helpers.c @@ -314,9 +314,8 @@ _posix_get_marker_all_contributions(posix_xattr_filler_t *filler) ssize_t size = -1, remaining_size = -1, list_offset = 0; int ret = -1; int len; - char *list = NULL, key[4096] = { - 0, - }; + char *list = NULL; + char *key; if (filler->real_path) size = sys_llistxattr(filler->real_path, NULL, 0); @@ -364,7 +363,8 @@ _posix_get_marker_all_contributions(posix_xattr_filler_t *filler) list_offset = 0; while (remaining_size > 0) { - len = snprintf(key, sizeof(key), "%s", list + list_offset); + key = list + list_offset; + len = strlen(key); if (fnmatch(marker_contri_key, key, 0) == 0) { (void)_posix_xattr_get_set_from_backend(filler, key); } diff --git a/xlators/storage/posix/src/posix-inode-fd-ops.c b/xlators/storage/posix/src/posix-inode-fd-ops.c index 911403ed3e2..83e295eb8ba 100644 --- a/xlators/storage/posix/src/posix-inode-fd-ops.c +++ b/xlators/storage/posix/src/posix-inode-fd-ops.c @@ -3457,9 +3457,7 @@ posix_get_ancestry_non_directory(xlator_t *this, inode_t *leaf_inode, inode_t *parent = NULL; loc_t *loc = NULL; char *leaf_path = NULL; - char key[4096] = { - 0, - }; + char *key; char dirpath[PATH_MAX] = { 0, }; @@ -3533,7 +3531,8 @@ posix_get_ancestry_non_directory(xlator_t *this, inode_t *leaf_inode, } while (remaining_size > 0) { - len = snprintf(key, sizeof(key), "%s", list + list_offset); + key = list + list_offset; + len = strlen(key); if (strncmp(key, PGFID_XATTR_KEY_PREFIX, SLEN(PGFID_XATTR_KEY_PREFIX)) != 0) goto next; @@ -4034,10 +4033,9 @@ posix_getxattr(call_frame_t *frame, xlator_t *this, loc_t *loc, } remaining_size = size; list_offset = 0; - keybuffer = alloca(XATTR_KEY_BUF_SIZE); while (remaining_size > 0) { - keybuff_len = snprintf(keybuffer, XATTR_KEY_BUF_SIZE, "%s", - list + list_offset); + keybuffer = list + list_offset; + keybuff_len = strlen(keybuffer); ret = posix_handle_georep_xattrs(frame, keybuffer, NULL, _gf_false); if (ret == -1) @@ -4163,9 +4161,13 @@ posix_fgetxattr(call_frame_t *frame, xlator_t *this, fd_t *fd, const char *name, char *list = NULL; dict_t *dict = NULL; int ret = -1; +#ifdef GF_DARWIN_HOST_OS char key[4096] = { 0, }; +#else + char *key = NULL; +#endif int key_len; char *value_buf = NULL; gf_boolean_t have_val = _gf_false; @@ -4279,8 +4281,11 @@ posix_fgetxattr(call_frame_t *frame, xlator_t *this, fd_t *fd, const char *name, value_buf = alloca(XATTR_VAL_BUF_SIZE); if (name) { +#ifndef GF_DARWIN_HOST_OS + key = name; + key_len = strlen(key); +#else key_len = snprintf(key, sizeof(key), "%s", name); -#ifdef GF_DARWIN_HOST_OS struct posix_private *priv = NULL; priv = this->private; if (priv->xattr_user_namespace == XATTR_STRIP) { @@ -4403,8 +4408,12 @@ posix_fgetxattr(call_frame_t *frame, xlator_t *this, fd_t *fd, const char *name, while (remaining_size > 0) { if (*(list + list_offset) == '\0') break; - +#ifndef GF_DARWIN_HOST_OS + key = list + list_offset; + key_len = strlen(key); +#else key_len = snprintf(key, sizeof(key), "%s", list + list_offset); +#endif have_val = _gf_false; size = sys_fgetxattr(_fd, key, value_buf, XATTR_VAL_BUF_SIZE - 1); if (size >= 0) { From 824b5530bd8e8a318a77493bf98a32a1a03bbcd4 Mon Sep 17 00:00:00 2001 From: Yaniv Michael Kaul Date: Wed, 29 May 2024 09:34:51 +0300 Subject: [PATCH 2/2] Fix compilation warning - make sure key is const Signed-off-by: Yaniv Kaul --- libglusterfs/src/dict.c | 6 +++--- libglusterfs/src/glusterfs/dict.h | 6 +++--- xlators/storage/posix/src/posix-inode-fd-ops.c | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/libglusterfs/src/dict.c b/libglusterfs/src/dict.c index 243889280ee..2c4fb20385f 100644 --- a/libglusterfs/src/dict.c +++ b/libglusterfs/src/dict.c @@ -357,7 +357,7 @@ dict_lookup(dict_t *this, char *key, data_t **data) } static int32_t -dict_set_lk(dict_t *this, char *key, const int key_len, data_t *value, +dict_set_lk(dict_t *this, const char *key, const int key_len, data_t *value, gf_boolean_t replace) { data_pair_t *pair; @@ -394,7 +394,7 @@ dict_set_lk(dict_t *this, char *key, const int key_len, data_t *value, } int32_t -dict_setn(dict_t *this, char *key, const int keylen, data_t *value) +dict_setn(dict_t *this, const char *key, const int keylen, data_t *value) { int32_t ret; @@ -2151,7 +2151,7 @@ dict_set_static_ptr(dict_t *this, char *key, void *ptr) } int -dict_set_dynptr(dict_t *this, char *key, void *ptr, size_t len) +dict_set_dynptr(dict_t *this, const char *key, void *ptr, size_t len) { data_t *data = data_from_dynptr(ptr, len); int ret = 0; diff --git a/libglusterfs/src/glusterfs/dict.h b/libglusterfs/src/glusterfs/dict.h index 635bdae3163..981a6f941ed 100644 --- a/libglusterfs/src/glusterfs/dict.h +++ b/libglusterfs/src/glusterfs/dict.h @@ -125,10 +125,10 @@ is_data_equal(data_t *one, data_t *two); /* function to set a key/value pair (overwrite existing if matches the key */ int32_t -dict_setn(dict_t *this, char *key, const int keylen, data_t *value); +dict_setn(dict_t *this, const char *key, const int keylen, data_t *value); static inline int32_t -dict_set(dict_t *this, char *key, data_t *value) +dict_set(dict_t *this, const char *key, data_t *value) { return dict_setn(this, key, strlen(key), value); } @@ -369,7 +369,7 @@ dict_get_ptr(dict_t *this, char *key, void **ptr); GF_MUST_CHECK int dict_get_ptr_and_len(dict_t *this, char *key, void **ptr, int *len); GF_MUST_CHECK int -dict_set_dynptr(dict_t *this, char *key, void *ptr, size_t size); +dict_set_dynptr(dict_t *this, const char *key, void *ptr, size_t size); GF_MUST_CHECK int dict_get_bin(dict_t *this, char *key, void **ptr); diff --git a/xlators/storage/posix/src/posix-inode-fd-ops.c b/xlators/storage/posix/src/posix-inode-fd-ops.c index 83e295eb8ba..a4223a690c0 100644 --- a/xlators/storage/posix/src/posix-inode-fd-ops.c +++ b/xlators/storage/posix/src/posix-inode-fd-ops.c @@ -4166,7 +4166,7 @@ posix_fgetxattr(call_frame_t *frame, xlator_t *this, fd_t *fd, const char *name, 0, }; #else - char *key = NULL; + const char *key = NULL; #endif int key_len; char *value_buf = NULL;