summaryrefslogtreecommitdiffstats
path: root/glusterfsd
diff options
context:
space:
mode:
authorKaleb S. KEITHLEY <kkeithle@redhat.com>2012-06-22 11:25:32 -0400
committerAnand Avati <avati@redhat.com>2013-02-08 12:04:42 -0800
commit988460a9f597a489f22d39cd259fdb17fe2e5de6 (patch)
tree1650bdd39d6f9cdb96d069203afe46852fd4e680 /glusterfsd
parent32fff8967f10efa391815c06093086a9ee276762 (diff)
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. BUG: 832173 Change-Id: Iee9719db4576eacc6c75694d9107954d0912cba8 Signed-off-by: Kaleb S. KEITHLEY <kkeithle@redhat.com> Reviewed-on: http://review.gluster.org/3613 Reviewed-by: Anand Avati <avati@redhat.com> Tested-by: Anand Avati <avati@redhat.com>
Diffstat (limited to 'glusterfsd')
-rw-r--r--glusterfsd/src/glusterfsd.c30
1 files changed, 13 insertions, 17 deletions
diff --git a/glusterfsd/src/glusterfsd.c b/glusterfsd/src/glusterfsd.c
index 0e9ff2ccc7f..51f0a30c9a9 100644
--- a/glusterfsd/src/glusterfsd.c
+++ b/glusterfsd/src/glusterfsd.c
@@ -917,7 +917,6 @@ generate_uuid ()
char tmp_str[1024] = {0,};
char hostname[256] = {0,};
struct timeval tv = {0,};
- struct tm now = {0, };
char now_str[32];
if (gettimeofday (&tv, NULL) == -1) {
@@ -926,15 +925,14 @@ generate_uuid ()
strerror (errno));
}
- if (gethostname (hostname, 256) == -1) {
+ if (gethostname (hostname, sizeof hostname) == -1) {
gf_log ("glusterfsd", GF_LOG_ERROR,
"gethostname: failed %s",
strerror (errno));
}
- localtime_r (&tv.tv_sec, &now);
- strftime (now_str, 32, "%Y/%m/%d-%H:%M:%S", &now);
- snprintf (tmp_str, 1024, "%s-%d-%s:%" GF_PRI_SUSECONDS,
+ gf_time_fmt (now_str, sizeof now_str, tv.tv_sec, gf_timefmt_Ymd_T);
+ snprintf (tmp_str, sizeof tmp_str, "%s-%d-%s:%" GF_PRI_SUSECONDS,
hostname, getpid(), now_str, tv.tv_usec);
return gf_strdup (tmp_str);
@@ -1176,16 +1174,14 @@ gf_check_and_set_mem_acct (int argc, char *argv[], glusterfs_ctx_t *ctx)
int
parse_cmdline (int argc, char *argv[], glusterfs_ctx_t *ctx)
{
- int process_mode = 0;
- int ret = 0;
- struct stat stbuf = {0, };
- struct tm *tm = NULL;
- time_t utime;
- char timestr[256];
- char tmp_logfile[1024] = { 0 };
- char *tmp_logfile_dyn = NULL;
- char *tmp_logfilebase = NULL;
- cmd_args_t *cmd_args = NULL;
+ int process_mode = 0;
+ int ret = 0;
+ struct stat stbuf = {0, };
+ char timestr[32];
+ char tmp_logfile[1024] = { 0 };
+ char *tmp_logfile_dyn = NULL;
+ char *tmp_logfilebase = NULL;
+ cmd_args_t *cmd_args = NULL;
cmd_args = &ctx->cmd_args;
@@ -1242,8 +1238,8 @@ parse_cmdline (int argc, char *argv[], glusterfs_ctx_t *ctx)
(S_ISREG (stbuf.st_mode) || S_ISLNK (stbuf.st_mode))) ||
(ret == -1)) {
/* Have separate logfile per run */
- tm = localtime (&utime);
- strftime (timestr, 256, "%Y%m%d.%H%M%S", tm);
+ gf_time_fmt (timestr, sizeof timestr, time (NULL),
+ gf_timefmt_FT);
sprintf (tmp_logfile, "%s.%s.%d",
cmd_args->log_file, timestr, getpid ());