bit-rot xlator: strncpy()->sprintf(), reduce strlen()'s

xlators/features/bit-rot/src/bitd/bit-rot-scrub-status.c
xlators/features/bit-rot/src/stub/bit-rot-stub-helpers.c
xlators/features/bit-rot/src/stub/bit-rot-stub.c
xlators/features/bit-rot/src/stub/bit-rot-stub.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(). Ensure sprintf() results do not
truncate.

Also:
- save the result of strlen() and re-use it when possible.
- move from strlen to SLEN or sizeof() for const strings.
- move ret from int32 to int.

Compile-tested only!

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

4 files changed, 29 insertions, 19 deletions

diff --git a/xlators/features/bit-rot/src/bitd/bit-rot-scrub-status.c b/xlators/features/bit-rot/src/bitd/bit-rot-scrub-status.c
index 2f9aaf31a52..210beca7e2f 100644
--- a/xlators/features/bit-rot/src/bitd/bit-rot-scrub-status.c
+++ b/xlators/features/bit-rot/src/bitd/bit-rot-scrub-status.c
@@ -9,6 +9,7 @@
 */
 
 #include <string.h>
+#include <stdio.h>
 
 #include "bit-rot-scrub-status.h"
 
@@ -61,6 +62,9 @@ br_update_scrub_finish_time (br_scrub_stats_t *scrub_stat, char *timestr,
                 return;
 
         lst_size = sizeof (scrub_stat->last_scrub_time);
+        if (strlen (timestr)  >= lst_size)
+                return;
+
         pthread_mutex_lock (&scrub_stat->lock);
         {
                 scrub_stat->scrub_end_tv.tv_sec = tv->tv_sec;
@@ -69,9 +73,8 @@ br_update_scrub_finish_time (br_scrub_stats_t *scrub_stat, char *timestr,
                                  scrub_stat->scrub_end_tv.tv_sec -
                                  scrub_stat->scrub_start_tv.tv_sec;
 
-                strncpy (scrub_stat->last_scrub_time, timestr,
-                         lst_size-1);
-                scrub_stat->last_scrub_time[lst_size-1] = '\0';
+                snprintf (scrub_stat->last_scrub_time, lst_size, "%s",
+                          timestr);
         }
         pthread_mutex_unlock (&scrub_stat->lock);
 }
diff --git a/xlators/features/bit-rot/src/stub/bit-rot-stub-helpers.c b/xlators/features/bit-rot/src/stub/bit-rot-stub-helpers.c
index cc1a6e9a66f..42398bbf2ca 100644
--- a/xlators/features/bit-rot/src/stub/bit-rot-stub-helpers.c
+++ b/xlators/features/bit-rot/src/stub/bit-rot-stub-helpers.c
@@ -257,10 +257,14 @@ br_stub_dir_create (xlator_t *this, br_stub_private_t *priv)
 
         gf_uuid_copy (priv->bad_object_dir_gfid, BR_BAD_OBJ_CONTAINER);
 
-        strncpy (fullpath, priv->stub_basepath, sizeof (fullpath));
+        if (snprintf (fullpath, sizeof (fullpath), "%s",
+		      priv->stub_basepath) >= sizeof (fullpath))
+                goto out;
 
-        snprintf (stub_gfid_path, sizeof (stub_gfid_path), "%s/stub-%s",
-                  priv->stub_basepath, uuid_utoa (priv->bad_object_dir_gfid));
+        if (snprintf (stub_gfid_path, sizeof (stub_gfid_path), "%s/stub-%s",
+                  priv->stub_basepath, uuid_utoa (priv->bad_object_dir_gfid))
+                  >= sizeof (stub_gfid_path))
+                goto out;
 
         ret = br_stub_check_stub_directory (this, fullpath);
         if (ret)
diff --git a/xlators/features/bit-rot/src/stub/bit-rot-stub.c b/xlators/features/bit-rot/src/stub/bit-rot-stub.c
index c4853487e80..f7fdd491809 100644
--- a/xlators/features/bit-rot/src/stub/bit-rot-stub.c
+++ b/xlators/features/bit-rot/src/stub/bit-rot-stub.c
@@ -55,11 +55,11 @@ mem_acct_init (xlator_t *this)
         return ret;
 }
 
-int32_t
+int
 br_stub_bad_object_container_init (xlator_t *this, br_stub_private_t *priv)
 {
         pthread_attr_t  w_attr;
-        int32_t         ret          = -1;
+        int         ret          = -1;
 
         ret = pthread_cond_init(&priv->container.bad_cond, NULL);
         if (ret != 0) {
@@ -117,7 +117,7 @@ out:
 int32_t
 init (xlator_t *this)
 {
-        int32_t ret = 0;
+        int ret = 0;
         char *tmp = NULL;
         struct timeval tv = {0,};
         br_stub_private_t *priv = NULL;
@@ -139,11 +139,14 @@ init (xlator_t *this)
         GF_OPTION_INIT ("bitrot", priv->do_versioning, bool, free_mempool);
 
         GF_OPTION_INIT ("export", tmp, str, free_mempool);
-        strncpy (priv->export, tmp, PATH_MAX-1);
-        priv->export[PATH_MAX-1] = '\0';
 
-        (void) snprintf (priv->stub_basepath, sizeof (priv->stub_basepath),
-                         "%s/%s", priv->export, BR_STUB_QUARANTINE_DIR);
+        if (snprintf (priv->export, PATH_MAX, "%s", tmp) >= PATH_MAX)
+                goto free_mempool;
+
+        if (snprintf (priv->stub_basepath, sizeof (priv->stub_basepath),
+                     "%s/%s", priv->export, BR_STUB_QUARANTINE_DIR) >=
+            sizeof (priv->stub_basepath))
+                goto free_mempool;
 
         (void) gettimeofday (&tv, NULL);
 
@@ -1774,7 +1777,7 @@ br_stub_getxattr (call_frame_t *frame, xlator_t *this,
          */
         if (name
             && (strncmp (name, GLUSTERFS_GET_BR_STUB_INIT_TIME,
-                          strlen (GLUSTERFS_GET_BR_STUB_INIT_TIME)) == 0)
+                          sizeof (GLUSTERFS_GET_BR_STUB_INIT_TIME) - 1) == 0)
             && ((gf_uuid_compare (loc->gfid, rootgfid) == 0)
                 || (gf_uuid_compare (loc->inode->gfid, rootgfid) == 0))) {
                 BR_STUB_RESET_LOCAL_NULL (frame);
@@ -1786,7 +1789,7 @@ br_stub_getxattr (call_frame_t *frame, xlator_t *this,
                 goto wind;
 
         if (name && (strncmp (name, GLUSTERFS_GET_OBJECT_SIGNATURE,
-                              strlen (GLUSTERFS_GET_OBJECT_SIGNATURE)) == 0)) {
+                              sizeof (GLUSTERFS_GET_OBJECT_SIGNATURE) - 1) == 0)) {
                 cookie = (void *) BR_STUB_REQUEST_COOKIE;
 
                 local = br_stub_alloc_local (this);
@@ -1854,7 +1857,7 @@ br_stub_fgetxattr (call_frame_t *frame, xlator_t *this,
          */
         if (name
             && (strncmp (name, GLUSTERFS_GET_BR_STUB_INIT_TIME,
-                         strlen (GLUSTERFS_GET_BR_STUB_INIT_TIME)) == 0)
+                         sizeof (GLUSTERFS_GET_BR_STUB_INIT_TIME) - 1) == 0)
             && (gf_uuid_compare (fd->inode->gfid, rootgfid) == 0)) {
                 BR_STUB_RESET_LOCAL_NULL (frame);
                 br_stub_send_stub_init_time (frame, this);
@@ -1865,7 +1868,7 @@ br_stub_fgetxattr (call_frame_t *frame, xlator_t *this,
                 goto wind;
 
         if (name && (strncmp (name, GLUSTERFS_GET_OBJECT_SIGNATURE,
-                              strlen (GLUSTERFS_GET_OBJECT_SIGNATURE)) == 0)) {
+                              sizeof (GLUSTERFS_GET_OBJECT_SIGNATURE) - 1) == 0)) {
                 cookie = (void *) BR_STUB_REQUEST_COOKIE;
 
                 local = br_stub_alloc_local (this);
diff --git a/xlators/features/bit-rot/src/stub/bit-rot-stub.h b/xlators/features/bit-rot/src/stub/bit-rot-stub.h
index ae4db0fd4f1..e15f1cecbc0 100644
--- a/xlators/features/bit-rot/src/stub/bit-rot-stub.h
+++ b/xlators/features/bit-rot/src/stub/bit-rot-stub.h
@@ -355,9 +355,9 @@ br_stub_is_internal_xattr (const char *name)
 {
         if (name
             && ((strncmp (name, BITROT_CURRENT_VERSION_KEY,
-                          strlen (BITROT_CURRENT_VERSION_KEY)) == 0)
+                          SLEN (BITROT_CURRENT_VERSION_KEY)) == 0)
                 || (strncmp (name, BITROT_SIGNING_VERSION_KEY,
-                             strlen (BITROT_SIGNING_VERSION_KEY)) == 0)))
+                             SLEN (BITROT_SIGNING_VERSION_KEY)) == 0)))
                 return 1;
         return 0;
 }