multiple xlators (storage/posix): strncpy()->sprintf(), reduce strlen()'s

xlators/storage/posix/src/posix-gfid-path.c
xlators/storage/posix/src/posix-handle.c
xlators/storage/posix/src/posix-helpers.c
xlators/storage/posix/src/posix-inode-fd-ops.c
xlators/storage/posix/src/posix.h

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: I056939f111a4ec6bc8ebd539ebcaf9eb67da6c95
updates: bz#1193929
Signed-off-by: Yaniv Kaul <ykaul@redhat.com>

5 files changed, 62 insertions, 53 deletions

diff --git a/xlators/storage/posix/src/posix-gfid-path.c b/xlators/storage/posix/src/posix-gfid-path.c
index 686276eb5a9..a08168a3bd4 100644
--- a/xlators/storage/posix/src/posix-gfid-path.c
+++ b/xlators/storage/posix/src/posix-gfid-path.c
@@ -124,6 +124,7 @@ posix_get_gfid2path (xlator_t *this, inode_t *inode, const char *real_path,
         struct posix_private  *priv                            = NULL;
         char                   pargfid_str[UUID_CANONICAL_FORM_LEN + 1] = {0,};
         gf_boolean_t           found                           = _gf_false;
+        int                    len;
 
         priv = this->private;
 
@@ -196,8 +197,8 @@ posix_get_gfid2path (xlator_t *this, inode_t *inode, const char *real_path,
                 remaining_size = size;
                 list_offset = 0;
                 while (remaining_size > 0) {
-                        strncpy (keybuffer, list + list_offset,
-                                 sizeof(keybuffer));
+                        snprintf (keybuffer, sizeof (keybuffer), "%s",
+                                  list + list_offset);
 
                         if (!posix_is_gfid2path_xattr (keybuffer)) {
                                 goto ignore;
@@ -228,8 +229,9 @@ posix_get_gfid2path (xlator_t *this, inode_t *inode, const char *real_path,
                         i++;
 
 ignore:
-                        remaining_size -= strlen (keybuffer) + 1;
-                        list_offset += strlen (keybuffer) + 1;
+                        len = strlen (keybuffer);
+                        remaining_size -= (len + 1);
+                        list_offset += (len + 1);
                 } /* while (remaining_size > 0) */
 
                 /* gfid2path xattr is absent in the list of xattrs */
diff --git a/xlators/storage/posix/src/posix-handle.c b/xlators/storage/posix/src/posix-handle.c
index 2c88bc63809..802dcf1cc2a 100644
--- a/xlators/storage/posix/src/posix-handle.c
+++ b/xlators/storage/posix/src/posix-handle.c
@@ -114,9 +114,7 @@ posix_make_ancestral_node (const char *priv_base_path, char *path, int pathsize,
                 entry->inode = inode_ref (inode);
 
                 list_add_tail (&entry->list, &head->list);
-                strcpy (real_path, priv_base_path);
-                strcat (real_path, "/");
-                strcat (real_path, path);
+                snprintf(real_path, sizeof (real_path), "%s/%s", priv_base_path, path);
                 loc.inode = inode_ref (inode);
                 gf_uuid_copy (loc.gfid, inode->gfid);
 
@@ -173,7 +171,7 @@ posix_make_ancestryfromgfid (xlator_t *this, char *path, int pathsize,
 
                         *parent = inode_ref (itable->root);
 
-                        saved_dir = alloca(strlen("/") + 1);
+                        saved_dir = alloca(sizeof ("/"));
                         strcpy(saved_dir, "/");
                         dir_stack[top] = saved_dir;
                         break;
@@ -498,7 +496,7 @@ posix_handle_gfid_path (xlator_t *this, uuid_t gfid, const char *basename,
                         len = snprintf (buf, buflen, "%s/%s", priv->base_path,
                                         basename);
                 } else {
-                        strncpy (buf, priv->base_path, buflen);
+                        len = snprintf (buf, buflen, "%s", priv->base_path);
                 }
                 goto out;
         }
@@ -536,8 +534,8 @@ posix_handle_init (xlator_t *this)
                 return -1;
         }
 
-        handle_pfx = alloca (priv->base_path_length + 1 + strlen (GF_HIDDEN_PATH)
-                             + 1);
+        handle_pfx = alloca (priv->base_path_length + 1 +
+                             SLEN (GF_HIDDEN_PATH) + 1);
 
         sprintf (handle_pfx, "%s/%s", priv->base_path, GF_HIDDEN_PATH);
 
@@ -716,16 +714,18 @@ posix_handle_trash_init (xlator_t *this)
 
         priv = this->private;
 
-        priv->trash_path = GF_CALLOC (1, priv->base_path_length + strlen ("/")
-                                      + strlen (GF_HIDDEN_PATH) + strlen ("/")
-                                      + strlen (TRASH_DIR) + 1,
+        priv->trash_path = GF_MALLOC (priv->base_path_length + SLEN ("/")
+                                      + SLEN (GF_HIDDEN_PATH) + SLEN ("/")
+                                      + SLEN (TRASH_DIR) + 1,
                                       gf_posix_mt_trash_path);
 
         if (!priv->trash_path)
                 goto out;
 
-        strncpy (priv->trash_path, priv->base_path, priv->base_path_length);
-        strcat (priv->trash_path, "/" GF_HIDDEN_PATH "/" TRASH_DIR);
+        snprintf (priv->trash_path, priv->base_path_length
+                  + SLEN (GF_HIDDEN_PATH) + SLEN (TRASH_DIR) + 3,
+                  "%s/%s/%s", priv->base_path, GF_HIDDEN_PATH, TRASH_DIR);
+
         ret = posix_handle_new_trash_init (this, priv->trash_path);
         if (ret)
                 goto out;
diff --git a/xlators/storage/posix/src/posix-helpers.c b/xlators/storage/posix/src/posix-helpers.c
index 99be6366e11..b4d433e369d 100644
--- a/xlators/storage/posix/src/posix-helpers.c
+++ b/xlators/storage/posix/src/posix-helpers.c
@@ -329,6 +329,7 @@ _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, };
 
         if (filler->real_path)
@@ -384,9 +385,9 @@ _posix_get_marker_all_contributions (posix_xattr_filler_t *filler)
                 if (fnmatch (marker_contri_key, key, 0) == 0) {
                         ret = _posix_xattr_get_set_from_backend (filler, key);
                 }
-
-                remaining_size -= strlen (key) + 1;
-                list_offset += strlen (key) + 1;
+                len = strlen (key);
+                remaining_size -= (len + 1);
+                list_offset += (len + 1);
         }
 
         ret = 0;
@@ -408,7 +409,7 @@ _posix_get_marker_quota_contributions (posix_xattr_filler_t *filler, char *key)
                 tmp_key = NULL;
         }
 
-        if (strncmp (token, "contri", strlen ("contri")) == 0) {
+        if (strncmp (token, "contri", SLEN ("contri")) == 0) {
                 ret = _posix_get_marker_all_contributions (filler);
         } else {
                 ret = _posix_xattr_get_set_from_backend (filler, key);
@@ -436,6 +437,7 @@ _posix_xattr_get_set (dict_t *xattr_req, char *key, data_t *data,
 {
         posix_xattr_filler_t *filler = xattrargs;
         int       ret      = -1;
+        int       len;
         char     *databuf  = NULL;
         int       _fd      = -1;
         ssize_t  req_size  = 0;
@@ -577,8 +579,9 @@ _posix_xattr_get_set (dict_t *xattr_req, char *key, data_t *data,
                         if (fnmatch (key, xattr, 0) == 0)
                                 ret = _posix_xattr_get_set_from_backend (filler,
                                                                          xattr);
-                        remaining_size -= strlen (xattr) + 1;
-                        list_offset += strlen (xattr) + 1;
+                        len = strlen (xattr);
+                        remaining_size -= (len + 1);
+                        list_offset += (len + 1);
                 }
         }
 out:
@@ -869,6 +872,7 @@ _handle_list_xattr (dict_t *xattr_req, const char *real_path, int fdnum,
         int32_t               list_offset           = 0;
         ssize_t               remaining_size        = 0;
         char                  *key                  = NULL;
+        int                   len;
 
         list_offset = 0;
         remaining_size = filler->list_size;
@@ -892,8 +896,9 @@ _handle_list_xattr (dict_t *xattr_req, const char *real_path, int fdnum,
 
                 (void) _posix_xattr_get_set_from_backend (filler, key);
 next:
-                remaining_size -= strlen (key) + 1;
-                list_offset += strlen (key) + 1;
+                len = strlen (key);
+                remaining_size -= (len + 1);
+                list_offset += (len + 1);
 
         } /* while (remaining_size > 0) */
         return;
@@ -2739,7 +2744,7 @@ posix_resolve_dirgfid_to_path (const uuid_t dirgfid, const char *brick_path,
                         goto out;
                 }
 
-                strncpy (pre_dir_name, result, sizeof(pre_dir_name));
+                snprintf (pre_dir_name, sizeof (pre_dir_name), "%s", result);
 
                 gf_uuid_parse (pgfidstr, tmp_gfid);
                 gf_uuid_copy (pargfid, tmp_gfid);
diff --git a/xlators/storage/posix/src/posix-inode-fd-ops.c b/xlators/storage/posix/src/posix-inode-fd-ops.c
index 64fa5ea7a82..89355757508 100644
--- a/xlators/storage/posix/src/posix-inode-fd-ops.c
+++ b/xlators/storage/posix/src/posix-inode-fd-ops.c
@@ -2228,7 +2228,8 @@ posix_setxattr (call_frame_t *frame, xlator_t *this,
         char          sxattr[4096];
         gf_cs_obj_state  state                   = -1;
         char          remotepath[4096]        = {0};
-        int      i     = 0;
+        int           i                       = 0;
+        int           len;
 
         DECLARE_OLD_FS_ID_VAR;
         SET_FS_ID (frame->root->uid, frame->root->gid);
@@ -2294,10 +2295,10 @@ posix_setxattr (call_frame_t *frame, xlator_t *this,
                         goto unlock;
                 }
 
-                sprintf (sxattr, "%lu", tmp_stbuf.ia_size);
+                len = sprintf (sxattr, "%lu", tmp_stbuf.ia_size);
 
                 ret = sys_lsetxattr (real_path, GF_CS_OBJECT_SIZE,
-                                     sxattr, strlen (sxattr), flags);
+                                     sxattr, len, flags);
                 if (ret) {
                         gf_msg (this->name, GF_LOG_ERROR, 0, 0,
                                 "setxattr failed. key %s err %d",
@@ -2512,7 +2513,7 @@ posix_xattr_get_real_filename (call_frame_t *frame, xlator_t *this, loc_t *loc,
         if (!fd)
                 return -errno;
 
-        fname = key + strlen (GF_XATTR_GET_REAL_FILENAME_KEY);
+        fname = key + SLEN (GF_XATTR_GET_REAL_FILENAME_KEY);
 
         for (;;) {
                 errno = 0;
@@ -2714,6 +2715,7 @@ posix_get_ancestry_non_directory (xlator_t *this, inode_t *leaf_inode,
         char                  key[4096]         = {0,};
         char                  dirpath[PATH_MAX] = {0,};
         char                  pgfidstr[UUID_CANONICAL_FORM_LEN+1] = {0,};
+        int                   len;
 
         priv = this->private;
 
@@ -2783,10 +2785,9 @@ posix_get_ancestry_non_directory (xlator_t *this, inode_t *leaf_inode,
         }
 
         while (remaining_size > 0) {
-                strncpy (key, list + list_offset, sizeof(key)-1);
-                key[sizeof(key)-1] = '\0';
+                snprintf(key, sizeof (key), "%s", list + list_offset);
                 if (strncmp (key, PGFID_XATTR_KEY_PREFIX,
-                             strlen (PGFID_XATTR_KEY_PREFIX)) != 0)
+                             SLEN (PGFID_XATTR_KEY_PREFIX)) != 0)
                         goto next;
 
                 op_ret = sys_lgetxattr (leaf_path, key,
@@ -2802,16 +2803,14 @@ posix_get_ancestry_non_directory (xlator_t *this, inode_t *leaf_inode,
 
                 nlink_samepgfid = ntoh32 (nlink_samepgfid);
 
-                strncpy (pgfidstr, key + strlen(PGFID_XATTR_KEY_PREFIX),
-                         sizeof(pgfidstr)-1);
-                pgfidstr[sizeof(pgfidstr)-1] = '\0';
+                snprintf (pgfidstr, sizeof (pgfidstr), "%s",
+                          key + SLEN (PGFID_XATTR_KEY_PREFIX));
                 gf_uuid_parse (pgfidstr, pgfid);
 
                 handle_size = POSIX_GFID_HANDLE_SIZE(priv->base_path_length);
 
                 /* constructing the absolute real path of parent dir */
-                strncpy (dirpath, priv->base_path, sizeof(dirpath)-1);
-                dirpath[sizeof(dirpath)-1] = '\0';
+                snprintf (dirpath, sizeof (dirpath), "%s", priv->base_path);
                 pathlen = PATH_MAX + 1 - priv->base_path_length;
 
                 op_ret = posix_make_ancestryfromgfid (this,
@@ -2840,8 +2839,9 @@ posix_get_ancestry_non_directory (xlator_t *this, inode_t *leaf_inode,
                 }
 
         next:
-                remaining_size -= strlen (key) + 1;
-                list_offset += strlen (key) + 1;
+                len = strlen (key);
+                remaining_size -= (len + 1);
+                list_offset += (len + 1);
         } /* while (remaining_size > 0) */
 
         op_ret = 0;
@@ -2908,6 +2908,7 @@ posix_getxattr (call_frame_t *frame, xlator_t *this,
         size_t                remaining_size        = 0;
         char                 *host_buf              = NULL;
         char                 *keybuffer             = NULL;
+        int                   keybuff_len;
         char                 *value_buf             = NULL;
         gf_boolean_t          have_val              = _gf_false;
 
@@ -2991,7 +2992,7 @@ posix_getxattr (call_frame_t *frame, xlator_t *this,
 
         if (loc->inode && name &&
             (strncmp (name, GF_XATTR_GET_REAL_FILENAME_KEY,
-                      strlen (GF_XATTR_GET_REAL_FILENAME_KEY)) == 0)) {
+                      SLEN (GF_XATTR_GET_REAL_FILENAME_KEY)) == 0)) {
                 ret = posix_xattr_get_real_filename (frame, this, loc,
                                                      name, dict, xdata);
                 if (ret < 0) {
@@ -3155,7 +3156,7 @@ posix_getxattr (call_frame_t *frame, xlator_t *this,
 
         if (loc->inode && name
              && (strncmp (name, GLUSTERFS_GET_OBJECT_SIGNATURE,
-                          strlen (GLUSTERFS_GET_OBJECT_SIGNATURE)) == 0)) {
+                          SLEN (GLUSTERFS_GET_OBJECT_SIGNATURE)) == 0)) {
                 op_ret = posix_get_objectsignature (real_path, dict);
                 if (op_ret < 0) {
                         op_errno = -op_ret;
@@ -3308,8 +3309,8 @@ posix_getxattr (call_frame_t *frame, xlator_t *this,
         list_offset = 0;
         keybuffer = alloca (XATTR_KEY_BUF_SIZE);
         while (remaining_size > 0) {
-                strncpy (keybuffer, list + list_offset, XATTR_KEY_BUF_SIZE-1);
-                keybuffer[XATTR_KEY_BUF_SIZE-1] = '\0';
+                keybuff_len = snprintf (keybuffer, XATTR_KEY_BUF_SIZE, "%s",
+                          list + list_offset);
 
                 ret = posix_handle_georep_xattrs (frame, keybuffer, NULL,
                                                   _gf_false);
@@ -3372,7 +3373,7 @@ posix_getxattr (call_frame_t *frame, xlator_t *this,
                 /* The protocol expect namespace for now */
                 char *newkey = NULL;
                 gf_add_prefix (XATTR_USER_PREFIX, keybuffer, &newkey);
-                strncpy (keybuffer, newkey, sizeof(keybuffer));
+                keybuff_len = snprintf (keybuffer, sizeof(keybuffer), "%s", newkey);
                 GF_FREE (newkey);
 #endif
                 op_ret = dict_set_dynptr (dict, keybuffer, value, size);
@@ -3387,8 +3388,8 @@ posix_getxattr (call_frame_t *frame, xlator_t *this,
                 }
 
 ignore:
-                remaining_size -= strlen (keybuffer) + 1;
-                list_offset += strlen (keybuffer) + 1;
+                remaining_size -= keybuff_len + 1;
+                list_offset += keybuff_len + 1;
 
         } /* while (remaining_size > 0) */
 
@@ -3429,6 +3430,7 @@ posix_fgetxattr (call_frame_t *frame, xlator_t *this,
         dict_t *          dict           = NULL;
         int               ret            = -1;
         char              key[4096]      = {0,};
+        int               key_len;
         char             *value_buf      = NULL;
         gf_boolean_t      have_val        = _gf_false;
 
@@ -3473,7 +3475,7 @@ posix_fgetxattr (call_frame_t *frame, xlator_t *this,
         }
 
         if (name && strncmp (name, GLUSTERFS_GET_OBJECT_SIGNATURE,
-                      strlen (GLUSTERFS_GET_OBJECT_SIGNATURE)) == 0) {
+                      SLEN (GLUSTERFS_GET_OBJECT_SIGNATURE)) == 0) {
                 op_ret = posix_fdget_objectsignature (_fd, dict);
                 if (op_ret < 0) {
                         gf_msg (this->name, GF_LOG_ERROR, 0, 0,
@@ -3494,14 +3496,14 @@ posix_fgetxattr (call_frame_t *frame, xlator_t *this,
         value_buf = alloca (XATTR_VAL_BUF_SIZE);
 
         if (name) {
-                strncpy (key, name, sizeof(key));
+                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) {
                         char *newkey = NULL;
                         gf_add_prefix (XATTR_USER_PREFIX, key, &newkey);
-                        strncpy (key, newkey, sizeof(key));
+                        key_len = snprintf (key, sizeof (key), "%s", newkey);
                         GF_FREE (newkey);
                 }
 #endif
@@ -3614,7 +3616,7 @@ posix_fgetxattr (call_frame_t *frame, xlator_t *this,
                 if(*(list + list_offset) == '\0')
                         break;
 
-                strncpy (key, list + list_offset, sizeof(key));
+                key_len = snprintf (key, sizeof (key), "%s", list + list_offset);
                 have_val = _gf_false;
                 size = sys_fgetxattr (_fd, key, value_buf,
                                       XATTR_VAL_BUF_SIZE-1);
@@ -3670,8 +3672,8 @@ posix_fgetxattr (call_frame_t *frame, xlator_t *this,
                         GF_FREE (value);
                         goto out;
                 }
-                remaining_size -= strlen (key) + 1;
-                list_offset += strlen (key) + 1;
+                remaining_size -= key_len + 1;
+                list_offset += key_len + 1;
 
         } /* while (remaining_size > 0) */
 
diff --git a/xlators/storage/posix/src/posix.h b/xlators/storage/posix/src/posix.h
index b53afdf4b59..5b013b4b516 100644
--- a/xlators/storage/posix/src/posix.h
+++ b/xlators/storage/posix/src/posix.h
@@ -300,7 +300,7 @@ typedef struct {
                 char gfid_str[64] = {0};                                                   \
                 uuid_utoa_r (gfid, gfid_str);                                              \
                 path_len = strlen (base_path) + 1 +                                        \
-                          strlen (GF_UNLINK_PATH) + 1 +                                    \
+                          SLEN (GF_UNLINK_PATH) + 1 +                                    \
                           strlen (gfid_str) + 1;                                           \
                 unlink_path = alloca (path_len);                                           \
                 if (!unlink_path) {                                                        \