[tz] [PROPOSED PATCH] Make the library thread-safe if THREAD_SAFE is defined.
walter harms
wharms at bfs.de
Mon Aug 18 08:23:39 UTC 2014
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;
> }
>
More information about the tz
mailing list