Message ID | 20210125130308.32242-1-lukma@denx.de |
---|---|
State | Superseded |
Headers | show |
Series | tst: Provide Y2038 tests for mktime (tst-mktime4.c) | expand |
error: tst-mktime4.c:53: *** mktime returned -3600 (expected 0) error: 1 test failures Andreas.
Hi Andreas, > error: tst-mktime4.c:53: *** mktime returned -3600 (expected 0) > > error: 1 test failures > Are you sure that this is correct? The -3600 would be from tst-difftime.c > Andreas. > Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On Jan 25 2021, Lukasz Majewski wrote: > Hi Andreas, > >> error: tst-mktime4.c:53: *** mktime returned -3600 (expected 0) >> >> error: 1 test failures >> > > Are you sure that this is correct? The -3600 would be from > tst-difftime.c That's literally the output of your test. Andreas.
On Mon, 25 Jan 2021, Lukasz Majewski wrote: > + /* Check that mktime(1970-01-01 00:00:00) returns 0. */ > + test_mktime_helper (&tm0, 0); > + > + /* Check that mktime(2038-01-19 03:14:07) returns 0x7FFFFFFF. */ > + test_mktime_helper (&tmY2038, 0x7fffffff); > + > + /* Check that mktime(2038-01-19 03:14:08) returns 0x80000000 > + (time_t overflow). */ > + if (sizeof (time_t) > 4) > + { > + tmY2038.tm_sec++; > + test_mktime_helper (&tmY2038, 0x80000000); > + } > + else > + FAIL_UNSUPPORTED ("32-bit time_t"); FAIL_UNSUPPORTED causes the whole test to report an UNSUPPORTED status. If a test has a mixture of assertions that work with 32-bit time and assertions that only work for 64-bit time, I think those should be split into separate tests so that those tests that make sense with 32-bit time still produce results in that case.
On 1/25/21 5:03 AM, Lukasz Majewski wrote: > + /* Set time zone for the test. */ > + TEST_VERIFY_EXIT (setenv ("TZ", "Europe/Warsaw", 1) == 0); > + tzset (); Your test data assume UTC, so why set the time zone to Warsaw's? Also, there's no need to call tzset; mktime is supposed to do that. I suggest also testing 2**32 - 1 and 2**32, when using 64-bit time_t.
* Paul Eggert: > On 1/25/21 5:03 AM, Lukasz Majewski wrote: > >> + /* Set time zone for the test. */ >> + TEST_VERIFY_EXIT (setenv ("TZ", "Europe/Warsaw", 1) == 0); >> + tzset (); > > Your test data assume UTC, so why set the time zone to Warsaw's? > > Also, there's no need to call tzset; mktime is supposed to do that. > > I suggest also testing 2**32 - 1 and 2**32, when using 64-bit time_t. Furthermore, if the test can't use UTC for some reason, please add a custom time zone for it, so that it does not depend on real tzdata files for future dates, which can change. Thanks, Florian
Hi Joseph, > On Mon, 25 Jan 2021, Lukasz Majewski wrote: > > > + /* Check that mktime(1970-01-01 00:00:00) returns 0. */ > > + test_mktime_helper (&tm0, 0); > > + > > + /* Check that mktime(2038-01-19 03:14:07) returns 0x7FFFFFFF. */ > > + test_mktime_helper (&tmY2038, 0x7fffffff); > > + > > + /* Check that mktime(2038-01-19 03:14:08) returns 0x80000000 > > + (time_t overflow). */ > > + if (sizeof (time_t) > 4) > > + { > > + tmY2038.tm_sec++; > > + test_mktime_helper (&tmY2038, 0x80000000); > > + } > > + else > > + FAIL_UNSUPPORTED ("32-bit time_t"); > > FAIL_UNSUPPORTED causes the whole test to report an UNSUPPORTED > status. If a test has a mixture of assertions that work with 32-bit > time and assertions that only work for 64-bit time, I think those > should be split into separate tests so that those tests that make > sense with 32-bit time still produce results in that case. > The idea behind this FAIL_UNSUPPORTED when sizeof(time_t) <= 4 is to: 1. On 32 bit ports - check correct operation of mktime for 32 bit time_t. If those checks fail, then we fail the test. Tests which require 64 bit size of time_t will not be supported until -D_TIME_BITS=64 is supported in glibc. 2. On 64 bit ports - the test will either end without errors or fail. 3. I do have corresponding test - tst-mktime4-y2038.c [1] which only has following lines: #define _TIME_BITS 64 #define _FILE_OFFSET_BITS 64 #include "tst-mktime4.c" Which will either pass or fail when 64 bit time_t support is added on ports with __WORDSIZE=32 && __TIMESIZE != 64. However, there is no point to send tst-mktime4-y2038.c until the above feature is supported. Links: [1] - https://github.com/lmajewski/y2038_glibc/commits/y2038_edge Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On Mon, 25 Jan 2021, Lukasz Majewski wrote: > 1. On 32 bit ports - check correct operation of mktime for 32 bit > time_t. If those checks fail, then we fail the test. But if those checks pass, the test will end with UNSUPPORTED rather than PASS. If there is anything useful to verify on 32-bit ports, the test should end with PASS if that verification succeeds and FAIL if that verification fails - not with a choice of UNSUPPORTED and FAIL.
Hi Florian, > * Paul Eggert: > > > On 1/25/21 5:03 AM, Lukasz Majewski wrote: > > > >> + /* Set time zone for the test. */ > >> + TEST_VERIFY_EXIT (setenv ("TZ", "Europe/Warsaw", 1) == 0); > >> + tzset (); > > > > Your test data assume UTC, so why set the time zone to Warsaw's? I thought that I do need to explicitly set TZ here (either "TZ=UTC" or "TZ=Europe/Warsaw"). When I poke on the manuals (like [1]) - it is all written that by default (when TZ is not set) the UTC is used. Hence, my question - why cannot I just check if TZ is set and if not set it to default "TZ=UTC" ? In that way I would have reproductible setup no matter if I run this test on HOST or TARGET (VM) machines. Another question - why in ./time/tst-y2039.c the setenv ("TZ", "PST8PDT,M3.2.0,M11.1.0", 1) is set? According to [2] the PST8PDT is deprecated. Why the UTC cannot be just used? > > > > Also, there's no need to call tzset; mktime is supposed to do that. Yes. Correct __tzset() is called in __mktime64() implementation. > > > > I suggest also testing 2**32 - 1 and 2**32, when using 64-bit > > time_t. Ok. > > Furthermore, if the test can't use UTC for some reason, please add a > custom time zone for it, so that it does not depend on real tzdata > files for future dates, which can change. Sorry, but I'm confused - how shall I evaluate if I cannot use UTC for some reason? As fair as I can tell the UTC is a default fallback (and is assumed) for several time related calls [2]. Could you point me to some test case, which I could use as an example? For example in ./time/tst-y2039.c the TZ is set to some arbitrary value to have some non UTC (i.e. default) TZ, so the test will be more reliable (detached from default TZ). Is there any issue with using "TZ=UTC" for Y2038 related tests? Links: [1] - https://linux.die.net/man/3/mktime [2] - https://en.wikipedia.org/wiki/List_of_tz_database_time_zones > > Thanks, > Florian Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Hi Joseph, > On Mon, 25 Jan 2021, Lukasz Majewski wrote: > > > 1. On 32 bit ports - check correct operation of mktime for 32 bit > > time_t. If those checks fail, then we fail the test. > > But if those checks pass, the test will end with UNSUPPORTED rather > than PASS. If there is anything useful to verify on 32-bit ports, > the test should end with PASS if that verification succeeds and FAIL > if that verification fails - not with a choice of UNSUPPORTED and > FAIL. > Please correct me if I'm wrong. Then the mktime4 test shall be as follows: 1. tst-mktime4-32time.c /* mktime tests which can be safely run with 32 bit time_t */ test_mktime_helper (&tm0, 0); test_mktime_helper (&tmY2038, 0x7fffffff); 2. tst-mktime4-64time.c if (sizeof (time_t) > 4) { Reuse code from tst-mktime4-32time.c and add extra: test_mktime_helper (&tmY2038, 0x80000000); } else FAIL_UNSUPPORTED ("32-bit time_t"); 3. tst-mktime4-y2038.c #define _TIME_BITS 64 #define _FILE_OFFSET_BITS 64 #include "tst-mktime4-64time.c" When we got -D_TIME_BITS=64 support in glibc accepted. Above use cases shall cover all use cases. Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On 1/25/21 2:28 PM, Lukasz Majewski wrote: > I thought that I do need to explicitly set TZ here (either "TZ=UTC" or > "TZ=Europe/Warsaw"). You do if you want reproducible results with mktime, yes. For glibc, perhaps the GNU extension "TZ=:" is the best, as it's the only way in glibc to guarantee that you're using UTC without leap seconds. (This whole area is a bit messy and undocumented in glibc, unfortunately.) If the glibc test environment is invariably set up without leap-second support (is it? I don't know) you needn't worry about leap seconds and so can use the POSIX-standard "TZ=UTC0". The trailing "0" is required. > When I poke on the manuals (like [1]) - it is all written that by > default (when TZ is not set) the UTC is used. I don't see where [1] says that. Anyway, that's incorrect; if TZ is unset, a system-specific default is used. > Hence, my question - why cannot I just check if TZ is set and if not > set it to default "TZ=UTC" ? "TZ=UTC" assumes tzdb is installed and that leap seconds are not in use. Are these assumptions for testing glibc? And an unset TZ means use the system-supplied default; can we assume that's UTC when testing glibc? > Another question - why in ./time/tst-y2039.c the > setenv ("TZ", "PST8PDT,M3.2.0,M11.1.0", 1) is set? > According to [2] the PST8PDT is deprecated. Why the UTC cannot be just > used? setenv ("TZ", "PST8PDT", 1) is deprecated but setenv ("TZ", "PST8PDT,M3.2.0,M11.1.0", 1) is not; it's standardized by POSIX. See: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03 and look for "TZ". Also, I would not rely on Wikipedia for this stuff; it has too many errors. I don't edit Wikipedia and probably wouldn't have time to fix its timezone problems even if I did. > For example in ./time/tst-y2039.c the TZ is set to some arbitrary value > to have some non UTC (i.e. default) TZ, so the test will be more > reliable (detached from default TZ). If memory serves, that particular bug occurred only in non-UTC environments, which is why that test had to use something other than UTC. > [1] - https://linux.die.net/man/3/mktime > [2] - https://en.wikipedia.org/wiki/List_of_tz_database_time_zones
diff --git a/time/Makefile b/time/Makefile index 7de2ce0196..f214d001f5 100644 --- a/time/Makefile +++ b/time/Makefile @@ -51,7 +51,7 @@ tests := test_time clocktest tst-posixtz tst-strptime tst_wcsftime \ tst-clock tst-clock2 tst-clock_nanosleep tst-cpuclock1 \ tst-adjtime tst-clock-y2038 tst-clock2-y2038 \ tst-cpuclock1-y2038 tst-clock_nanosleep-y2038 tst-clock_settime \ - tst-clock_adjtime tst-ctime tst-difftime + tst-clock_adjtime tst-ctime tst-difftime tst-mktime4 include ../Rules diff --git a/time/tst-mktime4.c b/time/tst-mktime4.c new file mode 100644 index 0000000000..468ca24174 --- /dev/null +++ b/time/tst-mktime4.c @@ -0,0 +1,82 @@ +/* Test for mktime + Copyright (C) 2021 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <time.h> +#include <stdlib.h> +#include <support/check.h> + +struct tm tm0 = { + .tm_year = 70, + .tm_mon = 0, + .tm_mday = 1, + .tm_hour = 0, + .tm_min = 0, + .tm_sec = 0, + .tm_wday = 4, + .tm_yday = 0 +}; + +struct tm tmY2038 = { + .tm_year = 138, + .tm_mon = 0, + .tm_mday = 19, + .tm_hour = 3, + .tm_min = 14, + .tm_sec = 7, + .tm_wday = 2, + .tm_yday = 18 +}; + +static +void test_mktime_helper (struct tm *tm, time_t exp_val) +{ + time_t result = mktime (tm); + if (result == -1) + FAIL_EXIT1 ("*** mktime failed: %m"); + + if (result != exp_val) + FAIL_EXIT1 ("*** mktime returned %ld (expected %ld)\n", result, exp_val); +} + +static int +do_test (void) +{ + /* Set time zone for the test. */ + TEST_VERIFY_EXIT (setenv ("TZ", "Europe/Warsaw", 1) == 0); + tzset (); + + /* Check that mktime(1970-01-01 00:00:00) returns 0. */ + test_mktime_helper (&tm0, 0); + + /* Check that mktime(2038-01-19 03:14:07) returns 0x7FFFFFFF. */ + test_mktime_helper (&tmY2038, 0x7fffffff); + + /* Check that mktime(2038-01-19 03:14:08) returns 0x80000000 + (time_t overflow). */ + if (sizeof (time_t) > 4) + { + tmY2038.tm_sec++; + test_mktime_helper (&tmY2038, 0x80000000); + } + else + FAIL_UNSUPPORTED ("32-bit time_t"); + + return 0; +} + +#include <support/test-driver.c>