[PROPOSED 1/3] tzset sets errno if lock fails
* localtime.c (tzset): Set errno if the lock fails. This should never happen, but if it does the least we can do is set errno, as other functions do. --- localtime.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/localtime.c b/localtime.c index 54db0ab7..ef094560 100644 --- a/localtime.c +++ b/localtime.c @@ -1788,8 +1788,11 @@ void tzset(void) { monotime_t now = get_monotonic_time(); - if (lock() != 0) + int err = lock(); + if (err != 0) { + errno = err; return; + } tzset_unlocked(now); unlock(); } -- 2.51.0
This avoids a call to malloc in some failure cases (e.g., bad TZ setting) and should simplify future improvements. * localtime.c (ALL_STATE): Default to 0. All uses changed. (tzloadbody): Last arg is now pointer-to-pointer, instead of just pointer, so that in the ALL_STATE case we malloc the storage instead of the caller doing it. Caller changed. --- localtime.c | 53 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/localtime.c b/localtime.c index ef094560..c7b9f663 100644 --- a/localtime.c +++ b/localtime.c @@ -378,12 +378,14 @@ static struct tm *timesub(time_t const *, int_fast32_t, struct state const *, struct tm *); static bool tzparse(char const *, struct state *, struct state const *); -#ifdef ALL_STATE +#ifndef ALL_STATE +# define ALL_STATE 0 +#endif + +#if ALL_STATE static struct state * lclptr; static struct state * gmtptr; -#endif /* defined ALL_STATE */ - -#ifndef ALL_STATE +#else static struct state lclmem; static struct state gmtmem; static struct state *const lclptr = &lclmem; @@ -702,18 +704,19 @@ enum { TZLOAD_TZSTRING = 2 }; /* Read any newline-surrounded TZ string. */ enum { TZLOAD_TZDIR_SUB = 4 }; /* TZ should be a file under TZDIR. */ /* Load tz data from the file named NAME into *SP. Respect TZLOADFLAGS. - Use *LSP for temporary storage. Return 0 on + Use **LSPP for temporary storage. Return 0 on success, an errno value on failure. */ static int tzloadbody(char const *name, struct state *sp, char tzloadflags, - union local_storage *lsp) + union local_storage **lspp) { register int i; register int fid; register int stored; register ssize_t nread; char const *relname; - register union input_buffer *up = &lsp->u.u; + union local_storage *lsp = *lspp; + union input_buffer *up; register int tzheadsize = sizeof(struct tzhead); int dd = AT_FDCWD; int oflags = (O_RDONLY | O_BINARY | O_CLOEXEC | O_CLOFORK @@ -793,6 +796,12 @@ tzloadbody(char const *name, struct state *sp, char tzloadflags, /* Create a string "TZDIR/NAME". Using sprintf here would pull in stdio (and would fail if the resulting string length exceeded INT_MAX!). */ + if (ALL_STATE) { + lsp = malloc(sizeof *lsp); + if (!lsp) + return HAVE_MALLOC_ERRNO ? errno : ENOMEM; + *lspp = lsp; + } cp = lsp->fullname; cp = mempcpy(cp, tzdirslash, tzdirslashlen); cp = mempcpy(cp, name, namelen); @@ -814,6 +823,13 @@ tzloadbody(char const *name, struct state *sp, char tzloadflags, && !tzfile_changed(fid, &st)) err = -1; else { + if (ALL_STATE && !lsp) { + lsp = malloc(sizeof *lsp); + if (!lsp) + return HAVE_MALLOC_ERRNO ? errno : ENOMEM; + *lspp = lsp; + } + up = &lsp->u.u; nread = read(fid, up->buf, sizeof up->buf); err = tzheadsize <= nread ? 0 : nread < 0 ? errno : EINVAL; } @@ -1069,19 +1085,18 @@ tzloadbody(char const *name, struct state *sp, char tzloadflags, static int tzload(char const *name, struct state *sp, char tzloadflags) { -#ifdef ALL_STATE - union local_storage *lsp = malloc(sizeof *lsp); - if (!lsp) { - return HAVE_MALLOC_ERRNO ? errno : ENOMEM; - } else { - int err = tzloadbody(name, sp, tzloadflags, lsp); - free(lsp); - return err; - } + int r; + union local_storage *lsp; +#if ALL_STATE + lsp = NULL; #else union local_storage ls; - return tzloadbody(name, sp, tzloadflags, &ls); + lsp = &ls; #endif + r = tzloadbody(name, sp, tzloadflags, &lsp); + if (ALL_STATE) + free(lsp); + return r; } static const int mon_lengths[2][MONSPERYEAR] = { @@ -1741,7 +1756,7 @@ tzset_unlocked(monotime_t now) ? 0 < lcl_is_set && strcmp(lcl_TZname, name) == 0 : lcl_is_set < 0)) return; -# ifdef ALL_STATE +# if ALL_STATE if (! sp) lclptr = sp = malloc(sizeof *lclptr); # endif @@ -1805,7 +1820,7 @@ gmtcheck(void) if (lock() != 0) return; if (! gmt_is_set) { -#ifdef ALL_STATE +#if ALL_STATE gmtptr = malloc(sizeof *gmtptr); #endif if (gmtptr) -- 2.51.0
Do not impose an arbitrary file name length limit on platforms like GNU/Hurd that do not define PATH_MAX. Conversely, when PATH_MAX is defined (e.g., GNU/Linux, the BSDs), speed things up a bit if the TZ string is very long. * NEWS: Mention this. * localtime.c (union local_storage.fullname): Now of size PATH_MAX if PATH_MAX is defined, and removed otherwise. All uses changed. (tzloadbody) [!PATH_MAX]: If the TZ string is too long, allocate a larger buffer instead of failing with ENAMETOOLONG. This happens only with artificial TZ strings like "America/./././Los_Angeles" except with many more "/."s. This supports the GNU/Hurd philosophy of no arbitrary limits. (tzloadbody) [PATH_MAX]: Speed things up a bit. (tzload): Call ‘free’ even if !ALL_STATE, if tzloadbody allocated a larger buffer. With decent optimization this call is optimized away if PATH_MAX && !ALL_STATE (the default case on most platforms). --- NEWS | 4 ++++ localtime.c | 35 ++++++++++++++++++++++------------- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/NEWS b/NEWS index 5b00817f..64d9336d 100644 --- a/NEWS +++ b/NEWS @@ -77,6 +77,10 @@ Unreleased, experimental changes tzset etc. now have an experimental OPENAT_TZDIR option; see Makefile and localtime.c for details. + On platforms like GNU/Hurd that do not define PATH_MAX, + exceedingly long TZ strings no longer fail merely because they + exceed an arbitrary file name length limit imposed by tzcode. + Changes to commentary The leapseconds file contains commentary about the IERS and NIST diff --git a/localtime.c b/localtime.c index c7b9f663..7ab8aa41 100644 --- a/localtime.c +++ b/localtime.c @@ -689,13 +689,10 @@ union local_storage { struct state st; } u; - /* The name of the file to be opened. Ideally this would have no - size limits, to support arbitrarily long Zone names. - Limiting Zone names to 1024 bytes should suffice for practical use. - However, there is no need for this to be smaller than struct - file_analysis as that struct is allocated anyway, as the other - union member. */ - char fullname[max(sizeof(struct file_analysis), sizeof TZDIR + 1024)]; +#ifdef PATH_MAX + /* The name of the file to be opened. */ + char fullname[PATH_MAX]; +#endif }; /* These tzload flags can be ORed together, and fit into 'char'. */ @@ -788,25 +785,35 @@ tzloadbody(char const *name, struct state *sp, char tzloadflags, if (!OPENAT_TZDIR && !SUPPRESS_TZDIR && name[0] != '/') { char *cp; - size_t namesizemax = sizeof lsp->fullname - tzdirslashlen; + size_t fullnamesize; +#ifdef PATH_MAX + static_assert(tzdirslashlen <= PATH_MAX); + size_t namesizemax = PATH_MAX - tzdirslashlen; size_t namelen = strnlen (name, namesizemax); if (namesizemax <= namelen) return ENAMETOOLONG; +#else + size_t namelen = strlen (name); +#endif + fullnamesize = tzdirslashlen + namelen + 1; /* Create a string "TZDIR/NAME". Using sprintf here would pull in stdio (and would fail if the resulting string length exceeded INT_MAX!). */ - if (ALL_STATE) { - lsp = malloc(sizeof *lsp); + if (ALL_STATE || sizeof *lsp <= fullnamesize) { + lsp = malloc(max(sizeof *lsp, fullnamesize)); if (!lsp) return HAVE_MALLOC_ERRNO ? errno : ENOMEM; *lspp = lsp; } - cp = lsp->fullname; - cp = mempcpy(cp, tzdirslash, tzdirslashlen); + cp = mempcpy(lsp, tzdirslash, tzdirslashlen); cp = mempcpy(cp, name, namelen); *cp = '\0'; +#ifdef PATH_MAX name = lsp->fullname; +#else + name = (char *) lsp; +#endif } fid = OPENAT_TZDIR ? openat(dd, relname, oflags) : open(name, oflags); @@ -1086,6 +1093,7 @@ static int tzload(char const *name, struct state *sp, char tzloadflags) { int r; + union local_storage *lsp0; union local_storage *lsp; #if ALL_STATE lsp = NULL; @@ -1093,8 +1101,9 @@ tzload(char const *name, struct state *sp, char tzloadflags) union local_storage ls; lsp = &ls; #endif + lsp0 = lsp; r = tzloadbody(name, sp, tzloadflags, &lsp); - if (ALL_STATE) + if (lsp != lsp0) free(lsp); return r; } -- 2.51.0
* localtime.c (tzloadbody): Change ‘<=’ to ‘<’. This fixes an obscure and unlikely performance bug, where malloc was called when it need not have been called in the !ALL_STATE case. --- localtime.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/localtime.c b/localtime.c index 7ab8aa41..3f3bdce5 100644 --- a/localtime.c +++ b/localtime.c @@ -800,7 +800,7 @@ tzloadbody(char const *name, struct state *sp, char tzloadflags, /* Create a string "TZDIR/NAME". Using sprintf here would pull in stdio (and would fail if the resulting string length exceeded INT_MAX!). */ - if (ALL_STATE || sizeof *lsp <= fullnamesize) { + if (ALL_STATE || sizeof *lsp < fullnamesize) { lsp = malloc(max(sizeof *lsp, fullnamesize)); if (!lsp) return HAVE_MALLOC_ERRNO ? errno : ENOMEM; -- 2.48.1
participants (1)
-
Paul Eggert