[PATCH v2] tst: Provide test for difftime

[PATCH v2] tst: Provide test for difftime

[Lukasz Majewski]

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

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>
-- 
2.20.1

Re: [PATCH v2] tst: Provide test for difftime

[Paul Eggert]

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).

Re: [PATCH v2] tst: Provide test for difftime

[Andreas Schwab]

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.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH v2] tst: Provide test for difftime

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 617 bytes --]

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 484 bytes --]

Re: [PATCH v2] tst: Provide test for difftime

[Paul Eggert]

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.

Re: [PATCH v2] tst: Provide test for difftime

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 682 bytes --]

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] tst: Provide test for difftime

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 1063 bytes --]

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] tst: Provide test for difftime

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 1618 bytes --]

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] tst: Provide test for difftime

[Paul Eggert]

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.