From 9f224b2078c41649355b34b4bbe0d88759a3776c Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sun, 21 Sep 2014 08:29:03 -0700 Subject: [PROPOSED PATCH 2/3] port to C11 memory model We don't know of any problems with the previous code on practical platforms, but it's safer to be portable. Problem reported by Andy Heninger in: http://mm.icann.org/pipermail/tz/2014-September/021599.html * localtime.c (VOLATILE): Remove. All uses removed. (gmtcheck): Don't access gmt_is_set until we have the lock. This may be significantly slower, but it's safer. (localtime_tzset): Likewise. This change isn't significantly slower, though; it's more of a refactoring. * NEWS: Document this. --- NEWS | 3 ++- localtime.c | 12 ++++-------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/NEWS b/NEWS index 56ae660..21e85a7 100644 --- a/NEWS +++ b/NEWS @@ -30,9 +30,10 @@ Unreleased, experimental changes An access to uninitalized data has been fixed. (Thanks to Jörg Richter for reporting the problem.) + When THREAD_SAFE is defined, the code ports to the C11 memory model. A memory leak has been fixed if ALL_STATE and THREAD_SAFE are defined and two threads race to initialize data used by gmtime-like functions. - (Thanks to Andy Heninger for reporting the problem.) + (Thanks to Andy Heninger for reporting the problems.) Changes affecting build procedure diff --git a/localtime.c b/localtime.c index ad1241b..30b5fbc 100644 --- a/localtime.c +++ b/localtime.c @@ -18,12 +18,10 @@ #if THREAD_SAFE # include -# define VOLATILE volatile static pthread_mutex_t locallock = PTHREAD_MUTEX_INITIALIZER; static int lock(void) { return pthread_mutex_lock(&locallock); } static void unlock(void) { pthread_mutex_unlock(&locallock); } #else -# define VOLATILE static int lock(void) { return 0; } static void unlock(void) { } #endif @@ -176,7 +174,7 @@ static struct state gmtmem; #endif /* !defined TZ_STRLEN_MAX */ static char lcl_TZname[TZ_STRLEN_MAX + 1]; -static int VOLATILE lcl_is_set; +static int lcl_is_set; char * tzname[2] = { (char *) wildabbr, @@ -1244,9 +1242,7 @@ tzset(void) static void gmtcheck(void) { - static bool VOLATILE gmt_is_set; - if (gmt_is_set) - return; + static bool gmt_is_set; if (lock() != 0) return; if (! gmt_is_set) { @@ -1400,7 +1396,7 @@ localtime_tzset(time_t const *timep, struct tm *tmp, bool settz, bool setname) errno = err; return NULL; } - if (settz) + if (settz || !lcl_is_set) tzset_unlocked(); tmp = localsub(lclptr, timep, setname, tmp); unlock(); @@ -1416,7 +1412,7 @@ localtime(const time_t *const timep) struct tm * localtime_r(const time_t *const timep, struct tm *tmp) { - return localtime_tzset(timep, tmp, lcl_is_set == 0, false); + return localtime_tzset(timep, tmp, false, false); } /* -- 1.9.3