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

xlators/features/changetimerecorder/src/changetimerecorder.c
xlators/features/changetimerecorder/src/ctr-helper.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(). Check for output truncation
where applicable.

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

2 files changed, 19 insertions, 12 deletions

diff --git a/xlators/features/changetimerecorder/src/changetimerecorder.c b/xlators/features/changetimerecorder/src/changetimerecorder.c
index 3f39c16ec5f..bc87518c0c0 100644
--- a/xlators/features/changetimerecorder/src/changetimerecorder.c
+++ b/xlators/features/changetimerecorder/src/changetimerecorder.c
@@ -128,10 +128,15 @@ ctr_lookup_wind(call_frame_t                    *frame,
                 /* Copy hard link info*/
                 gf_uuid_copy (CTR_DB_REC(ctr_local).pargfid,
                         *((NEW_LINK_CX(ctr_inode_cx))->pargfid));
-                strncpy (CTR_DB_REC(ctr_local).file_name,
-                         NEW_LINK_CX(ctr_inode_cx)->basename,
-                         sizeof(CTR_DB_REC(ctr_local).file_name));
-
+                if (snprintf (CTR_DB_REC(ctr_local).file_name,
+                              sizeof (CTR_DB_REC(ctr_local).file_name), "%s",
+                              NEW_LINK_CX(ctr_inode_cx)->basename) >=
+                    sizeof (CTR_DB_REC(ctr_local).file_name)) {
+                        gf_msg (this->name, GF_LOG_ERROR, 0,
+                                CTR_MSG_CREATE_CTR_LOCAL_ERROR_WIND,
+                                "WIND: Error copying filename of ctr local");
+                        goto out;
+                }
                 /* Since we are in lookup we can ignore errors while
                  * Inserting in the DB, because there may be many
                  * to write to the DB attempts for healing.
@@ -1878,7 +1883,7 @@ ctr_ipc_helper (xlator_t *this, dict_t *in_dict,
 
         /*if its a db clear operation */
         if (strncmp (ctr_ipc_ops, GFDB_IPC_CTR_CLEAR_OPS,
-                        strlen (GFDB_IPC_CTR_CLEAR_OPS)) == 0) {
+                        SLEN (GFDB_IPC_CTR_CLEAR_OPS)) == 0) {
 
                 ret = clear_files_heat (priv->_db_conn);
                 if (ret)
@@ -1886,7 +1891,7 @@ ctr_ipc_helper (xlator_t *this, dict_t *in_dict,
 
         } /* if its a query operation, in  which case its query + clear db*/
         else if (strncmp (ctr_ipc_ops, GFDB_IPC_CTR_QUERY_OPS,
-                                strlen (GFDB_IPC_CTR_QUERY_OPS)) == 0) {
+                                SLEN (GFDB_IPC_CTR_QUERY_OPS)) == 0) {
 
                 ret = dict_get_str (in_dict, GFDB_IPC_CTR_GET_QFILE_PATH,
                                                                 &query_file);
@@ -1917,7 +1922,7 @@ ctr_ipc_helper (xlator_t *this, dict_t *in_dict,
 
         } /* if its a query for db version */
         else if (strncmp (ctr_ipc_ops, GFDB_IPC_CTR_GET_DB_VERSION_OPS,
-                        strlen (GFDB_IPC_CTR_GET_DB_VERSION_OPS)) == 0) {
+                        SLEN (GFDB_IPC_CTR_GET_DB_VERSION_OPS)) == 0) {
 
                 ret = get_db_version (priv->_db_conn, &db_version);
                 if (ret == -1 || !db_version) {
@@ -1932,7 +1937,7 @@ ctr_ipc_helper (xlator_t *this, dict_t *in_dict,
 
         } /* if its a query for a db setting */
         else if (strncmp (ctr_ipc_ops, GFDB_IPC_CTR_GET_DB_PARAM_OPS,
-                                strlen (GFDB_IPC_CTR_GET_DB_PARAM_OPS)) == 0) {
+                                SLEN (GFDB_IPC_CTR_GET_DB_PARAM_OPS)) == 0) {
 
                 ret = dict_get_str (in_dict, GFDB_IPC_CTR_GET_DB_KEY,
                                 &db_param_key);
@@ -1952,7 +1957,7 @@ ctr_ipc_helper (xlator_t *this, dict_t *in_dict,
                                         db_param, ret, error);
         } /* if its an attempt to compact the database */
         else if (strncmp (ctr_ipc_ops, GFDB_IPC_CTR_SET_COMPACT_PRAGMA,
-                          strlen (GFDB_IPC_CTR_SET_COMPACT_PRAGMA)) == 0) {
+                          SLEN (GFDB_IPC_CTR_SET_COMPACT_PRAGMA)) == 0) {
 
                 ret = pthread_mutex_lock (&priv->compact_lock);
                 if (ret) {
diff --git a/xlators/features/changetimerecorder/src/ctr-helper.h b/xlators/features/changetimerecorder/src/ctr-helper.h
index f4506115056..f821201d3a5 100644
--- a/xlators/features/changetimerecorder/src/ctr-helper.h
+++ b/xlators/features/changetimerecorder/src/ctr-helper.h
@@ -641,15 +641,17 @@ ctr_delete_hard_link_from_db (xlator_t               *this,
         /* Set gfdb_db_record to 0 */
         memset (&gfdb_db_record, 0, sizeof(gfdb_db_record));
 
+        /* Copy basename */
+        if (snprintf (gfdb_db_record.file_name, GF_NAME_MAX, "%s",
+	              basename) >= GF_NAME_MAX)
+                goto out;
+
         /* Copy gfid into db record */
         gf_uuid_copy (gfdb_db_record.gfid, gfid);
 
         /* Copy pargid into db record */
         gf_uuid_copy (gfdb_db_record.pargfid, pargfid);
 
-        /* Copy basename */
-        strncpy (gfdb_db_record.file_name, basename, GF_NAME_MAX - 1);
-
         gfdb_db_record.gfdb_fop_path = fop_path;
         gfdb_db_record.gfdb_fop_type = fop_type;