{cli-cmd-parser|cli-rpc-ops||cli-xml-output}.c: strncpy()->sprintf(), reduce strlen()'s

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 GF_CALLOC() to GF_MALLOC() for the strings.
- move from strlen to sizeof() for const strings.

Compile-tested only!

Change-Id: I3cf49c5401ee100a5db6a4954c3d699ec1814c17
updates: bz#1193929
Signed-off-by: Yaniv Kaul <ykaul@redhat.com>

3 files changed, 31 insertions, 24 deletions

diff --git a/cli/src/cli-cmd-parser.c b/cli/src/cli-cmd-parser.c
index 21d9b7d2eb4..44bef1d82f4 100644
--- a/cli/src/cli-cmd-parser.c
+++ b/cli/src/cli-cmd-parser.c
@@ -407,6 +407,7 @@ cli_validate_volname (const char *volname)
 {
         int32_t            ret                       = -1;
         int32_t            i                         = -1;
+        int                volname_len;
         static const char * const invalid_volnames[] = {
                                       "volume", "type", "subvolumes", "option",
                                       "end-volume", "all", "volume_not_in_ring",
@@ -429,13 +430,14 @@ cli_validate_volname (const char *volname)
         if (strchr (volname, '/'))
                 goto out;
 
-        if (strlen (volname) > GD_VOLUME_NAME_MAX) {
+        volname_len = strlen (volname);
+        if (volname_len > GD_VOLUME_NAME_MAX) {
                 cli_err("Volume name exceeds %d characters.",
                         GD_VOLUME_NAME_MAX);
                 goto out;
         }
 
-        for (i = 0; i < strlen (volname); i++) {
+        for (i = 0; i < volname_len; i++) {
                 if (!isalnum (volname[i]) && (volname[i] != '_') &&
                    (volname[i] != '-')) {
                         cli_err ("Volume name should not contain \"%c\""
@@ -3717,17 +3719,18 @@ extract_hostname_path_from_token (const char *tmp_words, char **hostname,
         char *tmp_host = NULL;
         char *host_name = NULL;
         char *words = NULL;
-
+        int str_len = 0;
         *hostname = NULL;
         *path = NULL;
 
-        words = GF_CALLOC (1, strlen (tmp_words) + 1, gf_common_mt_char);
+        str_len = strlen (tmp_words) + 1;
+        words = GF_MALLOC (str_len, gf_common_mt_char);
         if (!words){
                 ret = -1;
                 goto out;
         }
 
-        strncpy (words, tmp_words, strlen (tmp_words) + 1);
+        snprintf (words, str_len, "%s", tmp_words);
 
         if (validate_brick_name (words)) {
                 cli_err ("Wrong brick type: %s, use <HOSTNAME>:"
@@ -3740,15 +3743,14 @@ extract_hostname_path_from_token (const char *tmp_words, char **hostname,
                 if (ret) {
                         goto out;
                 } else {
-                        *path = GF_CALLOC (1, strlen (delimiter+1) +1,
-                                           gf_common_mt_char);
+                        str_len = strlen (delimiter + 1) + 1;
+                        *path = GF_MALLOC (str_len, gf_common_mt_char);
                         if (!*path) {
                            ret = -1;
                                 goto out;
 
                         }
-                        strncpy (*path, delimiter +1,
-                                 strlen(delimiter + 1) + 1);
+                        snprintf (*path, str_len, "%s", delimiter +1);
                 }
         }
 
@@ -3781,13 +3783,13 @@ extract_hostname_path_from_token (const char *tmp_words, char **hostname,
                 goto out;
         }
 
-        *hostname = GF_CALLOC (1, strlen (host_name) + 1,
-                                       gf_common_mt_char);
+        str_len = strlen (host_name) + 1;
+        *hostname = GF_MALLOC (str_len, gf_common_mt_char);
         if (!*hostname) {
                 ret = -1;
                 goto out;
         }
-        strncpy (*hostname, host_name, strlen (host_name) + 1);
+        snprintf (*hostname, str_len, "%s", host_name);
         ret = 0;
 
 out:
@@ -4185,6 +4187,7 @@ cli_snap_create_desc_parse (dict_t *dict, const char **words,
         int32_t        ret      = -1;
         char          *desc     = NULL;
         int32_t        desc_len = 0;
+        int            len;
 
         desc = GF_CALLOC (MAX_SNAP_DESCRIPTION_LEN + 1, sizeof(char),
                           gf_common_mt_char);
@@ -4194,16 +4197,16 @@ cli_snap_create_desc_parse (dict_t *dict, const char **words,
         }
 
 
-        if (strlen (words[desc_opt_loc]) >= MAX_SNAP_DESCRIPTION_LEN) {
+        len = strlen (words[desc_opt_loc]);
+        if (len >= MAX_SNAP_DESCRIPTION_LEN) {
                 cli_out ("snapshot create: description truncated: "
                          "Description provided is longer than 1024 characters");
                 desc_len = MAX_SNAP_DESCRIPTION_LEN;
         } else {
-                desc_len = strlen (words[desc_opt_loc]);
+                desc_len = len;
         }
 
-        strncpy (desc, words[desc_opt_loc], desc_len);
-        desc[desc_len] = '\0';
+        snprintf (desc, desc_len + 1, "%s", words[desc_opt_loc]);
         /* Calculating the size of the description as given by the user */
 
         ret = dict_set_dynstr (dict, "description", desc);
@@ -5507,6 +5510,7 @@ cli_cmd_validate_volume (char *volname)
 {
         int       i        =  0;
         int       ret      = -1;
+        int       volname_len;
 
 
         if (volname[0] == '-')
@@ -5522,13 +5526,14 @@ cli_cmd_validate_volume (char *volname)
                 return ret;
         }
 
-        if (strlen (volname) > GD_VOLUME_NAME_MAX) {
+        volname_len = strlen (volname);
+        if (volname_len > GD_VOLUME_NAME_MAX) {
                 cli_err ("Volname can not exceed %d characters.",
                         GD_VOLUME_NAME_MAX);
                 return ret;
         }
 
-        for (i = 0; i < strlen (volname); i++)
+        for (i = 0; i < volname_len ; i++)
                 if (!isalnum (volname[i]) && (volname[i] != '_') &&
                     (volname[i] != '-')) {
                         cli_err ("Volume name should not contain \"%c\""
diff --git a/cli/src/cli-rpc-ops.c b/cli/src/cli-rpc-ops.c
index 9812bbdab18..1e95836e4af 100644
--- a/cli/src/cli-rpc-ops.c
+++ b/cli/src/cli-rpc-ops.c
@@ -5936,8 +5936,8 @@ write_contents_to_common_pem_file (dict_t *dict, int output_count)
                                 goto out;
                         }
                         /* Adding the new line character */
-                        bytes_written = sys_write (fd, "\n", strlen("\n"));
-                        if (bytes_written != strlen("\n")) {
+                        bytes_written = sys_write (fd, "\n", 1);
+                        if (bytes_written != 1) {
                                 gf_log ("", GF_LOG_ERROR,
                                         "Failed to add new line char");
                                 ret = -1;
@@ -12027,10 +12027,10 @@ gf_cli_bitrot_cbk (struct rpc_req *req, struct iovec *iov,
                 ret = 0;
                 goto out;
         case GF_BITROT_OPTION_TYPE_SCRUB:
-                if (!strncmp ("pause", scrub_cmd, strlen("pause")))
+                if (!strncmp ("pause", scrub_cmd, sizeof ("pause")))
                         cli_out("volume bitrot: scrubber paused "
                                 "for volume %s", volname);
-                if (!strncmp ("resume", scrub_cmd, strlen("resume")))
+                if (!strncmp ("resume", scrub_cmd, sizeof ("resume")))
                         cli_out("volume bitrot: scrubber resumed "
                                 "for volume %s", volname);
                 ret = 0;
diff --git a/cli/src/cli-xml-output.c b/cli/src/cli-xml-output.c
index 78aa8439e9d..dcb1715e625 100644
--- a/cli/src/cli-xml-output.c
+++ b/cli/src/cli-xml-output.c
@@ -1746,6 +1746,7 @@ cli_xml_output_vol_top_rw_perf (xmlTextWriterPtr writer, dict_t *dict,
         long int   time_usec = 0;
         char       timestr[256] = {0,};
         char       key[1024] = {0,};
+        int        len;
 
         /* <file> */
         ret = xmlTextWriterStartElement (writer, (xmlChar *)"file");
@@ -1781,8 +1782,9 @@ cli_xml_output_vol_top_rw_perf (xmlTextWriterPtr writer, dict_t *dict,
                 goto out;
 
         gf_time_fmt (timestr, sizeof timestr, time_sec, gf_timefmt_FT);
-        snprintf (timestr + strlen (timestr),
-                  sizeof timestr - strlen (timestr),
+        len = strlen (timestr);
+        snprintf (timestr + len,
+                  sizeof (timestr) - len,
                   ".%"GF_PRI_SUSECONDS, time_usec);
         ret = xmlTextWriterWriteFormatElement (writer, (xmlChar *)"time",
                                                "%s", timestr);