Message ID | 20210127124234.19540-1-lukma@denx.de |
---|---|
State | Superseded |
Delegated to: | Florian Weimer |
Headers | show |
Series | [v2] tst: Provide test for difftime | expand |
On 1/27/21 4:42 AM, Lukasz Majewski wrote: > This change adds new test to assess difftime's functionality by > adding some arbitrary offsets to current time_t value (read via > time). That would make tests irreproducible. Why is that helpful for difftime? I suggest using only reproducible tests; this should make the code simpler anyway. > +static void > +test_difftime_helper (time_t start, time_t t1, time_t t0, double exp_val) The signature should be simpler: static void test_difftime_helper (long long t1, long long t0, double exp_val) And test_difftime_helper can test whether its arguments fit into time_t with code like this: time_t u1; if (__builtin_add_overflow (t1, 0, &u1)) return; That way, you can omit the 'sizeof (time_t) > 4' test, which is a bit dubious anyway as it wouldn't work on hypothetical hosts where char is wider than 8 bits. You can do this sort of thing in all the time_t tests, to omit all uses of sizeof (time_t).
On Jan 27 2021, Paul Eggert wrote: > dubious anyway as it wouldn't work on hypothetical hosts where char is > wider than 8 bits. This is irrelevant. Andreas.
Hi Andreas, Paul, > On Jan 27 2021, Paul Eggert wrote: > > > dubious anyway as it wouldn't work on hypothetical hosts where char > > is wider than 8 bits. > > This is irrelevant. I also think that sizeof (time_t) > 4 is more readable and simpler. Moreover, similar condition was used in ./time/tst-y2039.c test. > > 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 1/27/21 1:46 PM, Lukasz Majewski wrote: > I also think that sizeof (time_t) > 4 is more readable and simpler. > Moreover, similar condition was used in ./time/tst-y2039.c test. It would be simpler, except for the warnings from GCC. Using __builtin_add_overflow would mean we wouldn't have to worry about those warnings.
Hi Paul, > On 1/27/21 1:46 PM, Lukasz Majewski wrote: > > I also think that sizeof (time_t) > 4 is more readable and simpler. > > Moreover, similar condition was used in ./time/tst-y2039.c test. > > It would be simpler, except for the warnings from GCC. Using > __builtin_add_overflow would mean we wouldn't have to worry about > those warnings. I don't mind to use it, when other developers agree. 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
Dear Community, > Hi Paul, > > > On 1/27/21 1:46 PM, Lukasz Majewski wrote: > > > I also think that sizeof (time_t) > 4 is more readable and > > > simpler. Moreover, similar condition was used in > > > ./time/tst-y2039.c test. > > > > It would be simpler, except for the warnings from GCC. Using > > __builtin_add_overflow would mean we wouldn't have to worry about > > those warnings. > > I don't mind to use it, when other developers agree. > Any feedback on this idea? > > > > 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 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
Dear Community, > Dear Community, > > > Hi Paul, > > > > > On 1/27/21 1:46 PM, Lukasz Majewski wrote: > > > > I also think that sizeof (time_t) > 4 is more readable and > > > > simpler. Moreover, similar condition was used in > > > > ./time/tst-y2039.c test. > > > > > > It would be simpler, except for the warnings from GCC. Using > > > __builtin_add_overflow would mean we wouldn't have to worry about > > > those warnings. > > > > I don't mind to use it, when other developers agree. > > > > Any feedback on this idea? Would it be OK, if I follow the idea proposed by Paul to check if the overflow in time_t is present (with __builtin_add_overflow()) in tests, which require it? > > > > > > > > > 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 > > > > > 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 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 2/3/21 4:40 AM, Lukasz Majewski wrote: > Would it be OK, if I follow the idea proposed by Paul to check if the > overflow in time_t is present (with __builtin_add_overflow()) in tests, > which require it? I'd say yes (of course :-). I don't think anyone else cares, so I'd go ahead.
diff --git a/time/Makefile b/time/Makefile index 486fb02ecb..7de2ce0196 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-clock_adjtime tst-ctime tst-difftime include ../Rules diff --git a/time/tst-difftime.c b/time/tst-difftime.c new file mode 100644 index 0000000000..5933e82097 --- /dev/null +++ b/time/tst-difftime.c @@ -0,0 +1,55 @@ +/* Test for difftime + 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 <support/check.h> + +static void +test_difftime_helper (time_t start, time_t t1, time_t t0, double exp_val) +{ + time_t time1, time0; + + time1 = time0 = start; + time1 += t1; + time0 += t0; + + double sub = difftime (time1, time0); + if (sub != exp_val) + FAIL_EXIT1 ("*** Difftime returned %f (expected %f)\n", sub, exp_val); +} + +static int +do_test (void) +{ + /* Check if difftime works with current time. */ + test_difftime_helper (time (NULL), +1800, -1800, 3600.0); + test_difftime_helper (time (NULL), -1800, +1800, -3600.0); + + /* Check if difftime works correctly near 32 bit time_t overflow. */ + if (sizeof (time_t) > 4) + { + test_difftime_helper (0x7FFFFFFF, +1800, -1800, 3600.0); + test_difftime_helper (0x80000000, +1800, -1800, 3600.0); + test_difftime_helper (0x80000000, -1800, +1800, -3600.0); + test_difftime_helper (0x7FFFFFFF, -1800, +1800, -3600.0); + } + + return 0; +} + +#include <support/test-driver.c>