From ce17a3a66dd15f09d1bf5404f7f3dee860ca6f8c Mon Sep 17 00:00:00 2001 From: Yaniv Kaul Date: Tue, 21 Aug 2018 20:39:16 +0300 Subject: multiple xlators: strncpy()->sprintf(), reduce strlen()'s xlators/cluster/afr/src/afr-common.c xlators/cluster/dht/src/dht-common.c xlators/cluster/dht/src/dht-rebalance.c xlators/cluster/stripe/src/stripe-helpers.c strncpy may not be very efficient for short strings copied into a large buffer: If the length of src is less than n, strncpy() writes additional null bytes to dest to ensure that a total of n bytes are written. Instead, use snprintf(). Also: - save the result of strlen() and re-use it when possible. - move from strlen to SLEN (sizeof() ) for const strings. Compile-tested only! Change-Id: Icdf79dd3d9f9ff120e4720ff2b8bd016df575c38 updates: bz#1193929 Signed-off-by: Yaniv Kaul --- xlators/cluster/dht/src/dht-common.c | 22 ++++++++++------------ xlators/cluster/dht/src/dht-rebalance.c | 6 +++--- 2 files changed, 13 insertions(+), 15 deletions(-) (limited to 'xlators/cluster/dht') diff --git a/xlators/cluster/dht/src/dht-common.c b/xlators/cluster/dht/src/dht-common.c index c7c7fbf22ba..69555a22e5e 100644 --- a/xlators/cluster/dht/src/dht-common.c +++ b/xlators/cluster/dht/src/dht-common.c @@ -428,7 +428,7 @@ dht_aggregate (dict_t *this, char *key, data_t *value, void *data) goto out; } else { /* compare user xattrs only */ - if (!strncmp (key, "user.", strlen ("user."))) { + if (!strncmp (key, "user.", SLEN ("user."))) { ret = dict_lookup (dst, key, &dict_data); if (!ret && dict_data && value) { ret = is_data_equal (dict_data, value); @@ -4379,7 +4379,7 @@ dht_vgetxattr_alloc_and_fill (dht_local_t *local, dict_t *xattr, xlator_t *this, local->alloc_len += strlen(value); if (!local->xattr_val) { - local->alloc_len += sizeof (DHT_PATHINFO_HEADER) + 10; + local->alloc_len += (SLEN (DHT_PATHINFO_HEADER) + 10); local->xattr_val = GF_MALLOC (local->alloc_len, gf_common_mt_char); if (!local->xattr_val) { @@ -5189,8 +5189,9 @@ dht_getxattr (call_frame_t *frame, xlator_t *this, if (!DHT_IS_DIR(layout)) goto no_dht_is_dir; - if (strncmp (key, GF_XATTR_GET_REAL_FILENAME_KEY, - strlen (GF_XATTR_GET_REAL_FILENAME_KEY)) == 0) { + if ((strncmp (key, GF_XATTR_GET_REAL_FILENAME_KEY, + SLEN (GF_XATTR_GET_REAL_FILENAME_KEY)) == 0) + && DHT_IS_DIR(layout)) { dht_getxattr_get_real_filename (frame, this, loc, key, xdata); return 0; } @@ -5442,7 +5443,7 @@ dht_fgetxattr (call_frame_t *frame, xlator_t *this, if ((fd->inode->ia_type == IA_IFDIR) && key && (strncmp (key, GF_XATTR_LOCKINFO_KEY, - strlen (GF_XATTR_LOCKINFO_KEY)) != 0)) { + SLEN (GF_XATTR_LOCKINFO_KEY)) != 0)) { local->call_cnt = conf->subvolume_cnt; cnt = conf->subvolume_cnt; ret = dht_inode_ctx_mdsvol_get (fd->inode, this, &mds_subvol); @@ -6165,7 +6166,7 @@ dht_setxattr (call_frame_t *frame, xlator_t *this, goto err; } - memcpy (value, tmp->data, ((tmp->len < 4095) ? tmp->len : 4095)); + memcpy (value, tmp->data, min (tmp->len, 4095)); local->key = gf_strdup (value); local->call_cnt = conf->subvolume_cnt; @@ -6214,7 +6215,7 @@ dht_setxattr (call_frame_t *frame, xlator_t *this, tmp = dict_get (xattr, "distribute.directory-spread-count"); if (tmp) { /* Setxattr value is packed as 'binary', not string */ - memcpy (value, tmp->data, ((tmp->len < 4095)?tmp->len:4095)); + memcpy (value, tmp->data, min (tmp->len, 4095)); ret = gf_string2uint32 (value, &dir_spread); if (!ret && ((dir_spread <= conf->subvolume_cnt) && (dir_spread > 0))) { @@ -11422,7 +11423,6 @@ dht_log_new_layout_for_dir_selfheal (xlator_t *this, loc_t *loc, int i = 0; gf_loglevel_t log_level = gf_log_get_loglevel(); int ret = 0; - int max_string_len = 0; if (log_level < GF_LOG_INFO) return; @@ -11439,9 +11439,7 @@ dht_log_new_layout_for_dir_selfheal (xlator_t *this, loc_t *loc, if (!loc->path) return; - max_string_len = sizeof (string); - - ret = snprintf (string, max_string_len, "Setting layout of %s with ", + ret = snprintf (string, sizeof (string), "Setting layout of %s with ", loc->path); if (ret < 0) @@ -11461,7 +11459,7 @@ dht_log_new_layout_for_dir_selfheal (xlator_t *this, loc_t *loc, for (i = 0; i < layout->cnt; i++) { - ret = snprintf (string, max_string_len, + ret = snprintf (string, sizeof (string), "[Subvol_name: %s, Err: %d , Start: " "%"PRIu32 " , Stop: %"PRIu32 " , Hash: %" PRIu32 " ], ", diff --git a/xlators/cluster/dht/src/dht-rebalance.c b/xlators/cluster/dht/src/dht-rebalance.c index ed61880c8e4..9983429acec 100644 --- a/xlators/cluster/dht/src/dht-rebalance.c +++ b/xlators/cluster/dht/src/dht-rebalance.c @@ -147,12 +147,12 @@ dht_send_rebalance_event (xlator_t *this, int cmd, gf_defrag_status_t status) volname = defrag->tier_conf.volname; } else { /* DHT volume */ - len = strlen (this->name); + len = strlen (this->name) - strlen (suffix); tmpstr = gf_strdup (this->name); if (tmpstr) { - ptr = tmpstr + (len - strlen (suffix)); + ptr = tmpstr + len; if (!strcmp (ptr, suffix)) { - tmpstr[len - strlen (suffix)] = '\0'; + tmpstr[len] = '\0'; volname = tmpstr; } } -- cgit