This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 1/1] Y2038: add function __difftime64
On 06/20/2018 05:14 AM, Albert ARIBAUD (3ADEV) wrote:
This change batch creates a __difftime64 compatible with 64-bit time,
and makes difftime either an alias of it or a 32-bit wrapper around it.
Since no glibc code calls difftime, can we assume that a later patch
will change will make __difftime64 and difftime both available to user
code? I'm not getting the big picture here.
Which public development repository and branch are you using? I see lots
of glibc branches at sourceware.org named origin/aaribaud/y2038* but
none of them seem to correspond that the patches you're sending. If I
could see the current state of the entire set of patches you've
developed, that would save time reviewing.
We could make it return a long double
It's not just that existing programs expect difftime to return 'double';
it's also that C11 and POSIX both require it to return 'double'.
2. The 64-bit time implementation was obtained by duplicating the
original 32-bit code then simplifying the source code based on
the knowledge that __time64_t is a 64-bit signed integer
This sort of simplification won't be possible in Gnulib, where
__time64_t will be an alias of time_t and therefore could be an unsigned
type. So let's not do that simplification. (Gnulib-using programs will
always ask for 64-bit time_t if that is an option and 32-bit is the
default, as the 32-bit default is silly if you have source code.)
It is OK to simplify difftime.c based on the assumption that time_t is
an integer type. Glibc difftime.c was written back when POSIX allowed
time_t to be floating-point, and some ancient implementations did that.
This was widely regarded to be a mistake, POSIX no longer allows
floating-point time_t and we don't need to worry about those old
implementations. However, this simplification should be done as a
separate patch (see first attachment).
- in the difftime64 function, removal of code which handled
time bitsize smaller than or equal to that of a double matissa
(as __time64_t has more bits than a double mantissa can hold)
This simplification is not possible in Gnulib either, as it's not
portable to assume that time_t has more bits than a double fraction can
hold. (They're fractions, not mantissas, by the way; mantissas are for
logarithms not for floating-point.)
-static double
-subtract (time_t time1, time_t time0)
+static double subtract (__time64_t time1, __time64_t time0)
Don't change indentation in cases like this; just stick to the glibc style.
{
- if (! TYPE_SIGNED (time_t))
- return time1 - time0;
Let's leave that TYPE_SIGNED test in, for the benefit of non-glibc uses
where time_t is unsigned.
- if (TYPE_BITS (time_t) <= DBL_MANT_DIG
- || (TYPE_FLOATING (time_t) && sizeof (time_t) < sizeof (long double)))
- return (double) time1 - (double) time0;
Likewise, this needs to stay in, for portability.
The patch needs a better commit message, as the commit message doesn't
say what exactly changed and has too much unnecessary talk about things
we aren't doing. Commit messages should focus on what actually changed;
the meat of any comments explaining the code should be in the code.
Attached is a proposed pair of (untested) patches that should reflect
the above comments. What do you think?
PS. The mktime patches you sent have more problems like this. But that's
a bigger nut to crack, and let's get difftime done first.
>From f56173e42838ebe7eb00168ac401d52fe6898296 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 20 Jun 2018 10:58:09 -0700
Subject: [PATCH 1/2] difftime: do not worry about floating-point time_t
POSIX no longer allows this, and no implementations have it.
* time/difftime.c (TYPE_FLOATING): Remove. All uses removed.
---
ChangeLog | 6 ++++++
time/difftime.c | 9 +++------
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index fd6e171be2..0fda5ff2df 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2018-06-20 Paul Eggert <eggert@cs.ucla.edu>
+
+ difftime: do not worry about floating-point time_t
+ POSIX no longer allows this, and no implementations have it.
+ * time/difftime.c (TYPE_FLOATING): Remove. All uses removed.
+
2018-06-18 Joseph Myers <joseph@codesourcery.com>
* sysdeps/unix/sysv/linux/alpha/bits/shm.h [__USE_MISC]
diff --git a/time/difftime.c b/time/difftime.c
index 7c5dd9898b..7727f5cdb5 100644
--- a/time/difftime.c
+++ b/time/difftime.c
@@ -24,11 +24,9 @@
#include <stdint.h>
#define TYPE_BITS(type) (sizeof (type) * CHAR_BIT)
-#define TYPE_FLOATING(type) ((type) 0.5 == 0.5)
#define TYPE_SIGNED(type) ((type) -1 < 0)
-/* Return the difference between TIME1 and TIME0, where TIME0 <= TIME1.
- time_t is known to be an integer type. */
+/* Return the difference between TIME1 and TIME0, where TIME0 <= TIME1. */
static double
subtract (time_t time1, time_t time0)
@@ -104,13 +102,12 @@ __difftime (time_t time1, time_t time0)
/* Convert to double and then subtract if no double-rounding error could
result. */
- if (TYPE_BITS (time_t) <= DBL_MANT_DIG
- || (TYPE_FLOATING (time_t) && sizeof (time_t) < sizeof (long double)))
+ if (TYPE_BITS (time_t) <= DBL_MANT_DIG)
return (double) time1 - (double) time0;
/* Likewise for long double. */
- if (TYPE_BITS (time_t) <= LDBL_MANT_DIG || TYPE_FLOATING (time_t))
+ if (TYPE_BITS (time_t) <= LDBL_MANT_DIG)
return (long double) time1 - (long double) time0;
/* Subtract the smaller integer from the larger, convert the difference to
--
2.17.1
>From b6120a13a752335bc1c60d5d22a1cdb0b4403a7a Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 20 Jun 2018 11:37:54 -0700
Subject: [PATCH 2/2] difftime: new __time64_t variant
* include/time.h (__difftime64): New macro or function decl.
* time/difftime.c: Include "time-internal.h" if not glibc, for
outside-glibc uses.
(subtract, __difftime64) Use __time64_t, not time_t.
(__difftime) [__TIMESIZAE != 64]: Turn into a wrapper function
for __difftime64, a new function that contains the old body
of __difftime.
---
include/time.h | 6 ++++++
time/difftime.c | 36 ++++++++++++++++++++++++++----------
2 files changed, 32 insertions(+), 10 deletions(-)
diff --git a/include/time.h b/include/time.h
index 5afb12305e..63cc855749 100644
--- a/include/time.h
+++ b/include/time.h
@@ -125,6 +125,12 @@ extern char * __strptime_internal (const char *rp, const char *fmt,
struct tm *tm, void *statep,
locale_t locparam) attribute_hidden;
+#if __TIMESIZE == 64
+# define __difftime64 __difftime
+#else
+extern double __difftime64 (__time64_t time1, __time64_t time0);
+#endif
+
extern double __difftime (time_t time1, time_t time0);
/* Use in the clock_* functions. Size of the field representing the
diff --git a/time/difftime.c b/time/difftime.c
index 7727f5cdb5..4e765de624 100644
--- a/time/difftime.c
+++ b/time/difftime.c
@@ -23,19 +23,23 @@
#include <float.h>
#include <stdint.h>
+#ifndef _LIBC
+# include "time-internal.h"
+#endif
+
#define TYPE_BITS(type) (sizeof (type) * CHAR_BIT)
#define TYPE_SIGNED(type) ((type) -1 < 0)
/* Return the difference between TIME1 and TIME0, where TIME0 <= TIME1. */
static double
-subtract (time_t time1, time_t time0)
+subtract (__time64_t time1, __time64_t time0)
{
- if (! TYPE_SIGNED (time_t))
+ if (! TYPE_SIGNED (__time64_t))
return time1 - time0;
else
{
- /* Optimize the common special cases where time_t
+ /* Optimize the common special cases where __time64_t
can be converted to uintmax_t without losing information. */
uintmax_t dt = (uintmax_t) time1 - (uintmax_t) time0;
double delta = dt;
@@ -43,7 +47,7 @@ subtract (time_t time1, time_t time0)
if (UINTMAX_MAX / 2 < INTMAX_MAX)
{
/* This is a rare host where uintmax_t has padding bits, and possibly
- information was lost when converting time_t to uintmax_t.
+ information was lost when converting __time64_t to uintmax_t.
Check for overflow by comparing dt/2 to (time1/2 - time0/2).
Overflow occurred if they differ by more than a small slop.
Thanks to Clive D.W. Feather for detailed technical advice about
@@ -74,9 +78,9 @@ subtract (time_t time1, time_t time0)
1 is unsigned in C, so it need not be compared to zero. */
uintmax_t hdt = dt / 2;
- time_t ht1 = time1 / 2;
- time_t ht0 = time0 / 2;
- time_t dht = ht1 - ht0;
+ __time64_t ht1 = time1 / 2;
+ __time64_t ht0 = time0 / 2;
+ __time64_t dht = ht1 - ht0;
if (2 < dht - hdt + 1)
{
@@ -97,17 +101,17 @@ subtract (time_t time1, time_t time0)
/* Return the difference between TIME1 and TIME0. */
double
-__difftime (time_t time1, time_t time0)
+__difftime64 (__time64_t time1, __time64_t time0)
{
/* Convert to double and then subtract if no double-rounding error could
result. */
- if (TYPE_BITS (time_t) <= DBL_MANT_DIG)
+ if (TYPE_BITS (__time64_t) <= DBL_MANT_DIG)
return (double) time1 - (double) time0;
/* Likewise for long double. */
- if (TYPE_BITS (time_t) <= LDBL_MANT_DIG)
+ if (TYPE_BITS (__time64_t) <= LDBL_MANT_DIG)
return (long double) time1 - (long double) time0;
/* Subtract the smaller integer from the larger, convert the difference to
@@ -115,4 +119,16 @@ __difftime (time_t time1, time_t time0)
return time1 < time0 ? - subtract (time0, time1) : subtract (time1, time0);
}
+
+#if __TIMESIZE != 64
+
+/* Return the difference between TIME1 and TIME0. */
+double
+__difftime (time_t time1, time_t time0)
+{
+ return __difftime64 (time1, time0);
+}
+
+#endif
+
strong_alias (__difftime, difftime)
--
2.17.1