source code question regarding localtime.c PS
In localtime.c, function localsub() are these three lines of code: 1295 icycles = tcycles; 1296 if (tcycles - icycles >= 1 || icycles - tcycles >= 1) 1297 return NULL; I do not understand the reason why lines 1296 and 1297 exist. icycles and tcycles are equal. Their difference is always zero. Why test for it being not zero? PS: the reason why I am asking: I try to understand the source code of zdump.c It calls localtime() frequently, and has some cryptic code which depends on whether localtime() returned NULL. Therefore, I try to understand the conditions under which localtime() returns NULL. If that would be documented somewhee (I may have overlooked it), it would be great, as it is rather hard to understand it out of the terse code.
Since icycles and tcycles are of different types, there could be some conversion involved at line 1295, with possible data truncation. Thus, the comparisons on line 1296 might not be always false. (I think there were some recent comments about the possibility of time_t being a non-integral type...) -----Original Message----- From: tz-bounces@iana.org [mailto:tz-bounces@iana.org] On Behalf Of Alois Treindl Sent: Wednesday, August 07, 2013 6:09 AM To: tz@iana.org Subject: [tz] source code question regarding localtime.c PS In localtime.c, function localsub() are these three lines of code: 1295 icycles = tcycles; 1296 if (tcycles - icycles >= 1 || icycles - tcycles >= 1) 1297 return NULL; I do not understand the reason why lines 1296 and 1297 exist. icycles and tcycles are equal. Their difference is always zero. Why test for it being not zero? PS: the reason why I am asking: I try to understand the source code of zdump.c It calls localtime() frequently, and has some cryptic code which depends on whether localtime() returned NULL. Therefore, I try to understand the conditions under which localtime() returns NULL. If that would be documented somewhee (I may have overlooked it), it would be great, as it is rather hard to understand it out of the terse code.
Alois Treindl said:
In localtime.c, function localsub() are these three lines of code:
1295 icycles = tcycles; 1296 if (tcycles - icycles >= 1 || icycles - tcycles >= 1) 1297 return NULL;
I do not understand the reason why lines 1296 and 1297 exist. icycles and tcycles are equal.
time_t just has to be an arithmetic type. This could include long long, double, long double, or even a complex type. Therefore it's possible that tcycles could contain a value that's not in the range of int_fast64_t (the type of icycles). That code checks whether the value in tcycles is within the range of int_fast64_t. If it is, the conversion on line 1295 will either produce the same number or (if time_t is floating point) will round it off to the nearest integer. In that case, both halves of the test will be false. But if the value in tcycles is out of range, the conversion will generate a completely different number, and so one of those tests will be true. -- Clive D.W. Feather | If you lie to the compiler, Email: clive@davros.org | it will get its revenge. Web: http://www.davros.org | - Henry Spencer Mobile: +44 7973 377646
On Wed, 07 Aug 2013, Clive D.W. Feather wrote:
Alois Treindl said:
In localtime.c, function localsub() are these three lines of code:
1295 icycles = tcycles; 1296 if (tcycles - icycles >= 1 || icycles - tcycles >= 1) 1297 return NULL;
I do not understand the reason why lines 1296 and 1297 exist. icycles and tcycles are equal.
[tcycles has type time_t, which might not be an integer type] [icycles has type int_fast64_t]
That code checks whether the value in tcycles is within the range of int_fast64_t. If it is, the conversion on line 1295 will either produce the same number or (if time_t is floating point) will round it off to the nearest integer. In that case, both halves of the test will be false.
But if the value in tcycles is out of range, the conversion will generate a completely different number, and so one of those tests will be true.
If the value is out of range, then the assignment (icycles = tcycles) invokes undefined behaviour, so the test in the if statement might not do what one would expect. If the tz code wants to check that a variable is in range, I think it should do without performing a possibly-undefined operation. --apb (Alan Barrett)
thank you. I admit that I tend to overlook the problems arising from portability to unusual machines. More source code comments would be useful, as such code is not obvious. On 07.08.13 16:27, Clive D.W. Feather wrote:
Alois Treindl said:
In localtime.c, function localsub() are these three lines of code:
1295 icycles = tcycles; 1296 if (tcycles - icycles >= 1 || icycles - tcycles >= 1) 1297 return NULL;
I do not understand the reason why lines 1296 and 1297 exist. icycles and tcycles are equal.
time_t just has to be an arithmetic type. This could include long long, double, long double, or even a complex type. Therefore it's possible that tcycles could contain a value that's not in the range of int_fast64_t (the type of icycles).
That code checks whether the value in tcycles is within the range of int_fast64_t. If it is, the conversion on line 1295 will either produce the same number or (if time_t is floating point) will round it off to the nearest integer. In that case, both halves of the test will be false.
But if the value in tcycles is out of range, the conversion will generate a completely different number, and so one of those tests will be true.
Thanks for mentioning the problem. The code relies on undefined behavior when a time_t value is outside int_fast64_t range. It's better to avoid undefined behavior, and the code would be clearer if we put in some comments about this issue. Here's a proposed patch for this, which I've pushed to the experimental github repository.
From b0bf6d8fee915393274deb7df0ba2aacc8756d99 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Wed, 7 Aug 2013 09:03:09 -0700 Subject: [PATCH] Avoid undefined behavior on integer overflow.
Problem reported by Alois Treindl in <http://mm.icann.org/pipermail/tz/2013-August/019493.html>. * localtime.c (truncate_time): New function. (localsub): Use it to avoid undefined behavior on integer overflow. * private.h (INTMAX_MAX, INTMAX_MIN, UINTMAX_MAX): New macros, for older platforms that lack them. --- localtime.c | 50 ++++++++++++++++++++++++++++++++++++++------------ private.h | 15 +++++++++++++++ 2 files changed, 53 insertions(+), 12 deletions(-) diff --git a/localtime.c b/localtime.c index 23bc635..353c73b 100644 --- a/localtime.c +++ b/localtime.c @@ -1254,6 +1254,38 @@ tzset(void) settzname(); } +/* Return T with any fractional part discarded. */ +static time_t +truncate_time(time_t t) +{ + /* + ** If time_t is floating-point, convert it to integer and + ** back; this discards the fraction. Avoid using <math.h> + ** functions, as we don't want to depend on <math.h>. Use <, + ** not <=, when comparing against *_MIN and *_MAX values, as + ** this avoids undefined behavior when, for example, + ** INTMAX_MAX is converted to a larger time_t value before it + ** is compared. + ** + ** On all platforms that we know of, if a time value is + ** outside intmax_t/uintmax_t range, then it is an integer so we can + ** simply return it. There's no simple, portable way to check this. + ** If you know of a counterexample platform, please report a bug. + */ + if (!TYPE_INTEGRAL(time_t)) { + if (INTMAX_MIN < t && t < INTMAX_MAX) { + intmax_t i = t; + return i; + } + if (0 <= t && t < UINTMAX_MAX) { + uintmax_t i = t; + return i; + } + } + + return t; +} + /* ** The easy way to behave "as if no library function calls" localtime ** is to not call it--so we drop its guts into "localsub", which can be @@ -1283,21 +1315,15 @@ localsub(const time_t *const timep, const int_fast32_t offset, (sp->goahead && t > sp->ats[sp->timecnt - 1])) { time_t newt = t; register time_t seconds; - register time_t tcycles; - register int_fast64_t icycles; + register time_t years; if (t < sp->ats[0]) seconds = sp->ats[0] - t; else seconds = t - sp->ats[sp->timecnt - 1]; --seconds; - tcycles = seconds / YEARSPERREPEAT / AVGSECSPERYEAR; - ++tcycles; - icycles = tcycles; - if (tcycles - icycles >= 1 || icycles - tcycles >= 1) - return NULL; - seconds = icycles; - seconds *= YEARSPERREPEAT; - seconds *= AVGSECSPERYEAR; + years = (truncate_time (seconds / SECSPERREPEAT + 1) + * YEARSPERREPEAT); + seconds = years * AVGSECSPERYEAR; if (t < sp->ats[0]) newt += seconds; else newt -= seconds; @@ -1310,8 +1336,8 @@ localsub(const time_t *const timep, const int_fast32_t offset, newy = tmp->tm_year; if (t < sp->ats[0]) - newy -= icycles * YEARSPERREPEAT; - else newy += icycles * YEARSPERREPEAT; + newy -= years; + else newy += years; tmp->tm_year = newy; if (tmp->tm_year != newy) return NULL; diff --git a/private.h b/private.h index a31a26e..f00a926 100644 --- a/private.h +++ b/private.h @@ -167,9 +167,18 @@ typedef int int_fast32_t; # if defined LLONG_MAX || defined __LONG_LONG_MAX__ typedef long long intmax_t; # define PRIdMAX "lld" +# ifdef LLONG_MAX +# define INTMAX_MAX LLONG_MAX +# define INTMAX_MIN LLONG_MIN +# else +# define INTMAX_MAX __LONG_LONG_MAX__ +# define INTMAX_MIN __LONG_LONG_MIN__ +# endif # else typedef long intmax_t; # define PRIdMAX "ld" +# define INTMAX_MAX LONG_MAX +# define INTMAX_MIN LONG_MIN # endif #endif @@ -177,9 +186,15 @@ typedef long intmax_t; # if defined ULLONG_MAX || defined __LONG_LONG_MAX__ typedef unsigned long long uintmax_t; # define PRIuMAX "llu" +# ifdef ULLONG_MAX +# define UINTMAX_MAX ULLONG_MAX +# else +# define UINTMAX_MAX (__LONG_LONG_MAX__ * 2ULL + 1) +# endif # else typedef unsigned long uintmax_t; # define PRIuMAX "lu" +# define UINTMAX_MAX ULONG_MAX # endif #endif -- 1.8.1.2
Paul Eggert said:
Thanks for mentioning the problem. The code relies on undefined behavior when a time_t value is outside int_fast64_t range. It's better to avoid undefined behavior,
+ if (!TYPE_INTEGRAL(time_t)) { + if (INTMAX_MIN < t && t < INTMAX_MAX) {
In principle UINTMAX_MAX could be larger than the largest value representable in time_t if it is (say) float. If so, the implicit conversion to float is undefined and you haven't solved the problem. You need to compare the various limits to decide which type can handle all the values of the other. Of course, there's no requirement that time_t values map to times in a linear manner - for example, an integral time_t could be split into bit fields holding hours, minutes, seconds, etc. That's why difftime() exists. -- Clive D.W. Feather | If you lie to the compiler, Email: clive@davros.org | it will get its revenge. Web: http://www.davros.org | - Henry Spencer Mobile: +44 7973 377646
Of course, there's no requirement that time_t values map to times in a linear manner...
From... http://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/types.h.html ...we have: time_t: Used for time in seconds. @dashdashado On Wed, Aug 7, 2013 at 12:51 PM, Clive D.W. Feather <clive@davros.org>wrote:
Paul Eggert said:
Thanks for mentioning the problem. The code relies on undefined behavior when a time_t value is outside int_fast64_t range. It's better to avoid undefined behavior,
+ if (!TYPE_INTEGRAL(time_t)) { + if (INTMAX_MIN < t && t < INTMAX_MAX) {
In principle UINTMAX_MAX could be larger than the largest value representable in time_t if it is (say) float. If so, the implicit conversion to float is undefined and you haven't solved the problem. You need to compare the various limits to decide which type can handle all the values of the other.
Of course, there's no requirement that time_t values map to times in a linear manner - for example, an integral time_t could be split into bit fields holding hours, minutes, seconds, etc. That's why difftime() exists.
-- Clive D.W. Feather | If you lie to the compiler, Email: clive@davros.org | it will get its revenge. Web: http://www.davros.org | - Henry Spencer Mobile: +44 7973 377646
On Wed, 07 Aug 2013, Arthur David Olson wrote:
Of course, there's no requirement that time_t values map to times in a linear manner...
From... http://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/types.h.html ...we have: time_t: Used for time in seconds.
Yes, POSIX requires that time_t is encoded in seconds, but ISO C does not. ISO 8988:1999 section 7.23.2.4 says, of the time_t value returned by the the time() function, "The encoding of the value is unspecified." Perhaps there should be some clear documentation of the portability goals of the tz code project. As far as I am aware, the tz project assumes that time_t is encoded in seconds, and assumes that time_t is either an integer or floating point type. --apb (Alan Barrett)
On 08/07/2013 10:36 AM, Alan Barrett wrote:
the tz project assumes that time_t is encoded in seconds, and assumes that time_t is either an integer or floating point type.
Hah! I just checked, and we're behind the times. The latest version of POSIX, POSIX.1-2013, added the requirement that time_t must be an integer type. At some point we should remove support for floating-point time_t, as it hasn't been tested on any real-world platform and probably does not work. In the meantime I pushed the following proposed doc change to the experimental repository.
From 8fe97ff3bc1b1edca97286c7e6bbc03da340613f Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Wed, 7 Aug 2013 17:19:22 -0700 Subject: [PATCH] * Theory: Document time_t better and update for POSIX.1-2013.
Change "`" to "'"; these days, "`" and "'" are not symmetric. --- Theory | 125 +++++++++++++++++++++++++++++++++++++---------------------------- 1 file changed, 71 insertions(+), 54 deletions(-) diff --git a/Theory b/Theory index 751b12d..1b5374a 100644 --- a/Theory +++ b/Theory @@ -12,16 +12,13 @@ This file is in the public domain, so clarified as of ----- Time and date functions ----- -These time and date functions are upwards compatible with POSIX, +These time and date functions are upwards compatible with those of POSIX, an international standard for UNIX-like systems. As of this writing, the current edition of POSIX is: - Standard for Information technology - -- Portable Operating System Interface (POSIX (R)) - -- System Interfaces - IEEE Std 1003.1, 2004 Edition - <http://www.opengroup.org/online-pubs?DOC=7999959899> - <http://www.opengroup.org/pubs/catalog/t041.htm> + The Open Group Base Specifications Issue 7 + IEEE Std 1003.1, 2013 Edition + <http://pubs.opengroup.org/onlinepubs/9699919799/> POSIX has the following properties and limitations. @@ -34,7 +31,7 @@ POSIX has the following properties and limitations. The POSIX TZ string takes the following form: - stdoffset[dst[offset],date[/time],date[/time]] + stdoffset[dst[offset][,date[/time],date[/time]]] where: @@ -45,15 +42,17 @@ POSIX has the following properties and limitations. in a quoted form like "<UTC+10>"; this allows "+" and "-" in the names. offset - is of the form `[-]hh:[mm[:ss]]' and specifies the - offset west of UTC. The default DST offset is one hour - ahead of standard time. + is of the form '[+-]hh:[mm[:ss]]' and specifies the + offset west of UTC. 'hh' may be a single digit; 0<=hh<=24. + The default DST offset is one hour ahead of standard time. date[/time],date[/time] specifies the beginning and end of DST. If this is absent, the system supplies its own rules for DST, and these can differ from year to year; typically US DST rules are used. time - takes the form `hh:[mm[:ss]]' and defaults to 02:00. + takes the form 'hh:[mm[:ss]]' and defaults to 02:00. + This is the same format as the offset, except that a + leading '+' or '-' is not allowed. date takes one of the following forms: Jn (1<=n<=365) @@ -63,8 +62,10 @@ POSIX has the following properties and limitations. Mm.n.d (0[Sunday]<=d<=6[Saturday], 1<=n<=5, 1<=m<=12) for the dth day of week n of month m of the year, where week 1 is the first week in which day d appears, - and `5' stands for the last week in which day d appears + and '5' stands for the last week in which day d appears (which may be either the 4th or 5th week). + Typically, this is the only useful form; + the n and Jn forms are rarely used. Here is an example POSIX TZ string, for US Pacific time using rules appropriate from 1987 through 2006: @@ -95,6 +96,20 @@ POSIX has the following properties and limitations. * POSIX requires that systems ignore leap seconds. +* The tz code attempts attempts to support all the time_t implementations + allowed by POSIX. The time_t type represents a nonnegative count of + seconds since 1970-01-01 00:00:00 UTC, ignoring leap seconds. + In practice, time_t is usually a signed 64- or 32-bit integer; 32-bit + signed time_t values stop working after 2038-01-19 03:14:07 UTC, so + new implementations these days typically use a signed 64-bit integer. + Unsigned 32-bit integers are used on one or two platforms, + and 36-bit integers are also used occasionally. + Although POSIX.1-2013 requires time_t to be an integer type, + earlier POSIX versions also allowed time_t to be a floating-point type. + No known platforms have a floating-point time_t and although + the tz code attempts to support floating-point time_t this has not + been tested and will probably be removed at some point. + These are the extensions that have been made to the POSIX functions: * The "TZ" environment variable is used in generating the name of a file @@ -146,6 +161,8 @@ These are the extensions that have been made to the POSIX functions: environment variable; portable applications should not, however, rely on this behavior since it's not the way SVR2 systems behave.) +* Negative time_t values are supported, on systems where time_t is signed. + * These functions can account for leap seconds, thanks to Bradley White. Points of interest to folks with other systems: @@ -155,7 +172,7 @@ Points of interest to folks with other systems: On such hosts, the primary use of this package is to update obsolete time zone rule tables. To do this, you may need to compile the time zone compiler - `zic' supplied with this package instead of using the system `zic', + 'zic' supplied with this package instead of using the system 'zic', since the format of zic's input changed slightly in late 1994, and many vendors still do not support the new input format. @@ -251,19 +268,19 @@ one example. Names normally have the form AREA/LOCATION, where AREA is the name of a continent or ocean, and LOCATION is the name of a specific location within that region. North and South America share the same -area, `America'. Typical names are `Africa/Cairo', `America/New_York', -and `Pacific/Honolulu'. +area, 'America'. Typical names are 'Africa/Cairo', 'America/New_York', +and 'Pacific/Honolulu'. Here are the general rules used for choosing location names, in decreasing order of importance: Use only valid POSIX file name components (i.e., the parts of - names other than `/'). Within a file name component, - use only ASCII letters, `.', `-' and `_'. Do not use + names other than '/'). Within a file name component, + use only ASCII letters, '.', '-' and '_'. Do not use digits, as that might create an ambiguity with POSIX TZ strings. A file name component must not exceed 14 - characters or start with `-'. E.g., prefer `Brunei' - to `Bandar_Seri_Begawan'. + characters or start with '-'. E.g., prefer 'Brunei' + to 'Bandar_Seri_Begawan'. Do not use names that differ only in case. Although the reference implementation is case-sensitive, some other implementations are not, and they would mishandle names differing only in case. @@ -275,51 +292,51 @@ in decreasing order of importance: Otherwise these tables would become annoyingly large. If a name is ambiguous, use a less ambiguous alternative; e.g. many cities are named San Jose and Georgetown, so - prefer `Costa_Rica' to `San_Jose' and `Guyana' to `Georgetown'. + prefer 'Costa_Rica' to 'San_Jose' and 'Guyana' to 'Georgetown'. Keep locations compact. Use cities or small islands, not countries or regions, so that any future time zone changes do not split - locations into different time zones. E.g. prefer `Paris' - to `France', since France has had multiple time zones. - Use mainstream English spelling, e.g. prefer `Rome' to `Roma', and - prefer `Athens' to the true name (which uses Greek letters). + locations into different time zones. E.g. prefer 'Paris' + to 'France', since France has had multiple time zones. + Use mainstream English spelling, e.g. prefer 'Rome' to 'Roma', and + prefer 'Athens' to the true name (which uses Greek letters). The POSIX file name restrictions encourage this rule. Use the most populous among locations in a zone, - e.g. prefer `Shanghai' to `Beijing'. Among locations with + e.g. prefer 'Shanghai' to 'Beijing'. Among locations with similar populations, pick the best-known location, - e.g. prefer `Rome' to `Milan'. - Use the singular form, e.g. prefer `Canary' to `Canaries'. - Omit common suffixes like `_Islands' and `_City', unless that - would lead to ambiguity. E.g. prefer `Cayman' to - `Cayman_Islands' and `Guatemala' to `Guatemala_City', - but prefer `Mexico_City' to `Mexico' because the country + e.g. prefer 'Rome' to 'Milan'. + Use the singular form, e.g. prefer 'Canary' to 'Canaries'. + Omit common suffixes like '_Islands' and '_City', unless that + would lead to ambiguity. E.g. prefer 'Cayman' to + 'Cayman_Islands' and 'Guatemala' to 'Guatemala_City', + but prefer 'Mexico_City' to 'Mexico' because the country of Mexico has several time zones. - Use `_' to represent a space. - Omit `.' from abbreviations in names, e.g. prefer `St_Helena' - to `St._Helena'. + Use '_' to represent a space. + Omit '.' from abbreviations in names, e.g. prefer 'St_Helena' + to 'St._Helena'. Do not change established names if they only marginally violate the above rules. For example, don't change - the existing name `Rome' to `Milan' merely because + the existing name 'Rome' to 'Milan' merely because Milan's population has grown to be somewhat greater than Rome's. - If a name is changed, put its old spelling in the `backward' file. + If a name is changed, put its old spelling in the 'backward' file. This means old spellings will continue to work. -The file `zone.tab' lists the geographical locations used to name +The file 'zone.tab' lists the geographical locations used to name time zone rule files. It is intended to be an exhaustive list of names for geographic regions as described above. Older versions of this package used a different naming scheme, and these older names are still supported. -See the file `backward' for most of these older names -(e.g. `US/Eastern' instead of `America/New_York'). +See the file 'backward' for most of these older names +(e.g. 'US/Eastern' instead of 'America/New_York'). The other old-fashioned names still supported are -`WET', `CET', `MET', and `EET' (see the file `europe'). +'WET', 'CET', 'MET', and 'EET' (see the file 'europe'). ----- Time zone abbreviations ----- When this package is installed, it generates time zone abbreviations -like `EST' to be compatible with human tradition and POSIX. +like 'EST' to be compatible with human tradition and POSIX. Here are the general rules used for choosing time zone abbreviations, in decreasing order of importance: @@ -346,24 +363,24 @@ in decreasing order of importance: letters. Use abbreviations that are in common use among English-speakers, - e.g. `EST' for Eastern Standard Time in North America. + e.g. 'EST' for Eastern Standard Time in North America. We assume that applications translate them to other languages as part of the normal localization process; for example, - a French application might translate `EST' to `HNE'. + a French application might translate 'EST' to 'HNE'. For zones whose times are taken from a city's longitude, use the - traditional xMT notation, e.g. `PMT' for Paris Mean Time. - The only name like this in current use is `GMT'. + traditional xMT notation, e.g. 'PMT' for Paris Mean Time. + The only name like this in current use is 'GMT'. If there is no common English abbreviation, abbreviate the English translation of the usual phrase used by native speakers. If this is not available or is a phrase mentioning the country - (e.g. ``Cape Verde Time''), then: + (e.g. "Cape Verde Time"), then: When a country is identified with a single or principal zone, - append `T' to the country's ISO code, e.g. `CVT' for - Cape Verde Time. For summer time append `ST'; - for double summer time append `DST'; etc. + append 'T' to the country's ISO code, e.g. 'CVT' for + Cape Verde Time. For summer time append 'ST'; + for double summer time append 'DST'; etc. Otherwise, take the first three letters of an English place name identifying each zone and append 'T', 'ST', etc. as before; e.g. 'VLAST' for VLAdivostok Summer Time. @@ -373,10 +390,10 @@ in decreasing order of importance: in some sense, asleep. Application writers should note that these abbreviations are ambiguous -in practice: e.g. `EST' has a different meaning in Australia than +in practice: e.g. 'EST' has a different meaning in Australia than it does in the United States. In new applications, it's often better -to use numeric UTC offsets like `-0500' instead of time zone -abbreviations like `EST'; this avoids the ambiguity. +to use numeric UTC offsets like '-0500' instead of time zone +abbreviations like 'EST'; this avoids the ambiguity. ----- Calendrical issues ----- @@ -401,7 +418,7 @@ and (in Paris only) 1871-05-06 through 1871-05-23. Russia From Chris Carrier (1996-12-02): -On 1929-10-01 the Soviet Union instituted an ``Eternal Calendar'' +On 1929-10-01 the Soviet Union instituted an "Eternal Calendar" with 30-day months plus 5 holidays, with a 5-day week. On 1931-12-01 it changed to a 6-day week; in 1934 it reverted to the Gregorian calendar while retaining the 6-day week; on 1940-06-27 it -- 1.8.1.2
On 08/07/2013 09:51 AM, Clive D.W. Feather wrote:
In principle UINTMAX_MAX could be larger than the largest value representable in time_t if it is (say) float. If so, the implicit conversion to float is undefined and you haven't solved the problem.
True, that's an extra assumption that should be documented. And come to think of it there's no point using UINTMAX_MAX. Here's a further patch that implements this, which I've pushed to the experimental repository.
From 3638e641e8d05f8e8dd86a7c6f216e3527e2dccd Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Wed, 7 Aug 2013 16:08:39 -0700 Subject: [PATCH] Don't bother with uintmax_t in previous change.
See the thread starting with Clive D.W. Feather's comments in <http://mm.icann.org/pipermail/tz/2013-August/019496.html>. * localtime.c (truncate_time): Don't bother with uintmax_t, as using it doesn't help on any known platform. * private.h (UINTMAX_MAX): Remove. --- localtime.c | 21 +++++++++------------ private.h | 6 ------ 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/localtime.c b/localtime.c index 353c73b..3814a6a 100644 --- a/localtime.c +++ b/localtime.c @@ -1267,20 +1267,17 @@ truncate_time(time_t t) ** INTMAX_MAX is converted to a larger time_t value before it ** is compared. ** - ** On all platforms that we know of, if a time value is - ** outside intmax_t/uintmax_t range, then it is an integer so we can - ** simply return it. There's no simple, portable way to check this. + ** On all platforms that we know of (1) it is safe to compare + ** INTMAX_MIN and INTMAX_MAX to floating-point values without + ** worrying about undefined behavior due to floating-point + ** overflow on conversion, and (2) any time_t value outside + ** intmax_t range is an integer so we can simply return it. + ** We know of no simple, portable way to check these assumptions. ** If you know of a counterexample platform, please report a bug. */ - if (!TYPE_INTEGRAL(time_t)) { - if (INTMAX_MIN < t && t < INTMAX_MAX) { - intmax_t i = t; - return i; - } - if (0 <= t && t < UINTMAX_MAX) { - uintmax_t i = t; - return i; - } + if (!TYPE_INTEGRAL(time_t) && INTMAX_MIN < t && t < INTMAX_MAX) { + intmax_t i = t; + return i; } return t; diff --git a/private.h b/private.h index f00a926..bf6bad2 100644 --- a/private.h +++ b/private.h @@ -186,15 +186,9 @@ typedef long intmax_t; # if defined ULLONG_MAX || defined __LONG_LONG_MAX__ typedef unsigned long long uintmax_t; # define PRIuMAX "llu" -# ifdef ULLONG_MAX -# define UINTMAX_MAX ULLONG_MAX -# else -# define UINTMAX_MAX (__LONG_LONG_MAX__ * 2ULL + 1) -# endif # else typedef unsigned long uintmax_t; # define PRIuMAX "lu" -# define UINTMAX_MAX ULONG_MAX # endif #endif -- 1.8.1.2
participants (6)
-
Alan Barrett -
Alois Treindl -
Arthur David Olson -
Clive D.W. Feather -
Paul Eggert -
Paul Goyette