[PATCH] Change time_t to 64-bit by default

[PATCH] Change time_t to 64-bit by default

[Sebastian Huber]

In order to avoid the year 2038 problem, define time_t to a signed
integer with at least 64-bits.  The type for time_t can be forced to
long with the --enable-newlib-long-time_t configure option or with the
_USE_LONG_TIME_T system configuration define.

Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
---
 newlib/README                         |  6 ++++++
 newlib/configure                      | 26 ++++++++++++++++++++++++--
 newlib/configure.in                   | 15 +++++++++++++++
 newlib/libc/include/sys/_types.h      |  6 +++++-
 newlib/libc/include/sys/config.h      |  6 ++++++
 newlib/newlib.hin                     |  3 +++
 winsup/cygwin/include/cygwin/config.h |  1 +
 7 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/newlib/README b/newlib/README
index 78f4de846..8c97e24aa 100644
--- a/newlib/README
+++ b/newlib/README
@@ -343,6 +343,12 @@ One feature can be enabled by specifying `--enable-FEATURE=yes' or
      disables the optimization and saves size of text and stack.
      Enabled by default.
 
+`--enable-newlib-long-time_t'
+     Define time_t to long.  On platforms with a 32-bit long type, this gives
+     raise to the year 2038 problem.  The default type for time_t is a signed
+     64-bit integer on most systems.
+     Disabled by default.
+
 `--enable-multilib'
      Build many library versions.
      Enabled by default.
diff --git a/newlib/configure b/newlib/configure
index b2f0b3340..eb4b3b275 100755
--- a/newlib/configure
+++ b/newlib/configure
@@ -804,6 +804,7 @@ enable_newlib_unbuf_stream_opt
 enable_lite_exit
 enable_newlib_nano_formatted_io
 enable_newlib_retargetable_locking
+enable_newlib_long_time_t
 enable_multilib
 enable_target_optspace
 enable_malloc_debugging
@@ -1477,6 +1478,7 @@ Optional Features:
   --enable-lite-exit	enable light weight exit
   --enable-newlib-nano-formatted-io    Use nano version formatted IO
   --enable-newlib-retargetable-locking    Allow locking routines to be retargeted at link time
+  --enable-newlib-long-time_t   define time_t to long
   --enable-multilib         build many library versions (default)
   --enable-target-optspace  optimize for space
   --enable-malloc-debugging indicate malloc debugging requested
@@ -2501,6 +2503,19 @@ else
 fi
 
 
+# Check whether --enable-newlib-long-time_t was given.
+if test "${enable_newlib_long_time_t+set}" = set; then :
+  enableval=$enable_newlib_long_time_t; if test "${newlib_long_time_t+set}" != set; then
+  case "${enableval}" in
+    yes) newlib_long_time_t=yes ;;
+    no)  newlib_long_time_t=no  ;;
+    *)   as_fn_error $? "bad value ${enableval} for newlib-long-time_t option" "$LINENO" 5 ;;
+  esac
+ fi
+else
+  newlib_nano_malloc=
+fi
+
 
 # Make sure we can run config.sub.
 $SHELL "$ac_aux_dir/config.sub" sun4 >/dev/null 2>&1 ||
@@ -11807,7 +11822,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11810 "configure"
+#line 11825 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -11913,7 +11928,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11916 "configure"
+#line 11931 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -12496,6 +12511,13 @@ _ACEOF
 
 fi
 
+if test "${newlib_long_time_t}" = "yes"; then
+cat >>confdefs.h <<_ACEOF
+#define _WANT_USE_LONG_TIME_T 1
+_ACEOF
+
+fi
+
 
 if test "x${iconv_encodings}" != "x" \
    || test "x${iconv_to_encodings}" != "x" \
diff --git a/newlib/configure.in b/newlib/configure.in
index 5b86ee800..9d304817d 100644
--- a/newlib/configure.in
+++ b/newlib/configure.in
@@ -238,6 +238,17 @@ AC_ARG_ENABLE(newlib-retargetable-locking,
    *) AC_MSG_ERROR(bad value ${enableval} for newlib-retargetable-locking) ;;
  esac],[newlib_retargetable_locking=no])
 
+dnl Support --enable-newlib-long-time_t
+AC_ARG_ENABLE(newlib-long-time_t,
+[  --enable-newlib-long-time_t   define time_t to long],
+[if test "${newlib_long_time_t+set}" != set; then
+  case "${enableval}" in
+    yes) newlib_long_time_t=yes ;;
+    no)  newlib_long_time_t=no  ;;
+    *)   AC_MSG_ERROR(bad value ${enableval} for newlib-long-time_t option) ;;
+  esac
+ fi], [newlib_nano_malloc=])dnl
+
 NEWLIB_CONFIGURE(.)
 
 dnl We have to enable libtool after NEWLIB_CONFIGURE because if we try and
@@ -486,6 +497,10 @@ if test "${newlib_retargetable_locking}" = "yes"; then
 AC_DEFINE_UNQUOTED(_RETARGETABLE_LOCKING)
 fi
 
+if test "${newlib_long_time_t}" = "yes"; then
+AC_DEFINE_UNQUOTED(_WANT_USE_LONG_TIME_T)
+fi
+
 dnl
 dnl Parse --enable-newlib-iconv-encodings option argument
 dnl
diff --git a/newlib/libc/include/sys/_types.h b/newlib/libc/include/sys/_types.h
index d27979c9d..72e1dc17a 100644
--- a/newlib/libc/include/sys/_types.h
+++ b/newlib/libc/include/sys/_types.h
@@ -186,7 +186,11 @@ typedef void *_iconv_t;
 
 typedef	_CLOCK_T_	__clock_t;
 
-#define	_TIME_T_	long		/* time() */
+#if defined(_USE_LONG_TIME_T) || __LONG_MAX__ > 0x7fffffffL
+#define	_TIME_T_ long
+#else
+#define	_TIME_T_ __int_least64_t
+#endif
 typedef	_TIME_T_	__time_t;
 
 #define	_CLOCKID_T_ 	unsigned long
diff --git a/newlib/libc/include/sys/config.h b/newlib/libc/include/sys/config.h
index 8dc0c5e9d..2082dfdb1 100644
--- a/newlib/libc/include/sys/config.h
+++ b/newlib/libc/include/sys/config.h
@@ -283,6 +283,12 @@
 #endif
 #endif
 
+#ifdef _WANT_USE_LONG_TIME_T
+#ifndef _USE_LONG_TIME_T
+#define _USE_LONG_TIME_T
+#endif
+#endif
+
 /* If _MB_EXTENDED_CHARSETS_ALL is set, we want all of the extended
    charsets.  The extended charsets add a few functions and a couple
    of tables of a few K each. */
diff --git a/newlib/newlib.hin b/newlib/newlib.hin
index 45c683187..18306f293 100644
--- a/newlib/newlib.hin
+++ b/newlib/newlib.hin
@@ -90,6 +90,9 @@
 /* Define if using retargetable functions for default lock routines.  */
 #undef _RETARGETABLE_LOCKING
 
+/* Define to use type long for time_t.  */
+#undef _WANT_USE_LONG_TIME_T
+
 /*
  * Iconv encodings enabled ("to" direction)
  */
diff --git a/winsup/cygwin/include/cygwin/config.h b/winsup/cygwin/include/cygwin/config.h
index aede45f77..282637b67 100644
--- a/winsup/cygwin/include/cygwin/config.h
+++ b/winsup/cygwin/include/cygwin/config.h
@@ -79,6 +79,7 @@ extern inline struct _reent *__getreent (void)
 #define _STDIO_BSD_SEMANTICS 1
 #define __TM_GMTOFF tm_gmtoff
 #define __TM_ZONE   tm_zone
+#define _USE_LONG_TIME_T 1
 
 #if defined(__INSIDE_CYGWIN__) || defined(_COMPILING_NEWLIB)
 #define __EXPORT __declspec(dllexport)
-- 
2.12.3

Re: [PATCH] Change time_t to 64-bit by default

[Corinna Vinschen]

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

Hi Sebastian,

On Sep  7 09:20, Sebastian Huber wrote:
> In order to avoid the year 2038 problem, define time_t to a signed
> integer with at least 64-bits.  The type for time_t can be forced to
> long with the --enable-newlib-long-time_t configure option or with the
> _USE_LONG_TIME_T system configuration define.
> 
> Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
> ---
>  newlib/README                         |  6 ++++++
>  newlib/configure                      | 26 ++++++++++++++++++++++++--
>  newlib/configure.in                   | 15 +++++++++++++++
>  newlib/libc/include/sys/_types.h      |  6 +++++-
>  newlib/libc/include/sys/config.h      |  6 ++++++
>  newlib/newlib.hin                     |  3 +++
>  winsup/cygwin/include/cygwin/config.h |  1 +
>  7 files changed, 60 insertions(+), 3 deletions(-)

Your patch is fine, but aren't we missing a preliminary patch to the
time functions as discussed in
https://sourceware.org/ml/newlib/2017/msg00772.html first?


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] Change time_t to 64-bit by default

[Sebastian Huber]

On 07/09/17 14:34, Corinna Vinschen wrote:

> On Sep  7 09:20, Sebastian Huber wrote:
>> In order to avoid the year 2038 problem, define time_t to a signed
>> integer with at least 64-bits.  The type for time_t can be forced to
>> long with the --enable-newlib-long-time_t configure option or with the
>> _USE_LONG_TIME_T system configuration define.
>>
>> Signed-off-by: Sebastian Huber<sebastian.huber@embedded-brains.de>
>> ---
>>   newlib/README                         |  6 ++++++
>>   newlib/configure                      | 26 ++++++++++++++++++++++++--
>>   newlib/configure.in                   | 15 +++++++++++++++
>>   newlib/libc/include/sys/_types.h      |  6 +++++-
>>   newlib/libc/include/sys/config.h      |  6 ++++++
>>   newlib/newlib.hin                     |  3 +++
>>   winsup/cygwin/include/cygwin/config.h |  1 +
>>   7 files changed, 60 insertions(+), 3 deletions(-)
> Your patch is fine, but aren't we missing a preliminary patch to the
> time functions as discussed in
> https://sourceware.org/ml/newlib/2017/msg00772.html  first?

If I compare

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=newlib/libc/time/gmtime_r.c;h=81c7c94b1f9fe54f83c677e15c2ef6cc81509445;hb=HEAD

with

https://svnweb.freebsd.org/base/head/contrib/tzcode/stdtime/localtime.c?view=markup

or

https://android.googlesource.com/platform/bionic/+/master/libc/tzcode/localtime.c

I wonder if the Newlib version is correct in general. At least support 
for leap seconds seems to be missing.

The casts in Newlib gmtime_r() look pretty useless. I will send a patch 
with:

diff --git a/newlib/libc/time/gmtime_r.c b/newlib/libc/time/gmtime_r.c
index 81c7c94b1..6475df3ba 100644
--- a/newlib/libc/time/gmtime_r.c
+++ b/newlib/libc/time/gmtime_r.c
@@ -56,8 +56,8 @@ _DEFUN (gmtime_r, (tim_p, res),
    unsigned erayear, yearday, month, day;
    unsigned long eraday;

-  days = ((long)lcltime) / SECSPERDAY + EPOCH_ADJUSTMENT_DAYS;
-  rem = ((long)lcltime) % SECSPERDAY;
+  days = lcltime / SECSPERDAY + EPOCH_ADJUSTMENT_DAYS;
+  rem = lcltime % SECSPERDAY;
    if (rem < 0)
      {
        rem += SECSPERDAY;

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber@embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

Re: [PATCH] Change time_t to 64-bit by default

[Corinna Vinschen]

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

On Sep  7 15:00, Sebastian Huber wrote:
> On 07/09/17 14:34, Corinna Vinschen wrote:
> 
> > On Sep  7 09:20, Sebastian Huber wrote:
> > > In order to avoid the year 2038 problem, define time_t to a signed
> > > integer with at least 64-bits.  The type for time_t can be forced to
> > > long with the --enable-newlib-long-time_t configure option or with the
> > > _USE_LONG_TIME_T system configuration define.
> > > 
> > > Signed-off-by: Sebastian Huber<sebastian.huber@embedded-brains.de>
> > > ---
> > >   newlib/README                         |  6 ++++++
> > >   newlib/configure                      | 26 ++++++++++++++++++++++++--
> > >   newlib/configure.in                   | 15 +++++++++++++++
> > >   newlib/libc/include/sys/_types.h      |  6 +++++-
> > >   newlib/libc/include/sys/config.h      |  6 ++++++
> > >   newlib/newlib.hin                     |  3 +++
> > >   winsup/cygwin/include/cygwin/config.h |  1 +
> > >   7 files changed, 60 insertions(+), 3 deletions(-)
> > Your patch is fine, but aren't we missing a preliminary patch to the
> > time functions as discussed in
> > https://sourceware.org/ml/newlib/2017/msg00772.html  first?
> 
> If I compare
> 
> https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=newlib/libc/time/gmtime_r.c;h=81c7c94b1f9fe54f83c677e15c2ef6cc81509445;hb=HEAD
> 
> with
> 
> https://svnweb.freebsd.org/base/head/contrib/tzcode/stdtime/localtime.c?view=markup
> 
> or
> 
> https://android.googlesource.com/platform/bionic/+/master/libc/tzcode/localtime.c
> 
> I wonder if the Newlib version is correct in general. At least support for
> leap seconds seems to be missing.
> 
> The casts in Newlib gmtime_r() look pretty useless. I will send a patch
> with:

Thanks, feel free to push the patch from this thread after the
other one.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] Change time_t to 64-bit by default

[Freddie Chopin]

On Thu, 2017-09-07 at 15:00 +0200, Sebastian Huber wrote:
> The casts in Newlib gmtime_r() look pretty useless. I will send a
> patch 
> with:
> 
> diff --git a/newlib/libc/time/gmtime_r.c
> b/newlib/libc/time/gmtime_r.c
> index 81c7c94b1..6475df3ba 100644
> --- a/newlib/libc/time/gmtime_r.c
> +++ b/newlib/libc/time/gmtime_r.c
> @@ -56,8 +56,8 @@ _DEFUN (gmtime_r, (tim_p, res),
>     unsigned erayear, yearday, month, day;
>     unsigned long eraday;
> 
> -  days = ((long)lcltime) / SECSPERDAY + EPOCH_ADJUSTMENT_DAYS;
> -  rem = ((long)lcltime) % SECSPERDAY;
> +  days = lcltime / SECSPERDAY + EPOCH_ADJUSTMENT_DAYS;
> +  rem = lcltime % SECSPERDAY;
>     if (rem < 0)
>       {
>         rem += SECSPERDAY;

You should also replace "long days, rem;" with "time_t days, rem;".

Regards,
FCh

Re: [PATCH] Change time_t to 64-bit by default

[Sebastian Huber]

----- Freddie Chopin <freddie_chopin@op.pl> schrieb:
> On Thu, 2017-09-07 at 15:00 +0200, Sebastian Huber wrote:
> > The casts in Newlib gmtime_r() look pretty useless. I will send a
> > patch 
> > with:
> > 
> > diff --git a/newlib/libc/time/gmtime_r.c
> > b/newlib/libc/time/gmtime_r.c
> > index 81c7c94b1..6475df3ba 100644
> > --- a/newlib/libc/time/gmtime_r.c
> > +++ b/newlib/libc/time/gmtime_r.c
> > @@ -56,8 +56,8 @@ _DEFUN (gmtime_r, (tim_p, res),
> >     unsigned erayear, yearday, month, day;
> >     unsigned long eraday;
> > 
> > -  days = ((long)lcltime) / SECSPERDAY + EPOCH_ADJUSTMENT_DAYS;
> > -  rem = ((long)lcltime) % SECSPERDAY;
> > +  days = lcltime / SECSPERDAY + EPOCH_ADJUSTMENT_DAYS;
> > +  rem = lcltime % SECSPERDAY;
> >     if (rem < 0)
> >       {
> >         rem += SECSPERDAY;
> 
> You should also replace "long days, rem;" with "time_t days, rem;".

2147483647 days are roughly 5879489 years. This should be enough for most applications.

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber at embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine gesch&auml;ftliche Mitteilung im Sinne des EHUG.

Re: [PATCH] Change time_t to 64-bit by default

[Freddie Chopin]

On Thu, 2017-09-07 at 18:03 +0200, Sebastian Huber wrote:
> 2147483647 days are roughly 5879489 years. This should be enough for
> most applications.

Unless you don't use gmtime_r() for calculating current time, but for
experimenting with future dates. Such change does no harm in any case,
while expanding possible range of function inputs.

So why not?

Regards,
FCh

Re: [PATCH] Change time_t to 64-bit by default

[Brian Inglis]

On 2017-09-07 10:03, Sebastian Huber wrote:
> 
> ----- Freddie Chopin <freddie_chopin@op.pl> schrieb:
>> On Thu, 2017-09-07 at 15:00 +0200, Sebastian Huber wrote:
>>> The casts in Newlib gmtime_r() look pretty useless. I will send a
>>> patch 
>>> with:
>>>
>>> diff --git a/newlib/libc/time/gmtime_r.c
>>> b/newlib/libc/time/gmtime_r.c
>>> index 81c7c94b1..6475df3ba 100644
>>> --- a/newlib/libc/time/gmtime_r.c
>>> +++ b/newlib/libc/time/gmtime_r.c
>>> @@ -56,8 +56,8 @@ _DEFUN (gmtime_r, (tim_p, res),
>>>     unsigned erayear, yearday, month, day;
>>>     unsigned long eraday;
>>>
>>> -  days = ((long)lcltime) / SECSPERDAY + EPOCH_ADJUSTMENT_DAYS;
>>> -  rem = ((long)lcltime) % SECSPERDAY;
>>> +  days = lcltime / SECSPERDAY + EPOCH_ADJUSTMENT_DAYS;
>>> +  rem = lcltime % SECSPERDAY;
>>>     if (rem < 0)
>>>       {
>>>         rem += SECSPERDAY;
>>
>> You should also replace "long days, rem;" with "time_t days, rem;".
> 
> 2147483647 days are roughly 5879489 years. This should be enough for most applications.

The library supports tm_year from -2^31 -2147483648 to 2^31-1 2147483647 and
year from -2^31+1900 -2147481748 to 2^31-1+1900 2147485547 allowing time_t from
about -67768100536348801 to 67768036191676799.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

Re: [PATCH] Change time_t to 64-bit by default

[Brian Inglis]

On 2017-09-07 12:47, Brian Inglis wrote:
> On 2017-09-07 10:03, Sebastian Huber wrote:
>>
>> ----- Freddie Chopin <freddie_chopin@op.pl> schrieb:
>>> On Thu, 2017-09-07 at 15:00 +0200, Sebastian Huber wrote:
>>>> The casts in Newlib gmtime_r() look pretty useless. I will send a
>>>> patch 
>>>> with:
>>>>
>>>> diff --git a/newlib/libc/time/gmtime_r.c
>>>> b/newlib/libc/time/gmtime_r.c
>>>> index 81c7c94b1..6475df3ba 100644
>>>> --- a/newlib/libc/time/gmtime_r.c
>>>> +++ b/newlib/libc/time/gmtime_r.c
>>>> @@ -56,8 +56,8 @@ _DEFUN (gmtime_r, (tim_p, res),
>>>>     unsigned erayear, yearday, month, day;
>>>>     unsigned long eraday;
>>>>
>>>> -  days = ((long)lcltime) / SECSPERDAY + EPOCH_ADJUSTMENT_DAYS;
>>>> -  rem = ((long)lcltime) % SECSPERDAY;
>>>> +  days = lcltime / SECSPERDAY + EPOCH_ADJUSTMENT_DAYS;
>>>> +  rem = lcltime % SECSPERDAY;
>>>>     if (rem < 0)
>>>>       {
>>>>         rem += SECSPERDAY;
>>>
>>> You should also replace "long days, rem;" with "time_t days, rem;".
>>
>> 2147483647 days are roughly 5879489 years. This should be enough for most applications.
> 
> The library supports tm_year from -2^31 -2147483648 to 2^31-1 2147483647 and
> year from -2^31+1900 -2147481748 to 2^31-1+1900 2147485547 allowing time_t from
> about -67768100536348801 to 67768036191676799.

Should int ... year be changed to long year or time_t year to be able to handle
tm_year + 1900 mentioned above?

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

Re: [PATCH] Change time_t to 64-bit by default

[Freddie Chopin]

On Thu, 2017-09-07 at 14:59 -0600, Brian Inglis wrote:
> On 2017-09-07 12:47, Brian Inglis wrote:
> > On 2017-09-07 10:03, Sebastian Huber wrote:
> > > 
> > > ----- Freddie Chopin <freddie_chopin@op.pl> schrieb:
> > > > On Thu, 2017-09-07 at 15:00 +0200, Sebastian Huber wrote:
> > > > > The casts in Newlib gmtime_r() look pretty useless. I will
> > > > > send a
> > > > > patch 
> > > > > with:
> > > > > 
> > > > > diff --git a/newlib/libc/time/gmtime_r.c
> > > > > b/newlib/libc/time/gmtime_r.c
> > > > > index 81c7c94b1..6475df3ba 100644
> > > > > --- a/newlib/libc/time/gmtime_r.c
> > > > > +++ b/newlib/libc/time/gmtime_r.c
> > > > > @@ -56,8 +56,8 @@ _DEFUN (gmtime_r, (tim_p, res),
> > > > >     unsigned erayear, yearday, month, day;
> > > > >     unsigned long eraday;
> > > > > 
> > > > > -  days = ((long)lcltime) / SECSPERDAY +
> > > > > EPOCH_ADJUSTMENT_DAYS;
> > > > > -  rem = ((long)lcltime) % SECSPERDAY;
> > > > > +  days = lcltime / SECSPERDAY + EPOCH_ADJUSTMENT_DAYS;
> > > > > +  rem = lcltime % SECSPERDAY;
> > > > >     if (rem < 0)
> > > > >       {
> > > > >         rem += SECSPERDAY;
> > > > 
> > > > You should also replace "long days, rem;" with "time_t days,
> > > > rem;".
> > > 
> > > 2147483647 days are roughly 5879489 years. This should be enough
> > > for most applications.
> > 
> > The library supports tm_year from -2^31 -2147483648 to 2^31-1
> > 2147483647 and
> > year from -2^31+1900 -2147481748 to 2^31-1+1900 2147485547 allowing
> > time_t from
> > about -67768100536348801 to 67768036191676799.
> 
> Should int ... year be changed to long year or time_t year to be able
> to handle
> tm_year + 1900 mentioned above?

Probably, but there's a high probability that there are more things
that would need changing - for example without additional casts this:

year = ADJUSTED_EPOCH_YEAR + erayear + era * YEARS_PER_ERA + (month <= 1);

would probably be calculated as "int" anyway.

Regards,
FCh

Re: [PATCH] Change time_t to 64-bit by default

[Brian Inglis]

On 2017-09-07 15:36, Freddie Chopin wrote:
> On Thu, 2017-09-07 at 14:59 -0600, Brian Inglis wrote:
>> On 2017-09-07 12:47, Brian Inglis wrote:
>>> On 2017-09-07 10:03, Sebastian Huber wrote:
>>>>
>>>> ----- Freddie Chopin <freddie_chopin@op.pl> schrieb:
>>>>> On Thu, 2017-09-07 at 15:00 +0200, Sebastian Huber wrote:
>>>>>> The casts in Newlib gmtime_r() look pretty useless. I will
>>>>>> send a
>>>>>> patch 
>>>>>> with:
>>>>>>
>>>>>> diff --git a/newlib/libc/time/gmtime_r.c
>>>>>> b/newlib/libc/time/gmtime_r.c
>>>>>> index 81c7c94b1..6475df3ba 100644
>>>>>> --- a/newlib/libc/time/gmtime_r.c
>>>>>> +++ b/newlib/libc/time/gmtime_r.c
>>>>>> @@ -56,8 +56,8 @@ _DEFUN (gmtime_r, (tim_p, res),
>>>>>>     unsigned erayear, yearday, month, day;
>>>>>>     unsigned long eraday;
>>>>>>
>>>>>> -  days = ((long)lcltime) / SECSPERDAY +
>>>>>> EPOCH_ADJUSTMENT_DAYS;
>>>>>> -  rem = ((long)lcltime) % SECSPERDAY;
>>>>>> +  days = lcltime / SECSPERDAY + EPOCH_ADJUSTMENT_DAYS;
>>>>>> +  rem = lcltime % SECSPERDAY;
>>>>>>     if (rem < 0)
>>>>>>       {
>>>>>>         rem += SECSPERDAY;
>>>>>
>>>>> You should also replace "long days, rem;" with "time_t days,
>>>>> rem;".
>>>>
>>>> 2147483647 days are roughly 5879489 years. This should be enough
>>>> for most applications.
>>>
>>> The library supports tm_year from -2^31 -2147483648 to 2^31-1
>>> 2147483647 and
>>> year from -2^31+1900 -2147481748 to 2^31-1+1900 2147485547 allowing
>>> time_t from
>>> about -67768100536348801 to 67768036191676799.
>>
>> Should int ... year be changed to long year or time_t year to be able
>> to handle
>> tm_year + 1900 mentioned above?
> 
> Probably, but there's a high probability that there are more things
> that would need changing - for example without additional casts this:
> 
> year = ADJUSTED_EPOCH_YEAR + erayear + era * YEARS_PER_ERA + (month <= 1);
> 
> would probably be calculated as "int" anyway.

Those are the problems I'm looking to patch.
The question is, going forward, should we use long for bigger than int values as
usual, or use time_t instead, to consistently avoid long, in these time functions.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

Re: [PATCH] Change time_t to 64-bit by default

[Freddie Chopin]

On Thu, 2017-09-07 at 15:58 -0600, Brian Inglis wrote:
> The question is, going forward, should we use long for bigger than
> int values as
> usual, or use time_t instead, to consistently avoid long, in these
> time functions.

I think we should go with time_t, which will be 32-bit or 64-bit long,
depending on what was chosen during newlib compilation.

Regards,
FCh

Re: [PATCH] Change time_t to 64-bit by default

[Corinna Vinschen]

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

On Sep  8 07:40, Freddie Chopin wrote:
> On Thu, 2017-09-07 at 15:58 -0600, Brian Inglis wrote:
> > The question is, going forward, should we use long for bigger than
> > int values as
> > usual, or use time_t instead, to consistently avoid long, in these
> > time functions.
> 
> I think we should go with time_t, which will be 32-bit or 64-bit long,
> depending on what was chosen during newlib compilation.

While that's nice and all, two points to keep in mind:

* 64 bit computations are costly on 32 bit (or even 16 bit) platforms
  and should be avoided if overkill isn't your goal.

* days, months, years, etc, are *all* of type int in struct tm,
  per POSIX.  Adding 64 bit ops just to fold stuff back into ints
  might only make marginal sense.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] Change time_t to 64-bit by default

[Brian Inglis]

On 2017-09-08 02:18, Corinna Vinschen wrote:
> On Sep  8 07:40, Freddie Chopin wrote:
>> On Thu, 2017-09-07 at 15:58 -0600, Brian Inglis wrote:
>>> The question is, going forward, should we use long for bigger than
>>> int values as
>>> usual, or use time_t instead, to consistently avoid long, in these
>>> time functions.
>>
>> I think we should go with time_t, which will be 32-bit or 64-bit long,
>> depending on what was chosen during newlib compilation.
> 
> While that's nice and all, two points to keep in mind:
> 
> * 64 bit computations are costly on 32 bit (or even 16 bit) platforms
>   and should be avoided if overkill isn't your goal.
> 
> * days, months, years, etc, are *all* of type int in struct tm,
>   per POSIX.  Adding 64 bit ops just to fold stuff back into ints
>   might only make marginal sense.

Max year == MAX_INT + 1900 == 2^31-1 + 1900 == 2147485547 on 32 bit platforms,
if you want to avoid arbitrary limitations, and be as (Linux) compatible as
possible, which the existing library tries to do, but there are various issues
to be fixed. There are places where int, unsigned, long, and unsigned long are
used which may or may not work depending on time_t, int, and long sizes.

Is long bigger than int on all newlib platforms? What is bigger than int?

Using time_t as required makes temps and variables big enough to avoid overflow
for seconds, days, and years counts, without penalizing 32 bit time_t platforms.

For I/O conversion of 64 bit time_t max year on 32 bit platforms long long is
required, it may also be used with 32 bit time_t to avoid conditionals, and it
imposes no extra overhead on 64 bit platforms.

Everything depends on how many conditionals we want to use to minimize the data
and code space and time spent at runtime and for development.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

Re: [PATCH] Change time_t to 64-bit by default

[Corinna Vinschen]

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

On Sep  8 08:46, Brian Inglis wrote:
> On 2017-09-08 02:18, Corinna Vinschen wrote:
> > On Sep  8 07:40, Freddie Chopin wrote:
> >> On Thu, 2017-09-07 at 15:58 -0600, Brian Inglis wrote:
> >>> The question is, going forward, should we use long for bigger than
> >>> int values as
> >>> usual, or use time_t instead, to consistently avoid long, in these
> >>> time functions.
> >>
> >> I think we should go with time_t, which will be 32-bit or 64-bit long,
> >> depending on what was chosen during newlib compilation.
> > 
> > While that's nice and all, two points to keep in mind:
> > 
> > * 64 bit computations are costly on 32 bit (or even 16 bit) platforms
> >   and should be avoided if overkill isn't your goal.
> > 
> > * days, months, years, etc, are *all* of type int in struct tm,
> >   per POSIX.  Adding 64 bit ops just to fold stuff back into ints
> >   might only make marginal sense.
> 
> Max year == MAX_INT + 1900 == 2^31-1 + 1900 == 2147485547 on 32 bit platforms,

You wrote this before but it sounds like a pretty academic problem.

> if you want to avoid arbitrary limitations, and be as (Linux) compatible as
> possible, which the existing library tries to do, but there are various issues
> to be fixed. There are places where int, unsigned, long, and unsigned long are
> used which may or may not work depending on time_t, int, and long sizes.

Glibc compat is nice, and as far as Cygwin is concerned I'm not
concerned for size.  However, newlib strives to small footprints
given that many embedded targets are using it.  This should be kept
in mind.

> 
> Is long bigger than int on all newlib platforms? What is bigger than int?
> 
> Using time_t as required makes temps and variables big enough to avoid overflow
> for seconds, days, and years counts, without penalizing 32 bit time_t platforms.

Rather than int, long, long long you can use explicit sized ints
like int16_t, int32_t, uint64_t.

> For I/O conversion of 64 bit time_t max year on 32 bit platforms long long is
> required, it may also be used with 32 bit time_t to avoid conditionals, and it
> imposes no extra overhead on 64 bit platforms.
> 
> Everything depends on how many conditionals we want to use to minimize the data
> and code space and time spent at runtime and for development.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] Change time_t to 64-bit by default

[Brian Inglis]

On 2017-09-08 09:00, Corinna Vinschen wrote:
> On Sep  8 08:46, Brian Inglis wrote:
>> On 2017-09-08 02:18, Corinna Vinschen wrote:
>>> On Sep  8 07:40, Freddie Chopin wrote:
>>>> On Thu, 2017-09-07 at 15:58 -0600, Brian Inglis wrote:
>>>>> The question is, going forward, should we use long for bigger than 
>>>>> int values as usual, or use time_t instead, to consistently avoid
>>>>> long, in these time functions.
>>>>
>>>> I think we should go with time_t, which will be 32-bit or 64-bit long, 
>>>> depending on what was chosen during newlib compilation.
>>>
>>> While that's nice and all, two points to keep in mind:
>>>
>>> * 64 bit computations are costly on 32 bit (or even 16 bit) platforms and
>>>   should be avoided if overkill isn't your goal.
>>>
>>> * days, months, years, etc, are *all* of type int in struct tm, per
>>>   POSIX. Adding 64 bit ops just to fold stuff back into ints might only 
>>>   make marginal sense.
>>
>> Max year == MAX_INT + 1900 == 2^31-1 + 1900 == 2147485547 on 32 bit platforms,
> 
> You wrote this before but it sounds like a pretty academic problem.

Check out the strftime.c regression test vectors before you conclude that.

>> if you want to avoid arbitrary limitations, and be as (Linux) compatible as
>> possible, which the existing library tries to do, but there are various issues
>> to be fixed. There are places where int, unsigned, long, and unsigned long are
>> used which may or may not work depending on time_t, int, and long sizes.
> 
> Glibc compat is nice, and as far as Cygwin is concerned I'm not
> concerned for size.  However, newlib strives to small footprints
> given that many embedded targets are using it.  This should be kept
> in mind.
> 
>>
>> Is long bigger than int on all newlib platforms? What is bigger than int?
>>
>> Using time_t as required makes temps and variables big enough to avoid overflow
>> for seconds, days, and years counts, without penalizing 32 bit time_t platforms.
> 
> Rather than int, long, long long you can use explicit sized ints
> like int16_t, int32_t, uint64_t.

We need space for things that are bigger than ints up to a few to a couple
orders of magnitude smaller than time_t, not even half the length, so picking
types smaller than time_t does not seem to be worth the code?

>> For I/O conversion of 64 bit time_t max year on 32 bit platforms long long is
>> required, it may also be used with 32 bit time_t to avoid conditionals, and it
>> imposes no extra overhead on 64 bit platforms.
>>
>> Everything depends on how many conditionals we want to use to minimize the data
>> and code space and time spent at runtime and for development.

I'm inclined to use common code and types that will work correctly on all
platforms *in places where they are needed* to get things working correctly,
then we can worry about space and time reductions if required?

I can post diffs for discussion before making any local commits or patches.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

Re: Time Fixes Diff (was: Change time_t to 64-bit by default)

[Brian Inglis]

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

On 2017-09-08 10:34, Brian Inglis wrote:
> On 2017-09-08 09:00, Corinna Vinschen wrote:
>> On Sep  8 08:46, Brian Inglis wrote:
>>> On 2017-09-08 02:18, Corinna Vinschen wrote:
>>>> On Sep  8 07:40, Freddie Chopin wrote:
>>>>> On Thu, 2017-09-07 at 15:58 -0600, Brian Inglis wrote:
>>>>>> The question is, going forward, should we use long for bigger than 
>>>>>> int values as usual, or use time_t instead, to consistently avoid
>>>>>> long, in these time functions.
>>>>>
>>>>> I think we should go with time_t, which will be 32-bit or 64-bit long, 
>>>>> depending on what was chosen during newlib compilation.
>>>>
>>>> While that's nice and all, two points to keep in mind:
>>>>
>>>> * 64 bit computations are costly on 32 bit (or even 16 bit) platforms and
>>>>   should be avoided if overkill isn't your goal.
>>>>
>>>> * days, months, years, etc, are *all* of type int in struct tm, per
>>>>   POSIX. Adding 64 bit ops just to fold stuff back into ints might only 
>>>>   make marginal sense.
>>>
>>> Max year == MAX_INT + 1900 == 2^31-1 + 1900 == 2147485547 on 32 bit platforms,
>>
>> You wrote this before but it sounds like a pretty academic problem.
> 
> Check out the strftime.c regression test vectors before you conclude that.
> 
>>> if you want to avoid arbitrary limitations, and be as (Linux) compatible as
>>> possible, which the existing library tries to do, but there are various issues
>>> to be fixed. There are places where int, unsigned, long, and unsigned long are
>>> used which may or may not work depending on time_t, int, and long sizes.
>>
>> Glibc compat is nice, and as far as Cygwin is concerned I'm not
>> concerned for size.  However, newlib strives to small footprints
>> given that many embedded targets are using it.  This should be kept
>> in mind.
>>
>>>
>>> Is long bigger than int on all newlib platforms? What is bigger than int?
>>>
>>> Using time_t as required makes temps and variables big enough to avoid overflow
>>> for seconds, days, and years counts, without penalizing 32 bit time_t platforms.
>>
>> Rather than int, long, long long you can use explicit sized ints
>> like int16_t, int32_t, uint64_t.
> 
> We need space for things that are bigger than ints up to a few to a couple
> orders of magnitude smaller than time_t, not even half the length, so picking
> types smaller than time_t does not seem to be worth the code?
> 
>>> For I/O conversion of 64 bit time_t max year on 32 bit platforms long long is
>>> required, it may also be used with 32 bit time_t to avoid conditionals, and it
>>> imposes no extra overhead on 64 bit platforms.
>>>
>>> Everything depends on how many conditionals we want to use to minimize the data
>>> and code space and time spent at runtime and for development.
> 
> I'm inclined to use common code and types that will work correctly on all
> platforms *in places where they are needed* to get things working correctly,
> then we can worry about space and time reductions if required?
> 
> I can post diffs for discussion before making any local commits or patches.

Attached for discussion.

Used local.h definitions, removed some local duplications, changed
tests/compares of > ... - 1) to >= ..., changed declarations to time_t and added
casts for large secs, days, years counts to avoid overflow with extreme values,
cast to long long for I/O mainly snprintf.

Not yet tested due to unrelated build issues on other files in latest git pull.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

[-- Attachment #2: fix_time.diff --]
[-- Type: text/plain, Size: 17542 bytes --]

diff --git a/newlib/libc/time/gmtime_r.c b/newlib/libc/time/gmtime_r.c
index 6475df3ba..379e8c26b 100644
--- a/newlib/libc/time/gmtime_r.c
+++ b/newlib/libc/time/gmtime_r.c
@@ -50,9 +50,9 @@ _DEFUN (gmtime_r, (tim_p, res),
 	_CONST time_t *__restrict tim_p _AND
 	struct tm *__restrict res)
 {
-  long days, rem;
+  time_t days, rem, year;
   _CONST time_t lcltime = *tim_p;
-  int era, weekday, year;
+  int era, weekday; 
   unsigned erayear, yearday, month, day;
   unsigned long eraday;
 
@@ -81,12 +81,12 @@ _DEFUN (gmtime_r, (tim_p, res),
   era = (days >= 0 ? days : days - (DAYS_PER_ERA - 1)) / DAYS_PER_ERA;
   eraday = days - era * DAYS_PER_ERA;	/* [0, 146096] */
   erayear = (eraday - eraday / (DAYS_PER_4_YEARS - 1) + eraday / DAYS_PER_CENTURY -
-      eraday / (DAYS_PER_ERA - 1)) / 365;	/* [0, 399] */
+      eraday / (DAYS_PER_ERA - 1)) / DAYS_PER_YEAR;	/* [0, 399] */
   yearday = eraday - (DAYS_PER_YEAR * erayear + erayear / 4 - erayear / 100);	/* [0, 365] */
   month = (5 * yearday + 2) / 153;	/* [0, 11] */
   day = yearday - (153 * month + 2) / 5 + 1;	/* [1, 31] */
   month += month < 10 ? 2 : -10;
-  year = ADJUSTED_EPOCH_YEAR + erayear + era * YEARS_PER_ERA + (month <= 1);
+  year = ADJUSTED_EPOCH_YEAR + erayear + (time_t)era * YEARS_PER_ERA + (month <= 1);
 
   res->tm_yday = yearday >= DAYS_PER_YEAR - DAYS_IN_JANUARY - DAYS_IN_FEBRUARY ?
       yearday - (DAYS_PER_YEAR - DAYS_IN_JANUARY - DAYS_IN_FEBRUARY) :
diff --git a/newlib/libc/time/lcltime_r.c b/newlib/libc/time/lcltime_r.c
index 3342e9906..56247350d 100644
--- a/newlib/libc/time/lcltime_r.c
+++ b/newlib/libc/time/lcltime_r.c
@@ -22,13 +22,13 @@ _DEFUN (localtime_r, (tim_p, res),
 {
   long offset;
   int hours, mins, secs;
-  int year;
+  time_t year;
   __tzinfo_type *_CONST tz = __gettzinfo ();
   _CONST int *ip;
 
   res = gmtime_r (tim_p, res);
 
-  year = res->tm_year + YEAR_BASE;
+  year = (time_t)(res->tm_year) + YEAR_BASE;
   ip = __month_lengths[isleap(year)];
 
   TZ_LOCK;
@@ -85,7 +85,7 @@ _DEFUN (localtime_r, (tim_p, res),
     {
       ++res->tm_yday;
       ++res->tm_wday;
-      if (res->tm_wday > 6)
+      if (res->tm_wday >= DAYSPERWEEK)
 	res->tm_wday = 0;
       ++res->tm_mday;
       res->tm_hour -= HOURSPERDAY;
@@ -93,7 +93,7 @@ _DEFUN (localtime_r, (tim_p, res),
 	{
 	  res->tm_mday -= ip[res->tm_mon];
 	  res->tm_mon += 1;
-	  if (res->tm_mon == 12)
+	  if (res->tm_mon == MONSPERYEAR)
 	    {
 	      res->tm_mon = 0;
 	      res->tm_year += 1;
@@ -106,17 +106,17 @@ _DEFUN (localtime_r, (tim_p, res),
       res->tm_yday -= 1;
       res->tm_wday -= 1;
       if (res->tm_wday < 0)
-	res->tm_wday = 6;
+	res->tm_wday = DAYSPERWEEK - 1;
       res->tm_mday -= 1;
-      res->tm_hour += 24;
+      res->tm_hour += HOURSPERDAY;
       if (res->tm_mday == 0)
 	{
 	  res->tm_mon -= 1;
 	  if (res->tm_mon < 0)
 	    {
-	      res->tm_mon = 11;
+	      res->tm_mon = MONSPERYEAR - 1;
 	      res->tm_year -= 1;
-	      res->tm_yday = 364 + isleap(res->tm_year + YEAR_BASE);
+	      res->tm_yday = 364 + isleap((time_t)(res->tm_year) + YEAR_BASE);
 	    }
 	  res->tm_mday = ip[res->tm_mon];
 	}
diff --git a/newlib/libc/time/local.h b/newlib/libc/time/local.h
index af5793af9..1261496d0 100644
--- a/newlib/libc/time/local.h
+++ b/newlib/libc/time/local.h
@@ -19,7 +19,7 @@
 
 #define isleap(y) ((((y) % 4) == 0 && ((y) % 100) != 0) || ((y) % 400) == 0)
 
-int         _EXFUN (__tzcalc_limits, (int __year));
+int         _EXFUN (__tzcalc_limits, (time_t __year));
 
 extern _CONST int __month_lengths[2][MONSPERYEAR];
 
diff --git a/newlib/libc/time/mktime.c b/newlib/libc/time/mktime.c
index 44c0257f7..f01037b97 100644
--- a/newlib/libc/time/mktime.c
+++ b/newlib/libc/time/mktime.c
@@ -51,10 +51,6 @@ ANSI C requires <<mktime>>.
 #include <time.h>
 #include "local.h"
 
-#define _SEC_IN_MINUTE 60L
-#define _SEC_IN_HOUR 3600L
-#define _SEC_IN_DAY 86400L
-
 static _CONST int DAYS_IN_MONTH[12] =
 {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
 
@@ -63,8 +59,7 @@ static _CONST int DAYS_IN_MONTH[12] =
 static _CONST int _DAYS_BEFORE_MONTH[12] =
 {0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334};
 
-#define _ISLEAP(y) (((y) % 4) == 0 && (((y) % 100) != 0 || (((y)+1900) % 400) == 0))
-#define _DAYS_IN_YEAR(year) (_ISLEAP(year) ? 366 : 365)
+#define _DAYS_IN_YEAR(year) (isleap((year) + YEAR_BASE) ? 366 : 365)
 
 static void 
 _DEFUN(validate_structure, (tim_p),
@@ -74,46 +69,46 @@ _DEFUN(validate_structure, (tim_p),
   int days_in_feb = 28;
 
   /* calculate time & date to account for out of range values */
-  if (tim_p->tm_sec < 0 || tim_p->tm_sec > 59)
+  if (tim_p->tm_sec < 0 || tim_p->tm_sec >= SECSPERMIN)
     {
-      res = div (tim_p->tm_sec, 60);
+      res = div (tim_p->tm_sec, SECSPERMIN);
       tim_p->tm_min += res.quot;
       if ((tim_p->tm_sec = res.rem) < 0)
 	{
-	  tim_p->tm_sec += 60;
+	  tim_p->tm_sec += SECSPERMIN;
 	  --tim_p->tm_min;
 	}
     }
 
-  if (tim_p->tm_min < 0 || tim_p->tm_min > 59)
+  if (tim_p->tm_min < 0 || tim_p->tm_min >= MINSPERHOUR)
     {
-      res = div (tim_p->tm_min, 60);
+      res = div (tim_p->tm_min, MINSPERHOUR);
       tim_p->tm_hour += res.quot;
       if ((tim_p->tm_min = res.rem) < 0)
 	{
-	  tim_p->tm_min += 60;
+	  tim_p->tm_min += MINSPERHOUR;
 	  --tim_p->tm_hour;
         }
     }
 
-  if (tim_p->tm_hour < 0 || tim_p->tm_hour > 23)
+  if (tim_p->tm_hour < 0 || tim_p->tm_hour >= HOURSPERDAY)
     {
-      res = div (tim_p->tm_hour, 24);
+      res = div (tim_p->tm_hour, HOURSPERDAY);
       tim_p->tm_mday += res.quot;
       if ((tim_p->tm_hour = res.rem) < 0)
 	{
-	  tim_p->tm_hour += 24;
+	  tim_p->tm_hour += HOURSPERDAY;
 	  --tim_p->tm_mday;
         }
     }
 
-  if (tim_p->tm_mon < 0 || tim_p->tm_mon > 11)
+  if (tim_p->tm_mon < 0 || tim_p->tm_mon >= MONSPERYEAR)
     {
-      res = div (tim_p->tm_mon, 12);
+      res = div (tim_p->tm_mon, MONSPERYEAR);
       tim_p->tm_year += res.quot;
       if ((tim_p->tm_mon = res.rem) < 0)
         {
-	  tim_p->tm_mon += 12;
+	  tim_p->tm_mon += MONSPERYEAR;
 	  --tim_p->tm_year;
         }
     }
@@ -128,7 +123,7 @@ _DEFUN(validate_structure, (tim_p),
 	  if (--tim_p->tm_mon == -1)
 	    {
 	      tim_p->tm_year--;
-	      tim_p->tm_mon = 11;
+	      tim_p->tm_mon = MONSPERYEAR - 1;
 	      days_in_feb =
 		((_DAYS_IN_YEAR (tim_p->tm_year) == 366) ?
 		 29 : 28);
@@ -141,7 +136,7 @@ _DEFUN(validate_structure, (tim_p),
       while (tim_p->tm_mday > _DAYS_IN_MONTH (tim_p->tm_mon))
 	{
 	  tim_p->tm_mday -= _DAYS_IN_MONTH (tim_p->tm_mon);
-	  if (++tim_p->tm_mon == 12)
+	  if (++tim_p->tm_mon == MONSPERYEAR)
 	    {
 	      tim_p->tm_year++;
 	      tim_p->tm_mon = 0;
@@ -158,16 +153,17 @@ _DEFUN(mktime, (tim_p),
      struct tm *tim_p)
 {
   time_t tim = 0;
-  long days = 0;
-  int year, isdst=0;
+  time_t days = 0;
+  time_t year;
+  int isdst=0;
   __tzinfo_type *tz = __gettzinfo ();
 
   /* validate structure */
   validate_structure (tim_p);
 
   /* compute hours, minutes, seconds */
-  tim += tim_p->tm_sec + (tim_p->tm_min * _SEC_IN_MINUTE) +
-    (tim_p->tm_hour * _SEC_IN_HOUR);
+  tim += tim_p->tm_sec + (tim_p->tm_min * SECSPERMIN) +
+    (tim_p->tm_hour * SECSPERHOUR);
 
   /* compute days in year */
   days += tim_p->tm_mday - 1;
@@ -178,9 +174,6 @@ _DEFUN(mktime, (tim_p),
   /* compute day of the year */
   tim_p->tm_yday = days;
 
-  if (tim_p->tm_year > 10000 || tim_p->tm_year < -10000)
-      return (time_t) -1;
-
   /* compute days in other years */
   if ((year = tim_p->tm_year) > 70)
     {
@@ -195,7 +188,7 @@ _DEFUN(mktime, (tim_p),
     }
 
   /* compute total seconds */
-  tim += (days * _SEC_IN_DAY);
+  tim += (days * SECSPERDAY);
 
   TZ_LOCK;
 
@@ -204,7 +197,7 @@ _DEFUN(mktime, (tim_p),
   if (_daylight)
     {
       int tm_isdst;
-      int y = tim_p->tm_year + YEAR_BASE;
+      time_t y = tim_p->tm_year + YEAR_BASE;
       /* Convert user positive into 1 */
       tm_isdst = tim_p->tm_isdst > 0  ?  1 : tim_p->tm_isdst;
       isdst = tm_isdst;
@@ -236,7 +229,7 @@ _DEFUN(mktime, (tim_p),
 		     time zone offsets, depending on which way the user got it
 		     wrong. The diff is typically one hour, or 3600 seconds,
 		     and should fit in a 16-bit int, even though offset
-		     is a long to accomodate 12 hours. */
+		     is a long to accomodate 12+ hours. */
 		  int diff = (int) (tz->__tzrule[0].offset
 				    - tz->__tzrule[1].offset);
 		  if (!isdst)
@@ -261,7 +254,7 @@ _DEFUN(mktime, (tim_p),
 			  tim_p->tm_yday = _DAYS_IN_YEAR(year) - 1;
 		    } else {
 			  mday = _DAYS_IN_YEAR(year);
-			  if (tim_p->tm_yday > (mday - 1))
+			  if (tim_p->tm_yday >= mday)
 				tim_p->tm_yday -= mday;
 		    }
 		  }
@@ -282,8 +275,8 @@ _DEFUN(mktime, (tim_p),
   tim_p->tm_isdst = isdst;
 
   /* compute day of the week */
-  if ((tim_p->tm_wday = (days + 4) % 7) < 0)
-    tim_p->tm_wday += 7;
+  if ((tim_p->tm_wday = (days + EPOCH_WDAY) % DAYSPERWEEK) < 0)
+    tim_p->tm_wday += DAYSPERWEEK;
 	
   return tim;
 }
diff --git a/newlib/libc/time/strftime.c b/newlib/libc/time/strftime.c
index 382318047..b99b0671c 100644
--- a/newlib/libc/time/strftime.c
+++ b/newlib/libc/time/strftime.c
@@ -360,7 +360,7 @@ _DEFUN (iso_year_adjust, (tim_p),
 	_CONST struct tm *tim_p)
 {
   /* Account for fact that tm_year==0 is year 1900.  */
-  int leap = isleap (tim_p->tm_year + (YEAR_BASE
+  int leap = isleap ((time_t)(tim_p->tm_year) + (YEAR_BASE
 				       - (tim_p->tm_year < 0 ? 0 : 2000)));
 
   /* Pack the yday, wday, and leap year into a single int since there are so
@@ -872,7 +872,7 @@ recurse:
 		int neg = tim_p->tm_year < -YEAR_BASE;
 		int century = tim_p->tm_year >= 0
 		  ? tim_p->tm_year / 100 + YEAR_BASE / 100
-		  : abs (tim_p->tm_year + YEAR_BASE) / 100;
+		  : llabs ((time_t)(tim_p->tm_year) + YEAR_BASE) / 100;
 		if (pad) /* '0' or '+' */
 		  {
 		    fmt = CQ("%s%0.*d");
@@ -922,7 +922,7 @@ recurse:
 			  CQ("%.2d/%.2d/%.2d"),
 			  tim_p->tm_mon + 1, tim_p->tm_mday,
 			  tim_p->tm_year >= 0 ? tim_p->tm_year % 100
-			  : abs (tim_p->tm_year + YEAR_BASE) % 100);
+			  : llabs ((time_t)(tim_p->tm_year) + YEAR_BASE) % 100);
           CHECK_LENGTH ();
 	  break;
 	case CQ('F'):
@@ -961,7 +961,7 @@ recurse:
 	  {
 	    int adjust = iso_year_adjust (tim_p);
 	    int year = tim_p->tm_year >= 0 ? tim_p->tm_year % 100
-		: abs (tim_p->tm_year + YEAR_BASE) % 100;
+		: llabs ((time_t)(tim_p->tm_year) + YEAR_BASE) % 100;
 	    if (adjust < 0 && tim_p->tm_year <= -YEAR_BASE)
 		adjust = 1;
 	    else if (adjust > 0 && tim_p->tm_year < -YEAR_BASE)
@@ -980,9 +980,9 @@ recurse:
 	    int adjust = iso_year_adjust (tim_p);
 	    int century = tim_p->tm_year >= 0
 	      ? tim_p->tm_year / 100 + YEAR_BASE / 100
-	      : abs (tim_p->tm_year + YEAR_BASE) / 100;
+	      : llabs ((time_t)(tim_p->tm_year) + YEAR_BASE) / 100;
 	    int year = tim_p->tm_year >= 0 ? tim_p->tm_year % 100
-	      : abs (tim_p->tm_year + YEAR_BASE) % 100;
+	      : llabs ((time_t)(tim_p->tm_year) + YEAR_BASE) % 100;
 	    if (adjust < 0 && tim_p->tm_year <= -YEAR_BASE)
 	      sign = adjust = 1;
 	    else if (adjust > 0 && sign)
@@ -999,8 +999,8 @@ recurse:
 		++century;
 	      }
 	    CHAR fmtbuf[10], *fmt = fmtbuf;
-	    /* int potentially overflows, so use unsigned instead.  */
-	    unsigned p_year = century * 100 + year;
+	    /* int potentially overflows, so use time_t instead.  */
+	    time_t p_year = century * 100 + year;
 	    if (sign)
 	      *fmt++ = CQ('-');
 	    else if (pad == CQ('+') && p_year >= 10000)
@@ -1013,8 +1013,8 @@ recurse:
 	    *fmt++ = CQ('%');
 	    if (pad)
 	      *fmt++ = CQ('0');
-	    STRCPY (fmt, CQ(".*u"));
-	    len = snprintf (&s[count], maxsize - count, fmtbuf, width, p_year);
+	    STRCPY (fmt, CQ(".*lld"));
+	    len = snprintf (&s[count], maxsize - count, fmtbuf, width, (long long)p_year);
             if (len < 0  ||  (count+=len) >= maxsize)
               return 0;
 	  }
@@ -1105,31 +1105,7 @@ recurse:
           CHECK_LENGTH ();
           break;
 	case CQ('s'):
-/*
- * From:
- * The Open Group Base Specifications Issue 7
- * IEEE Std 1003.1, 2013 Edition
- * Copyright (c) 2001-2013 The IEEE and The Open Group
- * XBD Base Definitions
- * 4. General Concepts
- * 4.15 Seconds Since the Epoch
- * A value that approximates the number of seconds that have elapsed since the
- * Epoch. A Coordinated Universal Time name (specified in terms of seconds
- * (tm_sec), minutes (tm_min), hours (tm_hour), days since January 1 of the year
- * (tm_yday), and calendar year minus 1900 (tm_year)) is related to a time
- * represented as seconds since the Epoch, according to the expression below.
- * If the year is <1970 or the value is negative, the relationship is undefined.
- * If the year is >=1970 and the value is non-negative, the value is related to a
- * Coordinated Universal Time name according to the C-language expression, where
- * tm_sec, tm_min, tm_hour, tm_yday, and tm_year are all integer types:
- * tm_sec + tm_min*60 + tm_hour*3600 + tm_yday*86400 +
- *     (tm_year-70)*31536000 + ((tm_year-69)/4)*86400 -
- *     ((tm_year-1)/100)*86400 + ((tm_year+299)/400)*86400
- * OR
- * ((((tm_year-69)/4 - (tm_year-1)/100 + (tm_year+299)/400 +
- *         (tm_year-70)*365 + tm_yday)*24 + tm_hour)*60 + tm_min)*60 + tm_sec
- */
-/* modified from %z case by hoisting offset outside if block and initializing */
+/* modified from %z case by hoisting offset outside of block and initializing */
 	  {
 	    long offset = 0;	/* offset < 0 => W of GMT, > 0 => E of GMT:
 				   subtract to get UTC */
@@ -1162,13 +1138,40 @@ recurse:
 #endif
 		TZ_UNLOCK;
 	      }
+/*
+ * From:
+ * The Open Group Base Specifications Issue 7
+ * IEEE Std 1003.1, 2013 Edition
+ * Copyright (c) 2001-2013 The IEEE and The Open Group
+ * XBD Base Definitions
+ * 4. General Concepts
+ * 4.15 Seconds Since the Epoch
+ * A value that approximates the number of seconds that have elapsed since the
+ * Epoch. A Coordinated Universal Time name (specified in terms of seconds
+ * (tm_sec), minutes (tm_min), hours (tm_hour), days since January 1 of the year
+ * (tm_yday), and calendar year minus 1900 (tm_year)) is related to a time
+ * represented as seconds since the Epoch, according to the expression below.
+ * If the year is <1970 or the value is negative, the relationship is undefined.
+ * If the year is >=1970 and the value is non-negative, the value is related to a
+ * Coordinated Universal Time name according to the C-language expression, where
+ * tm_sec, tm_min, tm_hour, tm_yday, and tm_year are all integer types:
+ * tm_sec + tm_min*60 + tm_hour*3600 + tm_yday*86400 +
+ *     (tm_year-70)*31536000 + ((tm_year-69)/4)*86400 -
+ *     ((tm_year-1)/100)*86400 + ((tm_year+299)/400)*86400
+ * OR
+ * ((((tm_year-69)/4 - (tm_year-1)/100 + (tm_year+299)/400 +
+ *         (tm_year-70)*365 + tm_yday)*24 + tm_hour)*60 + tm_min)*60 + tm_sec
+ */
 	    len = snprintf (&s[count], maxsize - count, CQ("%lld"),
-			    (((((long long)tim_p->tm_year - 69)/4
-				- (tim_p->tm_year - 1)/100
-				+ (tim_p->tm_year + 299)/400
-				+ (tim_p->tm_year - 70)*365 + tim_p->tm_yday)*24
-			      + tim_p->tm_hour)*60 + tim_p->tm_min)*60
-			    + tim_p->tm_sec - offset);
+			    (long long)((((
+				  ((time_t)(tim_p->tm_year) - 70)*365
+					 + (tim_p->tm_year - 69)/4
+					 - (tim_p->tm_year - 1)/100
+				+ ((time_t)(tim_p->tm_year) + 299)/400
+					  + tim_p->tm_yday)*HOURSPERDAY
+					  + tim_p->tm_hour)*MINSPERHOUR
+					  + tim_p->tm_min)*SECSPERMIN
+					  + tim_p->tm_sec - offset));
 	    CHECK_LENGTH ();
 	  }
           break;
@@ -1241,7 +1244,7 @@ recurse:
 		   Fri, and also if current year starts on Sat and
 		   previous year was leap year.  */
 		week = 52 + (4 >= (wday - tim_p->tm_yday
-				   - isleap (tim_p->tm_year
+				   - isleap ((time_t)(tim_p->tm_year)
 					     + (YEAR_BASE - 1
 						- (tim_p->tm_year < 0
 						   ? 0 : 2000)))));
@@ -1295,7 +1298,7 @@ recurse:
 		  /* Be careful of both overflow and negative years, thanks to
 		     the asymmetric range of years.  */
 		  int year = tim_p->tm_year >= 0 ? tim_p->tm_year % 100
-			     : abs (tim_p->tm_year + YEAR_BASE) % 100;
+			     : llabs ((time_t)(tim_p->tm_year) + YEAR_BASE) % 100;
 #ifdef _WANT_C99_TIME_FORMATS
 		  if (alt != CQ('O') || !*alt_digits
 		      || !(len = conv_to_alt_digits (&s[count], maxsize - count,
@@ -1319,9 +1322,9 @@ recurse:
 	    {
 	      CHAR fmtbuf[10], *fmt = fmtbuf;
 	      int sign = tim_p->tm_year < -YEAR_BASE;
-	      /* int potentially overflows, so use unsigned instead.  */
-	      register unsigned year = (unsigned) tim_p->tm_year
-				       + (unsigned) YEAR_BASE;
+	      /* int potentially overflows, so use time_t instead.  */
+	      register time_t year = (time_t)(tim_p->tm_year)
+				       + YEAR_BASE;
 	      if (sign)
 		{
 		  *fmt++ = CQ('-');
@@ -1337,9 +1340,9 @@ recurse:
 	      *fmt++ = CQ('%');
 	      if (pad)
 		*fmt++ = CQ('0');
-	      STRCPY (fmt, CQ(".*u"));
+	      STRCPY (fmt, CQ(".*lld"));
 	      len = snprintf (&s[count], maxsize - count, fmtbuf, width,
-			      year);
+			      (long long)year);
 	      CHECK_LENGTH ();
 	    }
 	  break;
diff --git a/newlib/libc/time/tzcalc_limits.c b/newlib/libc/time/tzcalc_limits.c
index 8a0bda3da..e0be70596 100644
--- a/newlib/libc/time/tzcalc_limits.c
+++ b/newlib/libc/time/tzcalc_limits.c
@@ -12,9 +12,9 @@
 
 int
 _DEFUN (__tzcalc_limits, (year),
-	int year)
+	time_t year)
 {
-  int days, year_days, years;
+  time_t days, year_days, years;
   int i, j;
   __tzinfo_type *_CONST tz = __gettzinfo ();
 

Re: Time Fixes Diff

[Craig Howland]

On 09/10/2017 12:27 AM, Brian Inglis wrote:
> On 2017-09-08 10:34, Brian Inglis wrote:
>> On 2017-09-08 09:00, Corinna Vinschen wrote:
>>> On Sep  8 08:46, Brian Inglis wrote:
>>>> On 2017-09-08 02:18, Corinna Vinschen wrote:
>>>>> On Sep  8 07:40, Freddie Chopin wrote:
>>>>>> On Thu, 2017-09-07 at 15:58 -0600, Brian Inglis wrote:
>>>>>>> The question is, going forward, should we use long for bigger than
>>>>>>> int values as usual, or use time_t instead, to consistently avoid
>>>>>>> long, in these time functions.
>>>>>> I think we should go with time_t, which will be 32-bit or 64-bit long,
>>>>>> depending on what was chosen during newlib compilation.
>>>>> While that's nice and all, two points to keep in mind:
>>>>>
>>>>> * 64 bit computations are costly on 32 bit (or even 16 bit) platforms and
>>>>>    should be avoided if overkill isn't your goal.
>>>>>
>>>>> * days, months, years, etc, are *all* of type int in struct tm, per
>>>>>    POSIX. Adding 64 bit ops just to fold stuff back into ints might only
>>>>>    make marginal sense.
>>>> Max year == MAX_INT + 1900 == 2^31-1 + 1900 == 2147485547 on 32 bit platforms,
>>> You wrote this before but it sounds like a pretty academic problem.
>> Check out the strftime.c regression test vectors before you conclude that.
>>
>>>> if you want to avoid arbitrary limitations, and be as (Linux) compatible as
>>>> possible, which the existing library tries to do, but there are various issues
>>>> to be fixed. There are places where int, unsigned, long, and unsigned long are
>>>> used which may or may not work depending on time_t, int, and long sizes.
>>> Glibc compat is nice, and as far as Cygwin is concerned I'm not
>>> concerned for size.  However, newlib strives to small footprints
>>> given that many embedded targets are using it.  This should be kept
>>> in mind.
>>>
>>>> Is long bigger than int on all newlib platforms? What is bigger than int?
>>>>
>>>> Using time_t as required makes temps and variables big enough to avoid overflow
>>>> for seconds, days, and years counts, without penalizing 32 bit time_t platforms.
>>> Rather than int, long, long long you can use explicit sized ints
>>> like int16_t, int32_t, uint64_t.
>> We need space for things that are bigger than ints up to a few to a couple
>> orders of magnitude smaller than time_t, not even half the length, so picking
>> types smaller than time_t does not seem to be worth the code?
>>
>>>> For I/O conversion of 64 bit time_t max year on 32 bit platforms long long is
>>>> required, it may also be used with 32 bit time_t to avoid conditionals, and it
>>>> imposes no extra overhead on 64 bit platforms.
>>>>
>>>> Everything depends on how many conditionals we want to use to minimize the data
>>>> and code space and time spent at runtime and for development.
>> I'm inclined to use common code and types that will work correctly on all
>> platforms *in places where they are needed* to get things working correctly,
>> then we can worry about space and time reductions if required?
>>
>> I can post diffs for discussion before making any local commits or patches.
> Attached for discussion.
>
> Used local.h definitions, removed some local duplications, changed
> tests/compares of > ... - 1) to >= ..., changed declarations to time_t and added
> casts for large secs, days, years counts to avoid overflow with extreme values,
> cast to long long for I/O mainly snprintf.
>
> Not yet tested due to unrelated build issues on other files in latest git pull.
>
If you're going to bother to spend the time (I agree with Corinna that it is a 
purely academic exercise, of no practical value), it probably should be changed 
to use int_least types to be friendliest to the smallest-int targets, and it 
must have additional work to not be wrong on small-int targets.  For example, 
era as it is presently sized strictly does not work in the doomsday view, as it 
is only an int but can have a maximum theoretical value of about 730.7e6 (needs 
30 bits)--it won't work properly for a 16-bit target.

But in addition to the theoretical size problems in the variables, there is 
another little-known and very real consideration that really renders those 
problems moot, and that is leap seconds. gmtime() is defined as using UTC, to 
which leap seconds are applied, so this is a real consideration, not just 
theoretical.  (If it were GPS time things would be different, but even with UT1 
there would be issues.)  Long-time timekeeping is actually very complicated, 
especially if you're trying to use seconds as the basis for the measurement.  
(Using days and the proleptic Gregorian calendar is an improvement of sorts over 
seconds, but still has problems.  If you've never heard of the proleptic 
Gregorian calendar, you're in the majority, but even the fact that the concept 
is defined underscores the problems with time reporting over longer timeframes.)

Have you done a calculation to show where the present code (without this 
proposed patch applied) first breaks down?

The test case checking for year 2e9 is clearly insane.  With the unpredictable 
nature of leap seconds, there is no way to go that far ahead and get it right.  
At the recent rate--27 leap seconds since 1972--there might be as many as 1.3e9 
by the year 0x7FFFFFFF. That's potentially about 41 years worth of leap 
seconds.  Even if the year were limited to 10000 (7983 years from now), it is 
potentially 4790 leap seconds.  (That's 1:20:50, which makes seconds, minutes, 
hours, and possibly even the day of the week, wrong.)  This is even ignoring the 
rate at which earth's rotation is slowing, which serves to increase the 
leap-second insertion rate as the years go on.

As I've quantified this somewhat in forming this reply, I'm thinking that 
perhaps we should just leave the code the way it is and simply add a note to the 
man page which points out roughly the year at which the code might break but 
that going even that far is introducing a leap-second-based error which is 
impossible to account for (either forwards a long time, or backwards past 1972 
when leap seconds start for UTC).  Making changes to fix the places where the 
calculations can overflow is a false help, as while it addresses one issue, it 
does not address the larger issue that trying to use the function over such a 
larger time is an invalid use of it.  (A main reason for suggesting that it be 
left alone is to avoid penalizing small-int targets with performance penalties 
for a non-real help. If changes can turn out to not penalize it, cleaning up a 
little then would not hurt.)

Craig

Re: Time Fixes Diff

[Brian Inglis]

On 2017-09-11 09:17, Craig Howland wrote:
> On 09/10/2017 12:27 AM, Brian Inglis wrote:
>> On 2017-09-08 10:34, Brian Inglis wrote:
>>> On 2017-09-08 09:00, Corinna Vinschen wrote:
>>>> On Sep  8 08:46, Brian Inglis wrote:
>>>>> On 2017-09-08 02:18, Corinna Vinschen wrote:
>>>>>> On Sep  8 07:40, Freddie Chopin wrote:
>>>>>>> On Thu, 2017-09-07 at 15:58 -0600, Brian Inglis wrote:
>>>>>>>> The question is, going forward, should we use long for bigger than
>>>>>>>> int values as usual, or use time_t instead, to consistently avoid
>>>>>>>> long, in these time functions.
>>>>>>> I think we should go with time_t, which will be 32-bit or 64-bit long,
>>>>>>> depending on what was chosen during newlib compilation.
>>>>>> While that's nice and all, two points to keep in mind:
>>>>>>
>>>>>> * 64 bit computations are costly on 32 bit (or even 16 bit) platforms and
>>>>>>    should be avoided if overkill isn't your goal.
>>>>>>
>>>>>> * days, months, years, etc, are *all* of type int in struct tm, per
>>>>>>    POSIX. Adding 64 bit ops just to fold stuff back into ints might only
>>>>>>    make marginal sense.
>>>>> Max year == MAX_INT + 1900 == 2^31-1 + 1900 == 2147485547 on 32 bit platforms,
>>>> You wrote this before but it sounds like a pretty academic problem.
>>> Check out the strftime.c regression test vectors before you conclude that.
>>>
>>>>> if you want to avoid arbitrary limitations, and be as (Linux) compatible as
>>>>> possible, which the existing library tries to do, but there are various issues
>>>>> to be fixed. There are places where int, unsigned, long, and unsigned long are
>>>>> used which may or may not work depending on time_t, int, and long sizes.
>>>> Glibc compat is nice, and as far as Cygwin is concerned I'm not
>>>> concerned for size.  However, newlib strives to small footprints
>>>> given that many embedded targets are using it.  This should be kept
>>>> in mind.
>>>>
>>>>> Is long bigger than int on all newlib platforms? What is bigger than int?
>>>>>
>>>>> Using time_t as required makes temps and variables big enough to
>>>>> avoid overflow for seconds, days, and years counts, without
>>>>> penalizing 32 bit time_t platforms.
>>>> Rather than int, long, long long you can use explicit sized ints
>>>> like int16_t, int32_t, uint64_t.
>>> We need space for things that are bigger than ints up to a few to a couple
>>> orders of magnitude smaller than time_t, not even half the length, so picking
>>> types smaller than time_t does not seem to be worth the code?
>>>
>>>>> For I/O conversion of 64 bit time_t max year on 32 bit platforms long long is
>>>>> required, it may also be used with 32 bit time_t to avoid conditionals, and it
>>>>> imposes no extra overhead on 64 bit platforms.
>>>>>
>>>>> Everything depends on how many conditionals we want to use to
>>>>> minimize the data and code space and time spent at runtime and for
>>>>> development.
>>> I'm inclined to use common code and types that will work correctly on all
>>> platforms *in places where they are needed* to get things working correctly,
>>> then we can worry about space and time reductions if required?
>>>
>>> I can post diffs for discussion before making any local commits or patches.
>> Attached for discussion.
>>
>> Used local.h definitions, removed some local duplications, changed
>> tests/compares of > ... - 1) to >= ..., changed declarations to time_t and added
>> casts for large secs, days, years counts to avoid overflow with extreme values,
>> cast to long long for I/O mainly snprintf.
>>
>> Not yet tested due to unrelated build issues on other files in latest git pull.
>>
> If you're going to bother to spend the time (I agree with Corinna that it is a
> purely academic exercise, of no practical value), it probably should be changed
> to use int_least types to be friendliest to the smallest-int targets, and it
> must have additional work to not be wrong on small-int targets.  For example,
> era as it is presently sized strictly does not work in the doomsday view, as it
> is only an int but can have a maximum theoretical value of about 730.7e6 (needs
> 30 bits)--it won't work properly for a 16-bit target.

There's a couple of considerations here - compatibility with 64 bit time_t Unix
platforms which correctly handle values up to 32 bit INT_MAX tm_year + 1900 and
ensuring 32 bit time_t works okay until 2038-01-19 03:14:07+0000.

> But in addition to the theoretical size problems in the variables, there is
> another little-known and very real consideration that really renders those
> problems moot, and that is leap seconds. gmtime() is defined as using UTC, to
> which leap seconds are applied, so this is a real consideration, not just
> theoretical.  (If it were GPS time things would be different, but even with UT1
> there would be issues.)  Long-time timekeeping is actually very complicated,
> especially if you're trying to use seconds as the basis for the measurement. 
> (Using days and the proleptic Gregorian calendar is an improvement of sorts over
> seconds, but still has problems.  If you've never heard of the proleptic
> Gregorian calendar, you're in the majority, but even the fact that the concept
> is defined underscores the problems with time reporting over longer timeframes.)

Our primary compliance seems to be to the POSIX time scale, which ignores
leap-seconds, and gmtime() implements, so any leap second handling must happen
in the tz code, perhaps using zoneinfo right to handle TAI in the past.

Until someone can come up with a decent model of earth rotation or a better
approach to timekeeping, solutions must be application specific to be practical.

I've heard of, used, and interconverted dates from many calendars and time
scales, bought or collected and used most calendar and time scale conversion
code out there, but for personal use, I go back to basic rules and work from there.

> Have you done a calculation to show where the present code (without this
> proposed patch applied) first breaks down?

They break down when they put results from 64 bit time_t into years, days, and
seconds variables that are larger than int and smaller than time_t by factors of
only 86400 or 31536000.
Unfortunately every function uses different adhoc code to deal with the same
entities and do its job, so each has different issues in different places, not
all overflows. I'd like to be sure the calculations are being done properly for
64 bit time_t to validate the functions and fix other issues.

> The test case checking for year 2e9 is clearly insane.  With the unpredictable
> nature of leap seconds, there is no way to go that far ahead and get it right. 
> At the recent rate--27 leap seconds since 1972--there might be as many as 1.3e9
> by the year 0x7FFFFFFF. That's potentially about 41 years worth of leap
> seconds.  Even if the year were limited to 10000 (7983 years from now), it is
> potentially 4790 leap seconds.  (That's 1:20:50, which makes seconds, minutes,
> hours, and possibly even the day of the week, wrong.)  This is even ignoring the
> rate at which earth's rotation is slowing, which serves to increase the
> leap-second insertion rate as the years go on.

Unfortunately the short term variations caused by natural variations and
disasters including weather make leap second prediction an environmental
forecasting problem involving the earth's interior, volcanoes, glaciers and ice
shelfs, hurricanes, and droughts, which all change the earth's mass distribution
and rotation.

> As I've quantified this somewhat in forming this reply, I'm thinking that
> perhaps we should just leave the code the way it is and simply add a note to the
> man page which points out roughly the year at which the code might break but
> that going even that far is introducing a leap-second-based error which is
> impossible to account for (either forwards a long time, or backwards past 1972
> when leap seconds start for UTC).  Making changes to fix the places where the
> calculations can overflow is a false help, as while it addresses one issue, it
> does not address the larger issue that trying to use the function over such a
> larger time is an invalid use of it.  (A main reason for suggesting that it be
> left alone is to avoid penalizing small-int targets with performance penalties
> for a non-real help. If changes can turn out to not penalize it, cleaning up a
> little then would not hurt.)

Perhaps the proposal is premature, I should take the work offline, and maybe fix
Cygwin to at least handle time consistently within normal ranges. Then other
platform users can see if they want to test and enable the Cygwin patches.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

Re: Time Fixes Diff

[Sebastian Huber]

There are (probably) working implementations in FreeBSD, Bionic, OpenBSD 
and NetBSD with leap seconds support. Why don't we use one of them in 
Newlib?

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber@embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.