[PROPOSED PATCH] Make the library thread-safe if THREAD_SAFE is defined.
* localtime.c [THREAD_SAFE]: Include pthread.h. (VOLATILE): New macro. (locallock) [THREAD_SAFE]: New static var. (lock, unlock): New functions. (lcl_is_set, gmt_is_set): Now VOLATILE. (tzsetwall): Move cleaned-up guts to new function tzsetwall_unlocked, for which this is now merely a locking wrapper. (tzset): Similarly, for new function tzset_unlocked. (localtime_tzset): New function, which does proper locking. (localtime, localtime_r): Use it. (gmtsub): Do not worry about initializing gmtptr, as that's now the caller's responsibility. (gmtime): Reimplement in terms of gmtime_r. (timegm): Reimplement in terms of timeoff. (gmtime_r, offtime, mktime, timeoff, time2posix, posix2time): Lock at start and unlock at end. * Makefile, NEWS: Document this. --- Makefile | 3 + NEWS | 4 + localtime.c | 249 ++++++++++++++++++++++++++++++++++++++++-------------------- 3 files changed, 173 insertions(+), 83 deletions(-) diff --git a/Makefile b/Makefile index 5bbd7e7..3d24c2a 100644 --- a/Makefile +++ b/Makefile @@ -123,6 +123,9 @@ LDLIBS= # -DNO_RUN_TIME_WARNINGS_ABOUT_YEAR_2000_PROBLEMS_THANK_YOU=1 # if you do not want run time warnings about formats that may cause # year 2000 grief +# -DTHREAD_SAFE=1 to make localtime.c thread-safe, as POSIX requires; +# not needed by the main-program tz code, which is single-threaded. +# Append other compiler flags as needed, e.g., -pthread on GNU/Linux. # -Dtime_tz=\"T\" to use T as the time_t type, rather than the system time_t # -DTZ_DOMAIN=\"foo\" to use "foo" for gettext domain name; default is "tz" # -DTZ_DOMAINDIR=\"/path\" to use "/path" for gettext directory; diff --git a/NEWS b/NEWS index 28974c9..1a78041 100644 --- a/NEWS +++ b/NEWS @@ -36,6 +36,10 @@ Unreleased, experimental changes Changes affecting code + The tz library is now thread-safe if compiled with THREAD_SAFE defined. + Although not needed for tz's own applications, which are single-threaded, + this supports POSIX better if the tz library is used in multithreaded apps. + tzselect -c now uses a hybrid distance measure that works better in Africa. (Thanks to Alan Barrett for noting the problem.) diff --git a/localtime.c b/localtime.c index 551b5a2..f499583 100644 --- a/localtime.c +++ b/localtime.c @@ -16,6 +16,18 @@ #include "tzfile.h" #include "fcntl.h" +#if THREAD_SAFE +# include <pthread.h> +# 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 + #ifndef TZ_ABBR_MAX_LEN #define TZ_ABBR_MAX_LEN 16 #endif /* !defined TZ_ABBR_MAX_LEN */ @@ -154,8 +166,8 @@ static struct state gmtmem; #endif /* !defined TZ_STRLEN_MAX */ static char lcl_TZname[TZ_STRLEN_MAX + 1]; -static int lcl_is_set; -static int gmt_is_set; +static int VOLATILE lcl_is_set; +static int VOLATILE gmt_is_set; char * tzname[2] = { (char *) wildabbr, @@ -1139,6 +1151,21 @@ gmtload(struct state *const sp) (void) tzparse(gmt, sp, TRUE); } +static void +tzsetwall_unlocked(void) +{ + if (lcl_is_set < 0) + return; +#ifdef ALL_STATE + if (! lclptr) + lclptr = malloc(sizeof *lclptr); +#endif + if (lclptr && tzload(NULL, lclptr, TRUE) != 0) + gmtload(lclptr); + settzname(); + lcl_is_set = -1; +} + #ifndef STD_INSPIRED /* ** A non-static declaration of tzsetwall in a system header file @@ -1149,65 +1176,71 @@ static void tzsetwall(void) { - if (lcl_is_set < 0) - return; - lcl_is_set = -1; + if (lock() != 0) + return; + tzsetwall_unlocked(); + unlock(); +} +static void +tzset_unlocked(void) +{ + int shortname; + register char const *name = getenv("TZ"); + + if (!name) { + tzsetwall_unlocked(); + return; + } + shortname = strlen(name) < sizeof lcl_TZname; + if (0 < lcl_is_set && strcmp(lcl_TZname, name) == 0) + return; + if (shortname) + strcpy(lcl_TZname, name); #ifdef ALL_STATE - if (lclptr == NULL) { - lclptr = malloc(sizeof *lclptr); - if (lclptr == NULL) { - settzname(); /* all we can do */ - return; - } - } + if (! lclptr) + lclptr = malloc(sizeof *lclptr); #endif /* defined ALL_STATE */ - if (tzload(NULL, lclptr, TRUE) != 0) - gmtload(lclptr); - settzname(); + if (lclptr) { + if (*name == '\0') { + /* + ** User wants it fast rather than right. + */ + lclptr->leapcnt = 0; /* so, we're off a little */ + lclptr->timecnt = 0; + lclptr->typecnt = 0; + lclptr->ttis[0].tt_isdst = 0; + lclptr->ttis[0].tt_gmtoff = 0; + lclptr->ttis[0].tt_abbrind = 0; + strcpy(lclptr->chars, gmt); + } else if (tzload(name, lclptr, TRUE) != 0) + if (name[0] == ':' || tzparse(name, lclptr, FALSE) != 0) + gmtload(lclptr); + } + settzname(); + lcl_is_set = shortname; } void tzset(void) { - register const char * name; - - name = getenv("TZ"); - if (name == NULL) { - tzsetwall(); - return; - } - - if (lcl_is_set > 0 && strcmp(lcl_TZname, name) == 0) - return; - lcl_is_set = strlen(name) < sizeof lcl_TZname; - if (lcl_is_set) - (void) strcpy(lcl_TZname, name); + if (lock() != 0) + return; + tzset_unlocked(); + unlock(); +} +static void +gmtcheck(void) +{ + if (gmt_is_set) + return; #ifdef ALL_STATE - if (lclptr == NULL) { - lclptr = malloc(sizeof *lclptr); - if (lclptr == NULL) { - settzname(); /* all we can do */ - return; - } - } -#endif /* defined ALL_STATE */ - if (*name == '\0') { - /* - ** User wants it fast rather than right. - */ - lclptr->leapcnt = 0; /* so, we're off a little */ - lclptr->timecnt = 0; - lclptr->typecnt = 0; - lclptr->ttis[0].tt_isdst = 0; - lclptr->ttis[0].tt_gmtoff = 0; - lclptr->ttis[0].tt_abbrind = 0; - (void) strcpy(lclptr->chars, gmt); - } else if (tzload(name, lclptr, TRUE) != 0) - if (name[0] == ':' || tzparse(name, lclptr, FALSE) != 0) - (void) gmtload(lclptr); - settzname(); + gmtptr = malloc(sizeof *gmtptr); +#endif + if (gmtptr) + gmtload(gmtptr); + gmt_is_set = TRUE; } /* @@ -1296,11 +1329,25 @@ localsub(const time_t *const timep, const int_fast32_t offset, return result; } +static struct tm * +localtime_tzset(time_t const *timep, struct tm *tmp, int skip_tzset) +{ + int err = lock(); + if (err) { + errno = err; + return NULL; + } + if (!skip_tzset) + tzset_unlocked(); + tmp = localsub(timep, 0, tmp); + unlock(); + return tmp; +} + struct tm * localtime(const time_t *const timep) { - tzset(); - return localsub(timep, 0L, &tm); + return localtime_tzset(timep, &tm, 0); } /* @@ -1310,7 +1357,7 @@ localtime(const time_t *const timep) struct tm * localtime_r(const time_t *const timep, struct tm *tmp) { - return localsub(timep, 0L, tmp); + return localtime_tzset(timep, tmp, lcl_is_set); } /* @@ -1323,16 +1370,6 @@ gmtsub(const time_t *const timep, const int_fast32_t offset, { register struct tm * result; - if (!gmt_is_set) { -#ifdef ALL_STATE - gmtptr = malloc(sizeof *gmtptr); - gmt_is_set = gmtptr != NULL; -#else - gmt_is_set = TRUE; -#endif /* defined ALL_STATE */ - if (gmt_is_set) - gmtload(gmtptr); - } result = timesub(timep, offset, gmtptr, tmp); #ifdef TM_ZONE /* @@ -1348,7 +1385,7 @@ gmtsub(const time_t *const timep, const int_fast32_t offset, struct tm * gmtime(const time_t *const timep) { - return gmtsub(timep, 0L, &tm); + return gmtime_r(timep, &tm); } /* @@ -1358,7 +1395,15 @@ gmtime(const time_t *const timep) struct tm * gmtime_r(const time_t *const timep, struct tm *tmp) { - return gmtsub(timep, 0L, tmp); + int err = lock(); + if (err) { + errno = err; + return NULL; + } + gmtcheck(); + tmp = gmtsub(timep, 0, tmp); + unlock(); + return tmp; } #ifdef STD_INSPIRED @@ -1366,7 +1411,16 @@ gmtime_r(const time_t *const timep, struct tm *tmp) struct tm * offtime(const time_t *const timep, const long offset) { - return gmtsub(timep, offset, &tm); + struct tm *tmp; + int err = lock(); + if (err) { + errno = err; + return NULL; + } + gmtcheck(); + tmp = gmtsub(timep, offset, &tm); + unlock(); + return tmp; } #endif /* defined STD_INSPIRED */ @@ -1901,8 +1955,16 @@ time1(struct tm *const tmp, time_t mktime(struct tm *const tmp) { - tzset(); - return time1(tmp, localsub, 0L); + time_t t; + int err = lock(); + if (err) { + errno = err; + return -1; + } + tzset_unlocked(); + t = time1(tmp, localsub, 0); + unlock(); + return t; } #ifdef STD_INSPIRED @@ -1918,17 +1980,25 @@ timelocal(struct tm *const tmp) time_t timegm(struct tm *const tmp) { - if (tmp != NULL) - tmp->tm_isdst = 0; - return time1(tmp, gmtsub, 0L); + return timeoff(tmp, 0); } time_t timeoff(struct tm *const tmp, const long offset) { - if (tmp != NULL) - tmp->tm_isdst = 0; - return time1(tmp, gmtsub, offset); + time_t t; + int err; + if (tmp) + tmp->tm_isdst = 0; + err = lock(); + if (err) { + errno = err; + return -1; + } + gmtcheck(); + t = time1(tmp, gmtsub, offset); + unlock(); + return t; } #endif /* defined STD_INSPIRED */ @@ -1986,8 +2056,17 @@ leapcorr(time_t *timep) time_t time2posix(time_t t) { - tzset(); - return t - leapcorr(&t); + time_t p; + int err = lock(); + if (err) { + errno = err; + return -1; + } + if (!lcl_is_set) + tzset_unlocked(); + p = t - leapcorr(&t); + unlock(); + return p; } time_t @@ -1995,8 +2074,13 @@ posix2time(time_t t) { time_t x; time_t y; - - tzset(); + int err = lock(); + if (err) { + errno = err; + return -1; + } + if (!lcl_is_set) + tzset_unlocked(); /* ** For a positive leap second hit, the result ** is not unique. For a negative leap second @@ -2010,16 +2094,15 @@ posix2time(time_t t) x++; y = x - leapcorr(&x); } while (y < t); - if (t != y) - return x - 1; + x -= y != t; } else if (y > t) { do { --x; y = x - leapcorr(&x); } while (y > t); - if (t != y) - return x + 1; + x += y != t; } + unlock(); return x; } -- 1.9.1
Am 17.08.2014 22:36, schrieb Paul Eggert:
* localtime.c [THREAD_SAFE]: Include pthread.h. (VOLATILE): New macro. (locallock) [THREAD_SAFE]: New static var. (lock, unlock): New functions. (lcl_is_set, gmt_is_set): Now VOLATILE. (tzsetwall): Move cleaned-up guts to new function tzsetwall_unlocked, for which this is now merely a locking wrapper. (tzset): Similarly, for new function tzset_unlocked. (localtime_tzset): New function, which does proper locking. (localtime, localtime_r): Use it. (gmtsub): Do not worry about initializing gmtptr, as that's now the caller's responsibility. (gmtime): Reimplement in terms of gmtime_r. (timegm): Reimplement in terms of timeoff. (gmtime_r, offtime, mktime, timeoff, time2posix, posix2time): Lock at start and unlock at end. * Makefile, NEWS: Document this. --- Makefile | 3 + NEWS | 4 + localtime.c | 249 ++++++++++++++++++++++++++++++++++++++++-------------------- 3 files changed, 173 insertions(+), 83 deletions(-)
diff --git a/Makefile b/Makefile index 5bbd7e7..3d24c2a 100644 --- a/Makefile +++ b/Makefile @@ -123,6 +123,9 @@ LDLIBS= # -DNO_RUN_TIME_WARNINGS_ABOUT_YEAR_2000_PROBLEMS_THANK_YOU=1 # if you do not want run time warnings about formats that may cause # year 2000 grief +# -DTHREAD_SAFE=1 to make localtime.c thread-safe, as POSIX requires; +# not needed by the main-program tz code, which is single-threaded. +# Append other compiler flags as needed, e.g., -pthread on GNU/Linux. # -Dtime_tz=\"T\" to use T as the time_t type, rather than the system time_t # -DTZ_DOMAIN=\"foo\" to use "foo" for gettext domain name; default is "tz" # -DTZ_DOMAINDIR=\"/path\" to use "/path" for gettext directory; diff --git a/NEWS b/NEWS index 28974c9..1a78041 100644 --- a/NEWS +++ b/NEWS @@ -36,6 +36,10 @@ Unreleased, experimental changes
Changes affecting code
+ The tz library is now thread-safe if compiled with THREAD_SAFE defined. + Although not needed for tz's own applications, which are single-threaded, + this supports POSIX better if the tz library is used in multithreaded apps. + tzselect -c now uses a hybrid distance measure that works better in Africa. (Thanks to Alan Barrett for noting the problem.)
diff --git a/localtime.c b/localtime.c index 551b5a2..f499583 100644 --- a/localtime.c +++ b/localtime.c @@ -16,6 +16,18 @@ #include "tzfile.h" #include "fcntl.h"
+#if THREAD_SAFE +# include <pthread.h> +# 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 + #ifndef TZ_ABBR_MAX_LEN #define TZ_ABBR_MAX_LEN 16 #endif /* !defined TZ_ABBR_MAX_LEN */ @@ -154,8 +166,8 @@ static struct state gmtmem; #endif /* !defined TZ_STRLEN_MAX */
static char lcl_TZname[TZ_STRLEN_MAX + 1]; -static int lcl_is_set; -static int gmt_is_set; +static int VOLATILE lcl_is_set; +static int VOLATILE gmt_is_set;
char * tzname[2] = { (char *) wildabbr, @@ -1139,6 +1151,21 @@ gmtload(struct state *const sp) (void) tzparse(gmt, sp, TRUE); }
+static void +tzsetwall_unlocked(void) +{ + if (lcl_is_set < 0) + return; +#ifdef ALL_STATE + if (! lclptr) + lclptr = malloc(sizeof *lclptr); +#endif + if (lclptr && tzload(NULL, lclptr, TRUE) != 0) + gmtload(lclptr); + settzname(); + lcl_is_set = -1; +} + #ifndef STD_INSPIRED /* ** A non-static declaration of tzsetwall in a system header file @@ -1149,65 +1176,71 @@ static void tzsetwall(void) { - if (lcl_is_set < 0) - return; - lcl_is_set = -1; + if (lock() != 0) + return; + tzsetwall_unlocked(); + unlock(); +}
+static void +tzset_unlocked(void) +{ + int shortname; + register char const *name = getenv("TZ"); + + if (!name) { + tzsetwall_unlocked(); + return; + } + shortname = strlen(name) < sizeof lcl_TZname; + if (0 < lcl_is_set && strcmp(lcl_TZname, name) == 0) + return; + if (shortname) + strcpy(lcl_TZname, name); #ifdef ALL_STATE - if (lclptr == NULL) { - lclptr = malloc(sizeof *lclptr); - if (lclptr == NULL) { - settzname(); /* all we can do */ - return; - } - } + if (! lclptr) + lclptr = malloc(sizeof *lclptr);
maybe lclptr = calloc(sizeof *lclptr,1); ? that would remove the need for lclptr->leapcnt = 0; etc. any information leak via padding bytes would be closed also.
#endif /* defined ALL_STATE */ - if (tzload(NULL, lclptr, TRUE) != 0) - gmtload(lclptr); - settzname(); + if (lclptr) { + if (*name == '\0') { + /* + ** User wants it fast rather than right. + */ + lclptr->leapcnt = 0; /* so, we're off a little */ + lclptr->timecnt = 0; + lclptr->typecnt = 0; + lclptr->ttis[0].tt_isdst = 0; + lclptr->ttis[0].tt_gmtoff = 0; + lclptr->ttis[0].tt_abbrind = 0; + strcpy(lclptr->chars, gmt); + } else if (tzload(name, lclptr, TRUE) != 0) + if (name[0] == ':' || tzparse(name, lclptr, FALSE) != 0) + gmtload(lclptr); + } + settzname(); + lcl_is_set = shortname; }
void tzset(void) { - register const char * name; - - name = getenv("TZ"); - if (name == NULL) { - tzsetwall(); - return; - } - - if (lcl_is_set > 0 && strcmp(lcl_TZname, name) == 0) - return; - lcl_is_set = strlen(name) < sizeof lcl_TZname; - if (lcl_is_set) - (void) strcpy(lcl_TZname, name); + if (lock() != 0) + return; + tzset_unlocked(); + unlock(); +}
+static void +gmtcheck(void) +{ + if (gmt_is_set) + return; #ifdef ALL_STATE - if (lclptr == NULL) { - lclptr = malloc(sizeof *lclptr); - if (lclptr == NULL) { - settzname(); /* all we can do */ - return; - } - } -#endif /* defined ALL_STATE */ - if (*name == '\0') { - /* - ** User wants it fast rather than right. - */ - lclptr->leapcnt = 0; /* so, we're off a little */ - lclptr->timecnt = 0; - lclptr->typecnt = 0; - lclptr->ttis[0].tt_isdst = 0; - lclptr->ttis[0].tt_gmtoff = 0; - lclptr->ttis[0].tt_abbrind = 0; - (void) strcpy(lclptr->chars, gmt); - } else if (tzload(name, lclptr, TRUE) != 0) - if (name[0] == ':' || tzparse(name, lclptr, FALSE) != 0) - (void) gmtload(lclptr); - settzname(); + gmtptr = malloc(sizeof *gmtptr); +#endif + if (gmtptr) + gmtload(gmtptr); + gmt_is_set = TRUE; }
/* @@ -1296,11 +1329,25 @@ localsub(const time_t *const timep, const int_fast32_t offset, return result; }
+static struct tm * +localtime_tzset(time_t const *timep, struct tm *tmp, int skip_tzset) +{ + int err = lock(); + if (err) { + errno = err; + return NULL; + } + if (!skip_tzset) + tzset_unlocked(); + tmp = localsub(timep, 0, tmp); + unlock(); + return tmp; +} + struct tm * localtime(const time_t *const timep) { - tzset(); - return localsub(timep, 0L, &tm); + return localtime_tzset(timep, &tm, 0); }
/* @@ -1310,7 +1357,7 @@ localtime(const time_t *const timep) struct tm * localtime_r(const time_t *const timep, struct tm *tmp) { - return localsub(timep, 0L, tmp); + return localtime_tzset(timep, tmp, lcl_is_set); }
/* @@ -1323,16 +1370,6 @@ gmtsub(const time_t *const timep, const int_fast32_t offset, { register struct tm * result;
- if (!gmt_is_set) { -#ifdef ALL_STATE - gmtptr = malloc(sizeof *gmtptr); - gmt_is_set = gmtptr != NULL; -#else - gmt_is_set = TRUE; -#endif /* defined ALL_STATE */ - if (gmt_is_set) - gmtload(gmtptr); - } result = timesub(timep, offset, gmtptr, tmp); #ifdef TM_ZONE /* @@ -1348,7 +1385,7 @@ gmtsub(const time_t *const timep, const int_fast32_t offset, struct tm * gmtime(const time_t *const timep) { - return gmtsub(timep, 0L, &tm); + return gmtime_r(timep, &tm); }
/* @@ -1358,7 +1395,15 @@ gmtime(const time_t *const timep) struct tm * gmtime_r(const time_t *const timep, struct tm *tmp) { - return gmtsub(timep, 0L, tmp); + int err = lock(); + if (err) { + errno = err; + return NULL; + } + gmtcheck(); + tmp = gmtsub(timep, 0, tmp); + unlock(); + return tmp; }
#ifdef STD_INSPIRED @@ -1366,7 +1411,16 @@ gmtime_r(const time_t *const timep, struct tm *tmp) struct tm * offtime(const time_t *const timep, const long offset) { - return gmtsub(timep, offset, &tm); + struct tm *tmp; + int err = lock(); + if (err) { + errno = err; + return NULL; + } + gmtcheck(); + tmp = gmtsub(timep, offset, &tm); + unlock(); + return tmp; }
#endif /* defined STD_INSPIRED */ @@ -1901,8 +1955,16 @@ time1(struct tm *const tmp, time_t mktime(struct tm *const tmp) { - tzset(); - return time1(tmp, localsub, 0L); + time_t t; + int err = lock(); + if (err) { + errno = err; + return -1; + } + tzset_unlocked(); + t = time1(tmp, localsub, 0); + unlock(); + return t; }
#ifdef STD_INSPIRED @@ -1918,17 +1980,25 @@ timelocal(struct tm *const tmp) time_t timegm(struct tm *const tmp) { - if (tmp != NULL) - tmp->tm_isdst = 0; - return time1(tmp, gmtsub, 0L); + return timeoff(tmp, 0); }
time_t timeoff(struct tm *const tmp, const long offset) { - if (tmp != NULL) - tmp->tm_isdst = 0; - return time1(tmp, gmtsub, offset); + time_t t; + int err; + if (tmp) + tmp->tm_isdst = 0; + err = lock(); + if (err) { + errno = err; + return -1; + } + gmtcheck(); + t = time1(tmp, gmtsub, offset); + unlock(); + return t; }
#endif /* defined STD_INSPIRED */ @@ -1986,8 +2056,17 @@ leapcorr(time_t *timep) time_t time2posix(time_t t) { - tzset(); - return t - leapcorr(&t); + time_t p; + int err = lock(); + if (err) { + errno = err; + return -1; + } + if (!lcl_is_set) + tzset_unlocked(); + p = t - leapcorr(&t); + unlock(); + return p; }
time_t @@ -1995,8 +2074,13 @@ posix2time(time_t t) { time_t x; time_t y; - - tzset(); + int err = lock(); + if (err) { + errno = err; + return -1; + } + if (!lcl_is_set) + tzset_unlocked(); /* ** For a positive leap second hit, the result ** is not unique. For a negative leap second @@ -2010,16 +2094,15 @@ posix2time(time_t t) x++; y = x - leapcorr(&x); } while (y < t); - if (t != y) - return x - 1; + x -= y != t; } else if (y > t) { do { --x; y = x - leapcorr(&x); } while (y > t); - if (t != y) - return x + 1; + x += y != t; } + unlock(); return x; }
walter harms wrote:
maybe lclptr = calloc(sizeof *lclptr,1); ? that would remove the need for lclptr->leapcnt = 0; etc. any information leak via padding bytes would be closed also.
Sorry, I don't see the information leak here, as lclptr is static and does not escape to calling code. More generally, the current code always uses malloc to allocate objects dynamically, and switching to calloc would be a pragmatics change that should be done as a separate patch. I'm not entirely sold on the idea of using calloc to avoid leaking information from previous uses of the memory. If information leakage is a concern, surely it's better to use a malloc wrapper that clears memory rather than to manually inspect and modify every call to malloc.
Am 18.08.2014 16:44, schrieb Paul Eggert:
walter harms wrote:
maybe lclptr = calloc(sizeof *lclptr,1); ? that would remove the need for lclptr->leapcnt = 0; etc. any information leak via padding bytes would be closed also.
Sorry, I don't see the information leak here, as lclptr is static and does not escape to calling code.
I have no idea either but i prefer defensive coding.
More generally, the current code always uses malloc to allocate objects dynamically, and switching to calloc would be a pragmatics change that should be done as a separate patch.
sorry, i had the impression that the patch was up to discussion.
I'm not entirely sold on the idea of using calloc to avoid leaking information from previous uses of the memory. If information leakage is a concern, surely it's better to use a malloc wrapper that clears memory rather than to manually inspect and modify every call to malloc.
information leak is not the primary concern, in this case it would simply mean that every field is already 0 no need to do this manually, saving a few bytes. re, wh
walter harms wrote:
i had the impression that the patch was up to discussion
Yes, although the code you're mentioning is unchanged from 2014f, and is present in the diff only due to unrelated refactoring, so I'd rather install this change as a separate patch.
it would simply mean that every field is already 0 no need to do this manually, saving a few bytes.
That wouldn't work, because in the normal case malloc (or calloc) is not called and the storage is reused, so it needs to be initialized anyway. We could replace the initialization code with a call to memset but on my platform (Fedora 20 x86-64 GCC 4.9.1) that made the 'date' executable slightly larger and I presume slightly slower.
The attached minor fixup squashes a compiler warning caused by the proposed patch when compiled without STD_INSPIRED.
Attached is another fixup for that patch, to fix a race that could cause two threads try to assign to tzname simultaneously. Only tzset-like functions (or functions documented to call tzset) should assign to tzname anyway. The commentary for this fixup can be: (localsub): Don't set tzname here; that's not thread-safe. This change can lose information on hosts without TM_ZONE, but there is no reliable way to fix that in a thread-safe way.
participants (2)
-
Paul Eggert -
walter harms