diff mbox series

[v2] tst: Provide test for difftime

Message ID 20210127124234.19540-1-lukma@denx.de
State Superseded
Delegated to: Florian Weimer
Headers show
Series [v2] tst: Provide test for difftime | expand

Commit Message

Lukasz Majewski Jan. 27, 2021, 12:42 p.m. UTC 
This change adds new test to assess difftime's functionality by
adding some arbitrary offsets to current time_t value (read via
time).

If 64 bit time_t is supported, the same procedure is applied around
the threshold of Y2038 time overflow.

---
Changes for v2:
- Remove FAIL_UNSUPPORTED() when sizeof (time_t) <= 4
---
 time/Makefile       |  2 +-
 time/tst-difftime.c | 55 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 1 deletion(-)
 create mode 100644 time/tst-difftime.c

Comments

Paul Eggert Jan. 27, 2021, 6:42 p.m. UTC | #1 
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).
Andreas Schwab Jan. 27, 2021, 6:57 p.m. UTC | #2 
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.
Lukasz Majewski Jan. 27, 2021, 9:46 p.m. UTC | #3 
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
Paul Eggert Jan. 27, 2021, 11:04 p.m. UTC | #4 
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.
Lukasz Majewski Jan. 28, 2021, 8:21 a.m. UTC | #5 
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
Lukasz Majewski Jan. 29, 2021, 2:04 p.m. UTC | #6 
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
Lukasz Majewski Feb. 3, 2021, 12:40 p.m. UTC | #7 
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
Paul Eggert Feb. 3, 2021, 6:21 p.m. UTC | #8 
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 mbox series

Patch

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>