localtime and ctime are not MT-SAFE

There are a number of nit-level issues throughout the source with
the use of localtime and ctime. While they apparently aren't causing
too many problems, apart from the one in bz 828058, they ought to be
fixed.  Among the "real" problems that are fixed in this patch:
 1) general localtime and ctime not MT-SAFE. There's a non-zero chance
    that another thread calling localtime (or ctime) will over-write
    the static data about to be used in another thread
 2) localtime(& <64-bit-type>) or ctime(& <64-bit-type>) generally
    not a problem on 64-bit or little-endian 32-bit. But even though
    we probably have zero users on big-ending 32-bit platforms, it's
    still incorrect.
 3) multiple nested calls passed as params. Last one wins, i.e. over-
    writes result of prior calls.
 4) Inconsistent error handling. Most of these calls are for logging,
    tracing, or dumping. I submit that if an error somehow occurs in
    the call to localtime or ctime, the log/trace/dump still should
    still occur.
 5) Appliances should all have their clocks set to UTC, and all log
    entries, traces, and dumps should use GMT.
 6) fix strtok(), change to strtok_r()

Other things this patch fixes/changes (that aren't bugs per se):
 1) Change "%Y-%m-%d %H:%M:%S" and similar to their equivalent shorthand,
    e.g. "%F %T"
 2) change sizeof(timestr) to sizeof timestr. sizeof is an operator,
    not a function. You don't use i +(32), why use sizeof(<var>).
    (And yes, you do use parens with sizeof(<type>).)
 3) change 'char timestr[256]' to 'char timestr[32]' where appropriate.
    Per-thread stack is limited. Time strings are never longer than ~20
    characters, so why waste 220+ bytes on the stack?

Things this patch doesn't fix:
 1) hodgepodge of %Y-%m-%d %H:%M:%S versus %Y/%m/%d-%H%M%S and other
    variations. It's not clear to me whether this ever matters, not to
    mention 3rd party log filtering tools may already rely on a
    particular format. Still it would be nice to have a single manifest
    constant and have every call to localtime/strftime consistently use
    the same format.

Change-Id: I827cad7bf53e57b69c0173f67abe72884249c1a9
BUG: 832173
Signed-off-by: Kaleb S. KEITHLEY <kkeithle@redhat.com>
Reviewed-on: http://review.gluster.com/3568
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Anand Avati <avati@redhat.com>

3 files changed, 31 insertions, 39 deletions

diff --git a/xlators/features/locks/src/posix.c b/xlators/features/locks/src/posix.c
index 90caadb0cc8..3da40ffacac 100644
--- a/xlators/features/locks/src/posix.c
+++ b/xlators/features/locks/src/posix.c
@@ -1739,7 +1739,9 @@ pl_dump_lock (char *str, int size, struct gf_flock *flock,
               gf_lkowner_t *owner, void *trans, time_t *granted_time,
               time_t *blkd_time, gf_boolean_t active)
 {
-        char *type_str = NULL;
+        char  *type_str    = NULL;
+        char   granted[32] = {0,};
+        char   blocked[32] = {0,};
 
         switch (flock->l_type) {
         case F_RDLCK:
@@ -1763,16 +1765,17 @@ pl_dump_lock (char *str, int size, struct gf_flock *flock,
                                   (unsigned long long) flock->l_start,
                                   (unsigned long long) flock->l_len,
                                   (unsigned long long) flock->l_pid,
-                                  lkowner_utoa (owner),
-                                  trans, ctime (granted_time));
+                                  lkowner_utoa (owner), trans,
+                                  ctime_r (granted_time, granted));
                 } else {
                         snprintf (str, size, RANGE_BLKD_GRNTD_FMT,
                                   type_str, flock->l_whence,
                                   (unsigned long long) flock->l_start,
                                   (unsigned long long) flock->l_len,
                                   (unsigned long long) flock->l_pid,
-                                  lkowner_utoa (owner),
-                                  trans, ctime (blkd_time), ctime (granted_time));
+                                  lkowner_utoa (owner), trans,
+                                  ctime_r (blkd_time, blocked),
+                                  ctime_r (granted_time, granted));
                 }
         }
         else {
@@ -1781,8 +1784,8 @@ pl_dump_lock (char *str, int size, struct gf_flock *flock,
                           (unsigned long long) flock->l_start,
                           (unsigned long long) flock->l_len,
                           (unsigned long long) flock->l_pid,
-                          lkowner_utoa (owner),
-                          trans, ctime (blkd_time));
+                          lkowner_utoa (owner), trans,
+                          ctime_r (blkd_time, blocked));
         }
 
 }
@@ -1792,8 +1795,10 @@ __dump_entrylks (pl_inode_t *pl_inode)
 {
         pl_dom_list_t   *dom  = NULL;
         pl_entry_lock_t *lock = NULL;
-        int             count = 0;
-        char            key[GF_DUMP_MAX_BUF_LEN];
+        char             blocked[32] = {0,};
+        char             granted[32] = {0,};
+        int              count = 0;
+        char             key[GF_DUMP_MAX_BUF_LEN] = {0,};
 
         char tmp[256];
 
@@ -1817,15 +1822,15 @@ __dump_entrylks (pl_inode_t *pl_inode)
                                           "ENTRYLK_WRLCK", lock->basename,
                                           (unsigned long long) lock->client_pid,
                                           lkowner_utoa (&lock->owner), lock->trans,
-                                          ctime (&lock->granted_time.tv_sec));
+                                          ctime_r (&lock->granted_time.tv_sec, granted));
                         } else {
                                 snprintf (tmp, 256, ENTRY_BLKD_GRNTD_FMT,
                                           lock->type == ENTRYLK_RDLCK ? "ENTRYLK_RDLCK" :
                                           "ENTRYLK_WRLCK", lock->basename,
                                           (unsigned long long) lock->client_pid,
                                           lkowner_utoa (&lock->owner), lock->trans,
-                                          ctime (&lock->blkd_time.tv_sec),
-                                          ctime (&lock->granted_time.tv_sec));
+                                          ctime_r (&lock->blkd_time.tv_sec, blocked),
+                                          ctime_r (&lock->granted_time.tv_sec, granted));
                         }
 
                         gf_proc_dump_write(key, tmp);
@@ -1843,7 +1848,7 @@ __dump_entrylks (pl_inode_t *pl_inode)
                                   "ENTRYLK_WRLCK", lock->basename,
                                   (unsigned long long) lock->client_pid,
                                   lkowner_utoa (&lock->owner), lock->trans,
-                                  ctime (&lock->blkd_time.tv_sec));
+                                  ctime_r (&lock->blkd_time.tv_sec, blocked));
 
                         gf_proc_dump_write(key, tmp);
 
diff --git a/xlators/features/marker/utils/src/gsyncd.c b/xlators/features/marker/utils/src/gsyncd.c
index 9c598ce668e..7da2c983cff 100644
--- a/xlators/features/marker/utils/src/gsyncd.c
+++ b/xlators/features/marker/utils/src/gsyncd.c
@@ -61,6 +61,7 @@ static int
 str2argv (char *str, char ***argv)
 {
         char *p         = NULL;
+        char *savetok   = NULL;
         int argc        = 0;
         size_t argv_len = 32;
         int ret         = 0;
@@ -74,7 +75,7 @@ str2argv (char *str, char ***argv)
         if (!*argv)
                 goto error;
 
-        while ((p = strtok (str, " "))) {
+        while ((p = strtok_r (str, " ", &savetok))) {
                 str = NULL;
 
                 argc++;
diff --git a/xlators/features/trash/src/trash.c b/xlators/features/trash/src/trash.c
index 15d9a429f67..92ea19a55f3 100644
--- a/xlators/features/trash/src/trash.c
+++ b/xlators/features/trash/src/trash.c
@@ -504,9 +504,7 @@ trash_rename (call_frame_t *frame, xlator_t *this, loc_t *oldloc,
         trash_elim_pattern_t  *trav  = NULL;
         trash_private_t *priv = NULL;
         trash_local_t   *local = NULL;
-        struct tm       *tm = NULL;
-        char             timestr[256] = {0,};
-        time_t           utime = 0;
+        char             timestr[64] = {0,};
         int32_t          match = 0;
 
         priv = this->private;
@@ -553,9 +551,8 @@ trash_rename (call_frame_t *frame, xlator_t *this, loc_t *oldloc,
         {
                 /* append timestamp to file name */
                 /* TODO: can we make it optional? */
-                utime = time (NULL);
-                tm    = localtime (&utime);
-                strftime (timestr, 256, ".%Y-%m-%d-%H%M%S", tm);
+                gf_time_ftm (timestr, sizeof timestr, time (NULL),
+                             gf_timefmt_F_HMS);
                 strcat (local->newpath, timestr);
         }
 
@@ -574,9 +571,7 @@ trash_unlink (call_frame_t *frame, xlator_t *this, loc_t *loc)
         trash_elim_pattern_t  *trav  = NULL;
         trash_private_t *priv = NULL;
         trash_local_t   *local = NULL;
-        struct tm       *tm = NULL;
-        char             timestr[256] = {0,};
-        time_t           utime = 0;
+        char             timestr[64] = {0,};
         int32_t          match = 0;
 
         priv = this->private;
@@ -625,9 +620,8 @@ trash_unlink (call_frame_t *frame, xlator_t *this, loc_t *loc)
         {
                 /* append timestamp to file name */
                 /* TODO: can we make it optional? */
-                utime = time (NULL);
-                tm    = localtime (&utime);
-                strftime (timestr, 256, ".%Y-%m-%d-%H%M%S", tm);
+                gf_time_fmt (timestr, sizeof timestr, time (NULL),
+                             gf_timefmt_F_HMS);
                 strcat (local->newpath, timestr);
         }
 
@@ -943,10 +937,8 @@ trash_truncate_stat_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
 {
         trash_private_t     *priv  = NULL;
         trash_local_t       *local = NULL;
-        struct tm           *tm = NULL;
-        char                 timestr[256] = {0,};
+        char                 timestr[64] = {0,};
         char                 loc_newname[PATH_MAX] = {0,};
-        time_t               utime = 0;
         int32_t              flags = 0;
 
         priv = this->private;
@@ -978,9 +970,8 @@ trash_truncate_stat_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
         strcat (local->newpath, local->loc.path);
 
         {
-                utime = time (NULL);
-                tm    = localtime (&utime);
-                strftime (timestr, 256, ".%Y-%m-%d-%H%M%S", tm);
+                gf_time_fmt (timestr, sizeof timestr, time (NULL),
+                             gf_timefmt_F_HMS);
                 strcat (local->newpath, timestr);
         }
         strcpy (loc_newname,local->loc.name);
@@ -1346,11 +1337,9 @@ trash_ftruncate (call_frame_t *frame, xlator_t *this, fd_t *fd, off_t offset)
         trash_private_t       *priv = NULL;
         trash_local_t         *local = NULL;
         dentry_t              *dir_entry = NULL;
-        struct tm             *tm = NULL;
         char                  *pathbuf = NULL;
         inode_t               *newinode = NULL;
-        time_t                 utime = 0;
-        char                   timestr[256];
+        char                   timestr[64];
         int32_t                retval = 0;
         int32_t                match = 0;
 
@@ -1389,10 +1378,7 @@ trash_ftruncate (call_frame_t *frame, xlator_t *this, fd_t *fd, off_t offset)
                 return 0;
         }
 
-        utime = time (NULL);
-        tm    = localtime (&utime);
-        strftime (timestr, 256, ".%Y-%m-%d-%H%M%S", tm);
-
+        gf_time_fmt (timestr, sizeof timestr, time (NULL), gf_timefmt_F_HMS);
         strcpy (local->newpath, priv->trash_dir);
         strcat (local->newpath, pathbuf);
         strcat (local->newpath, timestr);