[PATCH 0/6] y2038: Convert settimeofday to be Y2038 safe

[PATCH 0/6] y2038: Convert settimeofday to be Y2038 safe

[Lukasz Majewski]

In this patch series the settimeofday undergoes some rework to internally use
__clock_settime64, which now supports 64 bit time (even on machines with
__TIMESIZE != 64).

It is now possible to use settimeofday in Y2038 systems (when proper redirection
is used).

To allow this conversion it was necessary to modify some alpha code and 
# include <sys/time.h> in ./include/time.h

The repository with Y2038 supprt (which uses this work)
https://github.com/lmajewski/y2038_glibc/commits/glibc_settimeofday_64bit_conversion_v1

Lukasz Majewski (6):
  y2038: Use __clock_settime64 in deprecated stime function
  y2038: hurd: Provide __clock_settime64 function
  y2038: Introduce struct __timeval64 - new internal glibc type
  y2038: alpha: Rename valid_timeval_to_timeval64 to
    valid_timeval32_to_timeval
  y2038: Provide conversion helpers for struct __timeval64
  y2038: linux: Provide __settimeofday64 implementation

 include/time.h                                | 59 +++++++++++++++++++
 sysdeps/mach/hurd/clock_settime.c             |  9 +++
 sysdeps/unix/sysv/linux/alpha/osf_adjtime.c   |  4 +-
 sysdeps/unix/sysv/linux/alpha/osf_setitimer.c |  4 +-
 sysdeps/unix/sysv/linux/alpha/osf_utimes.c    |  4 +-
 sysdeps/unix/sysv/linux/alpha/tv32-compat.h   |  2 +-
 time/settimeofday.c                           | 19 ++++--
 time/stime.c                                  |  4 +-
 8 files changed, 92 insertions(+), 13 deletions(-)

-- 
2.20.1

[PATCH 5/6] y2038: Provide conversion helpers for struct __timeval64

[Lukasz Majewski]

Those functions allow easy conversion between Y2038 safe, glibc internal
struct __timeval64 and other time related data structures (like struct timeval
or struct __timespec64).

Build tests:
./src/scripts/build-many-glibcs.py glibcs
---
 include/time.h | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/include/time.h b/include/time.h
index 22ecacd0d8..b40214c006 100644
--- a/include/time.h
+++ b/include/time.h
@@ -304,6 +304,30 @@ valid_timeval_to_timespec64 (const struct timeval tv)
   return ts64;
 }
 
+/* Convert a known valid struct timeval into a struct __timeval64.  */
+static inline struct __timeval64
+valid_timeval_to_timeval64 (const struct timeval tv)
+{
+  struct __timeval64 tv64;
+
+  tv64.tv_sec = tv.tv_sec;
+  tv64.tv_usec = tv.tv_usec;
+
+  return tv64;
+}
+
+/* Convert a struct __timeval64 into a struct __timespec64.  */
+static inline struct __timespec64
+timeval64_to_timespec64 (const struct __timeval64 tv64)
+{
+  struct __timespec64 ts64;
+
+  ts64.tv_sec = tv64.tv_sec;
+  ts64.tv_nsec = tv64.tv_usec * 1000;
+
+  return ts64;
+}
+
 /* Convert a known valid struct timespec into a struct __timespec64.  */
 static inline struct __timespec64
 valid_timespec_to_timespec64 (const struct timespec ts)
@@ -342,6 +366,18 @@ valid_timespec64_to_timeval (const struct __timespec64 ts64)
   return tv;
 }
 
+/* Convert a struct __timespec64 into a struct __timeval64.  */
+static inline struct __timeval64
+timespec64_to_timeval64 (const struct __timespec64 ts64)
+{
+  struct __timeval64 tv64;
+
+  tv64.tv_sec = ts64.tv_sec;
+  tv64.tv_usec = ts64.tv_nsec / 1000;
+
+  return tv64;
+}
+
 /* Check if a value is in the valid nanoseconds range. Return true if
    it is, false otherwise.  */
 static inline bool
-- 
2.20.1

Re: [PATCH 5/6] y2038: Provide conversion helpers for struct __timeval64

[Alistair Francis]

On Sat, Jan 18, 2020 at 5:21 PM Lukasz Majewski <lukma@denx.de> wrote:
>
> Those functions allow easy conversion between Y2038 safe, glibc internal
> struct __timeval64 and other time related data structures (like struct timeval
> or struct __timespec64).
>
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  include/time.h | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>
> diff --git a/include/time.h b/include/time.h
> index 22ecacd0d8..b40214c006 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -304,6 +304,30 @@ valid_timeval_to_timespec64 (const struct timeval tv)
>    return ts64;
>  }
>
> +/* Convert a known valid struct timeval into a struct __timeval64.  */
> +static inline struct __timeval64
> +valid_timeval_to_timeval64 (const struct timeval tv)
> +{
> +  struct __timeval64 tv64;
> +
> +  tv64.tv_sec = tv.tv_sec;
> +  tv64.tv_usec = tv.tv_usec;
> +
> +  return tv64;
> +}
> +
> +/* Convert a struct __timeval64 into a struct __timespec64.  */
> +static inline struct __timespec64
> +timeval64_to_timespec64 (const struct __timeval64 tv64)
> +{
> +  struct __timespec64 ts64;
> +
> +  ts64.tv_sec = tv64.tv_sec;
> +  ts64.tv_nsec = tv64.tv_usec * 1000;
> +
> +  return ts64;
> +}
> +
>  /* Convert a known valid struct timespec into a struct __timespec64.  */
>  static inline struct __timespec64
>  valid_timespec_to_timespec64 (const struct timespec ts)
> @@ -342,6 +366,18 @@ valid_timespec64_to_timeval (const struct __timespec64 ts64)
>    return tv;
>  }
>
> +/* Convert a struct __timespec64 into a struct __timeval64.  */
> +static inline struct __timeval64
> +timespec64_to_timeval64 (const struct __timespec64 ts64)
> +{
> +  struct __timeval64 tv64;
> +
> +  tv64.tv_sec = ts64.tv_sec;
> +  tv64.tv_usec = ts64.tv_nsec / 1000;
> +
> +  return tv64;
> +}
> +
>  /* Check if a value is in the valid nanoseconds range. Return true if
>     it is, false otherwise.  */
>  static inline bool
> --
> 2.20.1
>

[PATCH 1/6] y2038: Use __clock_settime64 in deprecated stime function

[Lukasz Majewski]

Now, internally the deprecated stime uses __clock_settime64. This patch is
necessary for having architectures with __WORDSIZE == 32 &&
__TIMESIZE == 32 (like e.g. ARM32) Y2038 safe.

Build tests:
./src/scripts/build-many-glibcs.py glibcs
---
 time/stime.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/time/stime.c b/time/stime.c
index 576fa9c0c9..963902126b 100644
--- a/time/stime.c
+++ b/time/stime.c
@@ -27,11 +27,11 @@ int
 attribute_compat_text_section
 __stime (const time_t *when)
 {
-  struct timespec ts;
+  struct __timespec64 ts;
   ts.tv_sec = *when;
   ts.tv_nsec = 0;
 
-  return __clock_settime (CLOCK_REALTIME, &ts);
+  return __clock_settime64 (CLOCK_REALTIME, &ts);
 }
 
 compat_symbol (libc, __stime, stime, GLIBC_2_0);
-- 
2.20.1

Re: [PATCH 1/6] y2038: Use __clock_settime64 in deprecated stime function

[Adhemerval Zanella]

On 18/01/2020 04:20, Lukasz Majewski wrote:
> Now, internally the deprecated stime uses __clock_settime64. This patch is
> necessary for having architectures with __WORDSIZE == 32 &&
> __TIMESIZE == 32 (like e.g. ARM32) Y2038 safe.
> 
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
> ---
>  time/stime.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/time/stime.c b/time/stime.c
> index 576fa9c0c9..963902126b 100644
> --- a/time/stime.c
> +++ b/time/stime.c
> @@ -27,11 +27,11 @@ int
>  attribute_compat_text_section
>  __stime (const time_t *when)
>  {
> -  struct timespec ts;
> +  struct __timespec64 ts;
>    ts.tv_sec = *when;
>    ts.tv_nsec = 0;
>  
> -  return __clock_settime (CLOCK_REALTIME, &ts);
> +  return __clock_settime64 (CLOCK_REALTIME, &ts);
>  }
>  
>  compat_symbol (libc, __stime, stime, GLIBC_2_0);
> 

For !__ASSUME_TIME64_SYSCALLS builds time_t will continue to be 32 bits
and stime will continue to be non Y2038 safe.  Also, internally
__clock_settime will first try to issue __NR_clock_settime64, so current
code already accomplishes what this patch is aiming to do.

Re: [PATCH 1/6] y2038: Use __clock_settime64 in deprecated stime function

[Lukasz Majewski]

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

Hi Adhemerval,

> On 18/01/2020 04:20, Lukasz Majewski wrote:
> > Now, internally the deprecated stime uses __clock_settime64. This
> > patch is necessary for having architectures with __WORDSIZE == 32 &&
> > __TIMESIZE == 32 (like e.g. ARM32) Y2038 safe.
> > 
> > Build tests:
> > ./src/scripts/build-many-glibcs.py glibcs
> > ---
> >  time/stime.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/time/stime.c b/time/stime.c
> > index 576fa9c0c9..963902126b 100644
> > --- a/time/stime.c
> > +++ b/time/stime.c
> > @@ -27,11 +27,11 @@ int
> >  attribute_compat_text_section
> >  __stime (const time_t *when)
> >  {
> > -  struct timespec ts;
> > +  struct __timespec64 ts;
> >    ts.tv_sec = *when;
> >    ts.tv_nsec = 0;
> >  
> > -  return __clock_settime (CLOCK_REALTIME, &ts);
> > +  return __clock_settime64 (CLOCK_REALTIME, &ts);
> >  }
> >  
> >  compat_symbol (libc, __stime, stime, GLIBC_2_0);
> >   
> 
> For !__ASSUME_TIME64_SYSCALLS builds time_t will continue to be 32
> bits and stime will continue to be non Y2038 safe.

This patch is a preparatory work for introducing support of
-D_TIME_BITS=64 compilation flag when compiling programs with glibc [1].

Then the __USE_TIME_BITS64 is defined for exported glibc headers.
After this the time_t will become __time64_t on e.g. ARM32 bits
(__WORDSIZE==32 && __TIMESIZE==32).

As pointed out by Joseph [3], without such code we will not be able
atomically introduce _TIME_BITS=64 support.

>  Also, internally
> __clock_settime will first try to issue __NR_clock_settime64, so
> current code already accomplishes what this patch is aiming to do.

There is a use case when we do need to call __clock_settime64 though:

- ARM32 program is compiled with -D_TIME_BITS=64
- The clock_settime (or stime) internally uses __clock_settime() which
  in this case supports only 32 bit time API.

  To have this system Y2038 safe - one needs to call __clock_settime64
  explicitly to support passing arguments via struct __timespec64.

  This is true for all calls of __clock_settime/__clock_gettime
  (functions to be used internally by glibc, as we ought to switch to
  e.g. struct __timespec64).

Links:

[1] -
https://github.com/lmajewski/y2038_glibc/commit/e03c7188c72dd78d7141cf88f24dd2a5fd14c4f1#diff-a5ab6c74681eaf0569ed54f6bf0d7978R386

[2] -
https://github.com/lmajewski/y2038_glibc/commit/e03c7188c72dd78d7141cf88f24dd2a5fd14c4f1#diff-c051022b496f12bd9028edb46b8ec04dR7

[3] - https://sourceware.org/ml/libc-alpha/2020-01/msg00369.html

[4] -
https://github.com/lmajewski/y2038_glibc/blob/glibc_settimeofday_64bit_conversion_v1/sysdeps/unix/sysv/linux/clock_settime.c#L57



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 1/6] y2038: Use __clock_settime64 in deprecated stime function

[Adhemerval Zanella]

On 20/01/2020 14:58, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 18/01/2020 04:20, Lukasz Majewski wrote:
>>> Now, internally the deprecated stime uses __clock_settime64. This
>>> patch is necessary for having architectures with __WORDSIZE == 32 &&
>>> __TIMESIZE == 32 (like e.g. ARM32) Y2038 safe.
>>>
>>> Build tests:
>>> ./src/scripts/build-many-glibcs.py glibcs
>>> ---
>>>  time/stime.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/time/stime.c b/time/stime.c
>>> index 576fa9c0c9..963902126b 100644
>>> --- a/time/stime.c
>>> +++ b/time/stime.c
>>> @@ -27,11 +27,11 @@ int
>>>  attribute_compat_text_section
>>>  __stime (const time_t *when)
>>>  {
>>> -  struct timespec ts;
>>> +  struct __timespec64 ts;
>>>    ts.tv_sec = *when;
>>>    ts.tv_nsec = 0;
>>>  
>>> -  return __clock_settime (CLOCK_REALTIME, &ts);
>>> +  return __clock_settime64 (CLOCK_REALTIME, &ts);
>>>  }
>>>  
>>>  compat_symbol (libc, __stime, stime, GLIBC_2_0);
>>>   
>>
>> For !__ASSUME_TIME64_SYSCALLS builds time_t will continue to be 32
>> bits and stime will continue to be non Y2038 safe.
> 
> This patch is a preparatory work for introducing support of
> -D_TIME_BITS=64 compilation flag when compiling programs with glibc [1].
> 
> Then the __USE_TIME_BITS64 is defined for exported glibc headers.
> After this the time_t will become __time64_t on e.g. ARM32 bits
> (__WORDSIZE==32 && __TIMESIZE==32).> 
> As pointed out by Joseph [3], without such code we will not be able
> atomically introduce _TIME_BITS=64 support.

The _TIME_BITS=64 is a redirection for programs build against the glibc
provided headers, not for glibc build *itself*. Otherwise we will change
the ABI depending of the minimum supported kernel (which might define
__ASSUME_TIME64_SYSCALLS).

In this compatibility case we need to always assure the internal
time_t is 32 bits to architecture that provided such interface.


> 
>>  Also, internally
>> __clock_settime will first try to issue __NR_clock_settime64, so
>> current code already accomplishes what this patch is aiming to do.
> 
> There is a use case when we do need to call __clock_settime64 though:
> 
> - ARM32 program is compiled with -D_TIME_BITS=64
> - The clock_settime (or stime) internally uses __clock_settime() which
>   in this case supports only 32 bit time API.

Arm programs linked against old glibc will need to continue to call
stime with time_t being 32-bits.  The stime won't be exported for
-D_TIME_BITS=64, neither a stime64 will be implemented.

> 
>   To have this system Y2038 safe - one needs to call __clock_settime64
>   explicitly to support passing arguments via struct __timespec64.
> 
>   This is true for all calls of __clock_settime/__clock_gettime
>   (functions to be used internally by glibc, as we ought to switch to
>   e.g. struct __timespec64).

The idea of deprecate and make stime a compat symbol is exactly to avoid
need to provide a Y2038 version of it. Newer programs that aim to be
Y2038 safe will need to use clock_settime instead.


> 
> Links:
> 
> [1] -
> https://github.com/lmajewski/y2038_glibc/commit/e03c7188c72dd78d7141cf88f24dd2a5fd14c4f1#diff-a5ab6c74681eaf0569ed54f6bf0d7978R386
> 
> [2] -
> https://github.com/lmajewski/y2038_glibc/commit/e03c7188c72dd78d7141cf88f24dd2a5fd14c4f1#diff-c051022b496f12bd9028edb46b8ec04dR7
> 
> [3] - https://sourceware.org/ml/libc-alpha/2020-01/msg00369.html
> 
> [4] -
> https://github.com/lmajewski/y2038_glibc/blob/glibc_settimeofday_64bit_conversion_v1/sysdeps/unix/sysv/linux/clock_settime.c#L57
> 
> 
> 
> 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
> 
 

Re: [PATCH 1/6] y2038: Use __clock_settime64 in deprecated stime function

[Lukasz Majewski]

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

Hi Adhemerval,

> On 20/01/2020 14:58, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >   
> >> On 18/01/2020 04:20, Lukasz Majewski wrote:  
> >>> Now, internally the deprecated stime uses __clock_settime64. This
> >>> patch is necessary for having architectures with __WORDSIZE == 32
> >>> && __TIMESIZE == 32 (like e.g. ARM32) Y2038 safe.
> >>>
> >>> Build tests:
> >>> ./src/scripts/build-many-glibcs.py glibcs
> >>> ---
> >>>  time/stime.c | 4 ++--
> >>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/time/stime.c b/time/stime.c
> >>> index 576fa9c0c9..963902126b 100644
> >>> --- a/time/stime.c
> >>> +++ b/time/stime.c
> >>> @@ -27,11 +27,11 @@ int
> >>>  attribute_compat_text_section
> >>>  __stime (const time_t *when)
> >>>  {
> >>> -  struct timespec ts;
> >>> +  struct __timespec64 ts;
> >>>    ts.tv_sec = *when;
> >>>    ts.tv_nsec = 0;
> >>>  
> >>> -  return __clock_settime (CLOCK_REALTIME, &ts);
> >>> +  return __clock_settime64 (CLOCK_REALTIME, &ts);
> >>>  }
> >>>  
> >>>  compat_symbol (libc, __stime, stime, GLIBC_2_0);
> >>>     
> >>
> >> For !__ASSUME_TIME64_SYSCALLS builds time_t will continue to be 32
> >> bits and stime will continue to be non Y2038 safe.  
> > 
> > This patch is a preparatory work for introducing support of
> > -D_TIME_BITS=64 compilation flag when compiling programs with glibc
> > [1].
> > 
> > Then the __USE_TIME_BITS64 is defined for exported glibc headers.
> > After this the time_t will become __time64_t on e.g. ARM32 bits
> > (__WORDSIZE==32 && __TIMESIZE==32).> 
> > As pointed out by Joseph [3], without such code we will not be able
> > atomically introduce _TIME_BITS=64 support.  
> 
> The _TIME_BITS=64 is a redirection for programs build against the
> glibc provided headers, not for glibc build *itself*. Otherwise we
> will change the ABI depending of the minimum supported kernel (which
> might define __ASSUME_TIME64_SYSCALLS).
> 
> In this compatibility case we need to always assure the internal
> time_t is 32 bits to architecture that provided such interface.
> 
> 
> >   
> >>  Also, internally
> >> __clock_settime will first try to issue __NR_clock_settime64, so
> >> current code already accomplishes what this patch is aiming to do.
> >>  
> > 
> > There is a use case when we do need to call __clock_settime64
> > though:
> > 
> > - ARM32 program is compiled with -D_TIME_BITS=64
> > - The clock_settime (or stime) internally uses __clock_settime()
> > which in this case supports only 32 bit time API.  
> 
> Arm programs linked against old glibc will need to continue to call
> stime with time_t being 32-bits.  The stime won't be exported for
> -D_TIME_BITS=64, neither a stime64 will be implemented.

Ok.

> 
> > 
> >   To have this system Y2038 safe - one needs to call
> > __clock_settime64 explicitly to support passing arguments via
> > struct __timespec64.
> > 
> >   This is true for all calls of __clock_settime/__clock_gettime
> >   (functions to be used internally by glibc, as we ought to switch
> > to e.g. struct __timespec64).  
> 
> The idea of deprecate and make stime a compat symbol is exactly to
> avoid need to provide a Y2038 version of it. Newer programs that aim
> to be Y2038 safe will need to use clock_settime instead.

I've noted that stime is marked as "deprecated". If we can agree that
it will be removed from glibc soon (or at least before Y2038 :-)) or we
explicitly state that it will NOT be Y2038 safe, then indeed this patch
is not needed.

> 
> 
> > 
> > Links:
> > 
> > [1] -
> > https://github.com/lmajewski/y2038_glibc/commit/e03c7188c72dd78d7141cf88f24dd2a5fd14c4f1#diff-a5ab6c74681eaf0569ed54f6bf0d7978R386
> > 
> > [2] -
> > https://github.com/lmajewski/y2038_glibc/commit/e03c7188c72dd78d7141cf88f24dd2a5fd14c4f1#diff-c051022b496f12bd9028edb46b8ec04dR7
> > 
> > [3] - https://sourceware.org/ml/libc-alpha/2020-01/msg00369.html
> > 
> > [4] -
> > https://github.com/lmajewski/y2038_glibc/blob/glibc_settimeofday_64bit_conversion_v1/sysdeps/unix/sysv/linux/clock_settime.c#L57
> > 
> > 
> > 
> > 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 1/6] y2038: Use __clock_settime64 in deprecated stime function

[Adhemerval Zanella]

On 21/01/2020 13:54, Lukasz Majewski wrote:
> Hi Adhemerval,
>>>   To have this system Y2038 safe - one needs to call
>>> __clock_settime64 explicitly to support passing arguments via
>>> struct __timespec64.
>>>
>>>   This is true for all calls of __clock_settime/__clock_gettime
>>>   (functions to be used internally by glibc, as we ought to switch
>>> to e.g. struct __timespec64).  
>>
>> The idea of deprecate and make stime a compat symbol is exactly to
>> avoid need to provide a Y2038 version of it. Newer programs that aim
>> to be Y2038 safe will need to use clock_settime instead.
> 
> I've noted that stime is marked as "deprecated". If we can agree that
> it will be removed from glibc soon (or at least before Y2038 :-)) or we
> explicitly state that it will NOT be Y2038 safe, then indeed this patch
> is not needed.

The stime prototype is not exported any longer, so newer programs trying
to use it against a future glibc with 64 bit time_t support will fail. We
can't remove compatibility symbols, so the only case I see that Y2038 safe
program to incorrectly use stime would be it define it itself with a
wrong signature (stime (time64_t)), which is outside the scope on how
glibc can mitigate it.

[PATCH 4/6] y2038: alpha: Rename valid_timeval_to_timeval64 to valid_timeval32_to_timeval

[Lukasz Majewski]

Without this patch the naming convention for functions to convert
struct timeval32 to struct timeval (which supports 64 bit time on Alpha) was
a bit misleading. The name 'valid_timeval_to_timeval64' suggest conversion
of struct timeval to struct __timeval64 (as in ./include/time.h).

As on alpha the struct timeval supports 64 bit time it seems more readable
to emphasis struct timeval32 in the conversion function name.

Hence the helper function naming change to 'valid_timeval32_to_timeval'.
---
 sysdeps/unix/sysv/linux/alpha/osf_adjtime.c   | 4 ++--
 sysdeps/unix/sysv/linux/alpha/osf_setitimer.c | 4 ++--
 sysdeps/unix/sysv/linux/alpha/osf_utimes.c    | 4 ++--
 sysdeps/unix/sysv/linux/alpha/tv32-compat.h   | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
index cd864686f6..5ac72e252f 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
@@ -57,7 +57,7 @@ int
 attribute_compat_text_section
 __adjtime_tv32 (const struct timeval32 *itv, struct timeval32 *otv)
 {
-  struct timeval itv64 = valid_timeval_to_timeval64 (*itv);
+  struct timeval itv64 = valid_timeval32_to_timeval (*itv);
   struct timeval otv64;
 
   if (__adjtime (&itv64, &otv64) == -1)
@@ -91,7 +91,7 @@ __adjtimex_tv32 (struct timex32 *tx)
   tx64.calcnt    = tx->calcnt;
   tx64.errcnt    = tx->errcnt;
   tx64.stbcnt    = tx->stbcnt;
-  tx64.time      = valid_timeval_to_timeval64 (tx->time);
+  tx64.time      = valid_timeval32_to_timeval (tx->time);
 
   int status = __adjtimex (&tx64);
   if (status < 0)
diff --git a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
index 418efbf546..3935d1cfb5 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
@@ -30,9 +30,9 @@ __setitimer_tv32 (int which, const struct itimerval32 *restrict new_value,
 {
   struct itimerval new_value_64;
   new_value_64.it_interval
-    = valid_timeval_to_timeval64 (new_value->it_interval);
+    = valid_timeval32_to_timeval (new_value->it_interval);
   new_value_64.it_value
-    = valid_timeval_to_timeval64 (new_value->it_value);
+    = valid_timeval32_to_timeval (new_value->it_value);
 
   if (old_value == NULL)
     return __setitimer (which, &new_value_64, NULL);
diff --git a/sysdeps/unix/sysv/linux/alpha/osf_utimes.c b/sysdeps/unix/sysv/linux/alpha/osf_utimes.c
index 423c2a8ef2..6c3fad0132 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_utimes.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_utimes.c
@@ -28,8 +28,8 @@ attribute_compat_text_section
 __utimes_tv32 (const char *filename, const struct timeval32 times32[2])
 {
   struct timeval times[2];
-  times[0] = valid_timeval_to_timeval64 (times32[0]);
-  times[1] = valid_timeval_to_timeval64 (times32[1]);
+  times[0] = valid_timeval32_to_timeval (times32[0]);
+  times[1] = valid_timeval32_to_timeval (times32[1]);
   return __utimes (filename, times);
 }
 
diff --git a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h b/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
index 6076d2ec05..7169909259 100644
--- a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
+++ b/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
@@ -70,7 +70,7 @@ struct rusage32
    overflow, they write { INT32_MAX, TV_USEC_MAX } to the output.  */
 
 static inline struct timeval
-valid_timeval_to_timeval64 (const struct timeval32 tv)
+valid_timeval32_to_timeval (const struct timeval32 tv)
 {
   return (struct timeval) { tv.tv_sec, tv.tv_usec };
 }
-- 
2.20.1

Re: [PATCH 4/6] y2038: alpha: Rename valid_timeval_to_timeval64 to valid_timeval32_to_timeval

[Alistair Francis]

On Sat, Jan 18, 2020 at 5:21 PM Lukasz Majewski <lukma@denx.de> wrote:
>
> Without this patch the naming convention for functions to convert
> struct timeval32 to struct timeval (which supports 64 bit time on Alpha) was
> a bit misleading. The name 'valid_timeval_to_timeval64' suggest conversion
> of struct timeval to struct __timeval64 (as in ./include/time.h).
>
> As on alpha the struct timeval supports 64 bit time it seems more readable
> to emphasis struct timeval32 in the conversion function name.
>
> Hence the helper function naming change to 'valid_timeval32_to_timeval'.

Looks good.

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  sysdeps/unix/sysv/linux/alpha/osf_adjtime.c   | 4 ++--
>  sysdeps/unix/sysv/linux/alpha/osf_setitimer.c | 4 ++--
>  sysdeps/unix/sysv/linux/alpha/osf_utimes.c    | 4 ++--
>  sysdeps/unix/sysv/linux/alpha/tv32-compat.h   | 2 +-
>  4 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
> index cd864686f6..5ac72e252f 100644
> --- a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
> +++ b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
> @@ -57,7 +57,7 @@ int
>  attribute_compat_text_section
>  __adjtime_tv32 (const struct timeval32 *itv, struct timeval32 *otv)
>  {
> -  struct timeval itv64 = valid_timeval_to_timeval64 (*itv);
> +  struct timeval itv64 = valid_timeval32_to_timeval (*itv);
>    struct timeval otv64;
>
>    if (__adjtime (&itv64, &otv64) == -1)
> @@ -91,7 +91,7 @@ __adjtimex_tv32 (struct timex32 *tx)
>    tx64.calcnt    = tx->calcnt;
>    tx64.errcnt    = tx->errcnt;
>    tx64.stbcnt    = tx->stbcnt;
> -  tx64.time      = valid_timeval_to_timeval64 (tx->time);
> +  tx64.time      = valid_timeval32_to_timeval (tx->time);
>
>    int status = __adjtimex (&tx64);
>    if (status < 0)
> diff --git a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
> index 418efbf546..3935d1cfb5 100644
> --- a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
> +++ b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
> @@ -30,9 +30,9 @@ __setitimer_tv32 (int which, const struct itimerval32 *restrict new_value,
>  {
>    struct itimerval new_value_64;
>    new_value_64.it_interval
> -    = valid_timeval_to_timeval64 (new_value->it_interval);
> +    = valid_timeval32_to_timeval (new_value->it_interval);
>    new_value_64.it_value
> -    = valid_timeval_to_timeval64 (new_value->it_value);
> +    = valid_timeval32_to_timeval (new_value->it_value);
>
>    if (old_value == NULL)
>      return __setitimer (which, &new_value_64, NULL);
> diff --git a/sysdeps/unix/sysv/linux/alpha/osf_utimes.c b/sysdeps/unix/sysv/linux/alpha/osf_utimes.c
> index 423c2a8ef2..6c3fad0132 100644
> --- a/sysdeps/unix/sysv/linux/alpha/osf_utimes.c
> +++ b/sysdeps/unix/sysv/linux/alpha/osf_utimes.c
> @@ -28,8 +28,8 @@ attribute_compat_text_section
>  __utimes_tv32 (const char *filename, const struct timeval32 times32[2])
>  {
>    struct timeval times[2];
> -  times[0] = valid_timeval_to_timeval64 (times32[0]);
> -  times[1] = valid_timeval_to_timeval64 (times32[1]);
> +  times[0] = valid_timeval32_to_timeval (times32[0]);
> +  times[1] = valid_timeval32_to_timeval (times32[1]);
>    return __utimes (filename, times);
>  }
>
> diff --git a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h b/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
> index 6076d2ec05..7169909259 100644
> --- a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
> +++ b/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
> @@ -70,7 +70,7 @@ struct rusage32
>     overflow, they write { INT32_MAX, TV_USEC_MAX } to the output.  */
>
>  static inline struct timeval
> -valid_timeval_to_timeval64 (const struct timeval32 tv)
> +valid_timeval32_to_timeval (const struct timeval32 tv)
>  {
>    return (struct timeval) { tv.tv_sec, tv.tv_usec };
>  }
> --
> 2.20.1
>

[PATCH 6/6] y2038: linux: Provide __settimeofday64 implementation

[Lukasz Majewski]

This patch provides new __settimeofday64 explicit 64 bit function for setting
64 bit time in the kernel (by internally calling __clock_settime64).
Moreover, a 32 bit version - __settimeofday has been refactored to internally
use __settimeofday64.

The __settimeofday is now supposed to be used on systems still supporting 32
bit time (__TIMESIZE != 64) - hence the necessary conversion of struct
timeval to 64 bit struct __timespec64.

Internally the settimeofday uses __settimeofday64. This patch is necessary
for having architectures with __WORDSIZE == 32 Y2038 safe.

Build tests:
./src/scripts/build-many-glibcs.py glibcs

Run-time tests:
- Run specific tests on ARM/x86 32bit systems (qemu):
  https://github.com/lmajewski/meta-y2038 and run tests:
  https://github.com/lmajewski/y2038-tests/commits/master

Above tests were performed with Y2038 redirection applied as well as without
to test proper usage of both __settimeofday64 and __settimeofday.
---
 include/time.h      |  9 +++++++++
 time/settimeofday.c | 19 +++++++++++++++----
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/include/time.h b/include/time.h
index b40214c006..b76ffea225 100644
--- a/include/time.h
+++ b/include/time.h
@@ -8,6 +8,7 @@
 # include <time/mktime-internal.h>
 # include <endian.h>
 # include <time-clockid.h>
+# include <sys/time.h>
 
 extern __typeof (strftime_l) __strftime_l;
 libc_hidden_proto (__strftime_l)
@@ -224,6 +225,14 @@ extern int __sched_rr_get_interval64 (pid_t pid, struct __timespec64 *tp);
 libc_hidden_proto (__sched_rr_get_interval64);
 #endif
 
+#if __TIMESIZE == 64
+# define __settimeofday64 __settimeofday
+#else
+extern int __settimeofday64 (const struct __timeval64 *tv,
+                             const struct timezone *tz);
+libc_hidden_proto (__settimeofday64)
+#endif
+
 /* Compute the `struct tm' representation of T,
    offset OFFSET seconds east of UTC,
    and store year, yday, mon, mday, wday, hour, min, sec into *TP.
diff --git a/time/settimeofday.c b/time/settimeofday.c
index bc43e70f61..dbf6cf501f 100644
--- a/time/settimeofday.c
+++ b/time/settimeofday.c
@@ -22,7 +22,7 @@
 /* Set the current time of day and timezone information.
    This call is restricted to the super-user.  */
 int
-__settimeofday (const struct timeval *tv, const struct timezone *tz)
+__settimeofday64 (const struct __timeval64 *tv, const struct timezone *tz)
 {
   if (__glibc_unlikely (tz != 0))
     {
@@ -34,11 +34,22 @@ __settimeofday (const struct timeval *tv, const struct timezone *tz)
       return __settimezone (tz);
     }
 
-  struct timespec ts;
-  TIMEVAL_TO_TIMESPEC (tv, &ts);
-  return __clock_settime (CLOCK_REALTIME, &ts);
+  struct __timespec64 ts = timeval64_to_timespec64 (*tv);
+  return __clock_settime64 (CLOCK_REALTIME, &ts);
 }
 
+#if __TIMESIZE != 64
+libc_hidden_def (__settimeofday64)
+
+int
+__settimeofday (const struct timeval *tv, const struct timezone *tz)
+{
+  struct __timeval64 tv64 = valid_timeval_to_timeval64 (*tv);
+
+  return __settimeofday64 (&tv64, tz);
+}
+#endif
+
 #ifdef VERSION_settimeofday
 weak_alias (__settimeofday, __settimeofday_w);
 default_symbol_version (__settimeofday_w, settimeofday, VERSION_settimeofday);
-- 
2.20.1

Re: [PATCH 6/6] y2038: linux: Provide __settimeofday64 implementation

[Alistair Francis]

On Sat, Jan 18, 2020 at 5:21 PM Lukasz Majewski <lukma@denx.de> wrote:
>
> This patch provides new __settimeofday64 explicit 64 bit function for setting
> 64 bit time in the kernel (by internally calling __clock_settime64).
> Moreover, a 32 bit version - __settimeofday has been refactored to internally
> use __settimeofday64.
>
> The __settimeofday is now supposed to be used on systems still supporting 32
> bit time (__TIMESIZE != 64) - hence the necessary conversion of struct
> timeval to 64 bit struct __timespec64.
>
> Internally the settimeofday uses __settimeofday64. This patch is necessary
> for having architectures with __WORDSIZE == 32 Y2038 safe.
>
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
>
> Run-time tests:
> - Run specific tests on ARM/x86 32bit systems (qemu):
>   https://github.com/lmajewski/meta-y2038 and run tests:
>   https://github.com/lmajewski/y2038-tests/commits/master
>
> Above tests were performed with Y2038 redirection applied as well as without
> to test proper usage of both __settimeofday64 and __settimeofday.

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  include/time.h      |  9 +++++++++
>  time/settimeofday.c | 19 +++++++++++++++----
>  2 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/include/time.h b/include/time.h
> index b40214c006..b76ffea225 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -8,6 +8,7 @@
>  # include <time/mktime-internal.h>
>  # include <endian.h>
>  # include <time-clockid.h>
> +# include <sys/time.h>
>
>  extern __typeof (strftime_l) __strftime_l;
>  libc_hidden_proto (__strftime_l)
> @@ -224,6 +225,14 @@ extern int __sched_rr_get_interval64 (pid_t pid, struct __timespec64 *tp);
>  libc_hidden_proto (__sched_rr_get_interval64);
>  #endif
>
> +#if __TIMESIZE == 64
> +# define __settimeofday64 __settimeofday
> +#else
> +extern int __settimeofday64 (const struct __timeval64 *tv,
> +                             const struct timezone *tz);
> +libc_hidden_proto (__settimeofday64)
> +#endif
> +
>  /* Compute the `struct tm' representation of T,
>     offset OFFSET seconds east of UTC,
>     and store year, yday, mon, mday, wday, hour, min, sec into *TP.
> diff --git a/time/settimeofday.c b/time/settimeofday.c
> index bc43e70f61..dbf6cf501f 100644
> --- a/time/settimeofday.c
> +++ b/time/settimeofday.c
> @@ -22,7 +22,7 @@
>  /* Set the current time of day and timezone information.
>     This call is restricted to the super-user.  */
>  int
> -__settimeofday (const struct timeval *tv, const struct timezone *tz)
> +__settimeofday64 (const struct __timeval64 *tv, const struct timezone *tz)
>  {
>    if (__glibc_unlikely (tz != 0))
>      {
> @@ -34,11 +34,22 @@ __settimeofday (const struct timeval *tv, const struct timezone *tz)
>        return __settimezone (tz);
>      }
>
> -  struct timespec ts;
> -  TIMEVAL_TO_TIMESPEC (tv, &ts);
> -  return __clock_settime (CLOCK_REALTIME, &ts);
> +  struct __timespec64 ts = timeval64_to_timespec64 (*tv);
> +  return __clock_settime64 (CLOCK_REALTIME, &ts);
>  }
>
> +#if __TIMESIZE != 64
> +libc_hidden_def (__settimeofday64)
> +
> +int
> +__settimeofday (const struct timeval *tv, const struct timezone *tz)
> +{
> +  struct __timeval64 tv64 = valid_timeval_to_timeval64 (*tv);
> +
> +  return __settimeofday64 (&tv64, tz);
> +}
> +#endif
> +
>  #ifdef VERSION_settimeofday
>  weak_alias (__settimeofday, __settimeofday_w);
>  default_symbol_version (__settimeofday_w, settimeofday, VERSION_settimeofday);
> --
> 2.20.1
>

[PATCH 2/6] y2038: hurd: Provide __clock_settime64 function

[Lukasz Majewski]

For Linux glibc ports the __TIMESIZE == 64 ensures proper aliasing for
__clock_settime64 (to __clock_settime).
When __TIMESIZE != 64 (like ARM32, PPC) the glibc expects separate definition
of the __clock_settime64.

The HURD port only provides __clock_settime, so this patch adds
__clock_settime64 as a tiny wrapper on it.
---
 sysdeps/mach/hurd/clock_settime.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/sysdeps/mach/hurd/clock_settime.c b/sysdeps/mach/hurd/clock_settime.c
index 2c77bad71a..db1ba860dc 100644
--- a/sysdeps/mach/hurd/clock_settime.c
+++ b/sysdeps/mach/hurd/clock_settime.c
@@ -53,3 +53,12 @@ versioned_symbol (libc, __clock_settime, clock_settime, GLIBC_2_17);
 strong_alias (__clock_settime, __clock_settime_2);
 compat_symbol (libc, __clock_settime_2, clock_settime, GLIBC_2_2);
 #endif
+
+int
+__clock_settime64 (clockid_t clock_id, const struct __timespec64 *ts64)
+{
+  struct timespec ts = valid_timespec64_to_timespec (*ts64);
+
+  return __clock_settime (clock_id, &ts);
+}
+libc_hidden_def (__clock_settime64)
-- 
2.20.1

Re: [PATCH 2/6] y2038: hurd: Provide __clock_settime64 function

[Adhemerval Zanella]

On 18/01/2020 04:20, Lukasz Majewski wrote:
> For Linux glibc ports the __TIMESIZE == 64 ensures proper aliasing for
> __clock_settime64 (to __clock_settime).
> When __TIMESIZE != 64 (like ARM32, PPC) the glibc expects separate definition
> of the __clock_settime64.
> 
> The HURD port only provides __clock_settime, so this patch adds
> __clock_settime64 as a tiny wrapper on it.
> ---
>  sysdeps/mach/hurd/clock_settime.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/sysdeps/mach/hurd/clock_settime.c b/sysdeps/mach/hurd/clock_settime.c
> index 2c77bad71a..db1ba860dc 100644
> --- a/sysdeps/mach/hurd/clock_settime.c
> +++ b/sysdeps/mach/hurd/clock_settime.c
> @@ -53,3 +53,12 @@ versioned_symbol (libc, __clock_settime, clock_settime, GLIBC_2_17);
>  strong_alias (__clock_settime, __clock_settime_2);
>  compat_symbol (libc, __clock_settime_2, clock_settime, GLIBC_2_2);
>  #endif
> +
> +int
> +__clock_settime64 (clockid_t clock_id, const struct __timespec64 *ts64)
> +{
> +  struct timespec ts = valid_timespec64_to_timespec (*ts64);
> +
> +  return __clock_settime (clock_id, &ts);
> +}
> +libc_hidden_def (__clock_settime64)
> 

As for https://sourceware.org/ml/libc-alpha/2020-01/msg00445.html I don't
think this patch is really required now.

Re: [PATCH 2/6] y2038: hurd: Provide __clock_settime64 function

[Lukasz Majewski]

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

Hi Adhemerval,

> On 18/01/2020 04:20, Lukasz Majewski wrote:
> > For Linux glibc ports the __TIMESIZE == 64 ensures proper aliasing
> > for __clock_settime64 (to __clock_settime).
> > When __TIMESIZE != 64 (like ARM32, PPC) the glibc expects separate
> > definition of the __clock_settime64.
> > 
> > The HURD port only provides __clock_settime, so this patch adds
> > __clock_settime64 as a tiny wrapper on it.
> > ---
> >  sysdeps/mach/hurd/clock_settime.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/sysdeps/mach/hurd/clock_settime.c
> > b/sysdeps/mach/hurd/clock_settime.c index 2c77bad71a..db1ba860dc
> > 100644 --- a/sysdeps/mach/hurd/clock_settime.c
> > +++ b/sysdeps/mach/hurd/clock_settime.c
> > @@ -53,3 +53,12 @@ versioned_symbol (libc, __clock_settime,
> > clock_settime, GLIBC_2_17); strong_alias (__clock_settime,
> > __clock_settime_2); compat_symbol (libc, __clock_settime_2,
> > clock_settime, GLIBC_2_2); #endif
> > +
> > +int
> > +__clock_settime64 (clockid_t clock_id, const struct __timespec64
> > *ts64) +{
> > +  struct timespec ts = valid_timespec64_to_timespec (*ts64);
> > +
> > +  return __clock_settime (clock_id, &ts);
> > +}
> > +libc_hidden_def (__clock_settime64)
> >   
> 
> As for https://sourceware.org/ml/libc-alpha/2020-01/msg00445.html I
> don't think this patch is really required now.

I think that it is required to allow using __clock_settime64() calls
internally in glibc.

The __clock_settime64 will be aliased to __clock_settime for archs with
__TIMESIZE==64 [*]. It is important for archs with Y2038 support - it
will allow in-glibc internal use of 64 bit time (i.e. by using struct
__timespec64).

Ports which are using Linux kernel already have it defined. The problem
is with HURD, which misses this definition.

(Please correct me if I'm wrong - but it seems to me that HURD is not
using 64 bit time?)

I thought about some kind of alias (weak, strong, #define), but the
problem was with passed data types - as __clock_settime64 requires
struct __timespec64.


Note:

[*] - __TIMESIZE indicates the default size of time_t (but on Y2038
safe ARM32 bit systems we do have __TIMESIZE=32 but we do have time_t
defined as __time64_t [1]).

Links:

[1] -
https://github.com/lmajewski/y2038_glibc/commit/e03c7188c72dd78d7141cf88f24dd2a5fd14c4f1#diff-c051022b496f12bd9028edb46b8ec04dR7




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 2/6] y2038: hurd: Provide __clock_settime64 function

[Adhemerval Zanella]

On 20/01/2020 15:08, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 18/01/2020 04:20, Lukasz Majewski wrote:
>>> For Linux glibc ports the __TIMESIZE == 64 ensures proper aliasing
>>> for __clock_settime64 (to __clock_settime).
>>> When __TIMESIZE != 64 (like ARM32, PPC) the glibc expects separate
>>> definition of the __clock_settime64.
>>>
>>> The HURD port only provides __clock_settime, so this patch adds
>>> __clock_settime64 as a tiny wrapper on it.
>>> ---
>>>  sysdeps/mach/hurd/clock_settime.c | 9 +++++++++
>>>  1 file changed, 9 insertions(+)
>>>
>>> diff --git a/sysdeps/mach/hurd/clock_settime.c
>>> b/sysdeps/mach/hurd/clock_settime.c index 2c77bad71a..db1ba860dc
>>> 100644 --- a/sysdeps/mach/hurd/clock_settime.c
>>> +++ b/sysdeps/mach/hurd/clock_settime.c
>>> @@ -53,3 +53,12 @@ versioned_symbol (libc, __clock_settime,
>>> clock_settime, GLIBC_2_17); strong_alias (__clock_settime,
>>> __clock_settime_2); compat_symbol (libc, __clock_settime_2,
>>> clock_settime, GLIBC_2_2); #endif
>>> +
>>> +int
>>> +__clock_settime64 (clockid_t clock_id, const struct __timespec64
>>> *ts64) +{
>>> +  struct timespec ts = valid_timespec64_to_timespec (*ts64);
>>> +
>>> +  return __clock_settime (clock_id, &ts);
>>> +}
>>> +libc_hidden_def (__clock_settime64)
>>>   
>>
>> As for https://sourceware.org/ml/libc-alpha/2020-01/msg00445.html I
>> don't think this patch is really required now.
> 
> I think that it is required to allow using __clock_settime64() calls
> internally in glibc.


Ok, I missed the patch where you adjusted the generic settimeofday.
In any case, I would like to avoid mess with generic implementation
for time64 since I am not sure how/when Hurd will try to fix it and
currently all time64 fixes are compartmentalized on Linux sysdep
folder.

I think it might be add Linux specific version of settimeofday to 
handle the time64 support.

Re: [PATCH 2/6] y2038: hurd: Provide __clock_settime64 function

[Lukasz Majewski]

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

Hi Adhemerval,

> On 20/01/2020 15:08, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >   
> >> On 18/01/2020 04:20, Lukasz Majewski wrote:  
> >>> For Linux glibc ports the __TIMESIZE == 64 ensures proper aliasing
> >>> for __clock_settime64 (to __clock_settime).
> >>> When __TIMESIZE != 64 (like ARM32, PPC) the glibc expects separate
> >>> definition of the __clock_settime64.
> >>>
> >>> The HURD port only provides __clock_settime, so this patch adds
> >>> __clock_settime64 as a tiny wrapper on it.
> >>> ---
> >>>  sysdeps/mach/hurd/clock_settime.c | 9 +++++++++
> >>>  1 file changed, 9 insertions(+)
> >>>
> >>> diff --git a/sysdeps/mach/hurd/clock_settime.c
> >>> b/sysdeps/mach/hurd/clock_settime.c index 2c77bad71a..db1ba860dc
> >>> 100644 --- a/sysdeps/mach/hurd/clock_settime.c
> >>> +++ b/sysdeps/mach/hurd/clock_settime.c
> >>> @@ -53,3 +53,12 @@ versioned_symbol (libc, __clock_settime,
> >>> clock_settime, GLIBC_2_17); strong_alias (__clock_settime,
> >>> __clock_settime_2); compat_symbol (libc, __clock_settime_2,
> >>> clock_settime, GLIBC_2_2); #endif
> >>> +
> >>> +int
> >>> +__clock_settime64 (clockid_t clock_id, const struct __timespec64
> >>> *ts64) +{
> >>> +  struct timespec ts = valid_timespec64_to_timespec (*ts64);
> >>> +
> >>> +  return __clock_settime (clock_id, &ts);
> >>> +}
> >>> +libc_hidden_def (__clock_settime64)
> >>>     
> >>
> >> As for https://sourceware.org/ml/libc-alpha/2020-01/msg00445.html I
> >> don't think this patch is really required now.  
> > 
> > I think that it is required to allow using __clock_settime64() calls
> > internally in glibc.  
> 
> 
> Ok, I missed the patch where you adjusted the generic settimeofday.
> In any case, I would like to avoid mess with generic implementation
> for time64 since I am not sure how/when Hurd will try to fix it and
> currently all time64 fixes are compartmentalized on Linux sysdep
> folder.
> 
> I think it might be add Linux specific version of settimeofday to 
> handle the time64 support.

Please correct me if I'm wrong - you suggest moving the code from this
patch (to convert settimeofday to __settimeofday64) to
sysdeps/unix/sysv/linux/settimeofday.c and do not modify
./time/settimeofday.c.

In that way there would be no need to add any code (regarding time64)
to HURD at all?


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 2/6] y2038: hurd: Provide __clock_settime64 function

[Adhemerval Zanella]

On 21/01/2020 13:34, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 20/01/2020 15:08, Lukasz Majewski wrote:
>>> Hi Adhemerval,
>>>   
>>>> On 18/01/2020 04:20, Lukasz Majewski wrote:  
>>>>> For Linux glibc ports the __TIMESIZE == 64 ensures proper aliasing
>>>>> for __clock_settime64 (to __clock_settime).
>>>>> When __TIMESIZE != 64 (like ARM32, PPC) the glibc expects separate
>>>>> definition of the __clock_settime64.
>>>>>
>>>>> The HURD port only provides __clock_settime, so this patch adds
>>>>> __clock_settime64 as a tiny wrapper on it.
>>>>> ---
>>>>>  sysdeps/mach/hurd/clock_settime.c | 9 +++++++++
>>>>>  1 file changed, 9 insertions(+)
>>>>>
>>>>> diff --git a/sysdeps/mach/hurd/clock_settime.c
>>>>> b/sysdeps/mach/hurd/clock_settime.c index 2c77bad71a..db1ba860dc
>>>>> 100644 --- a/sysdeps/mach/hurd/clock_settime.c
>>>>> +++ b/sysdeps/mach/hurd/clock_settime.c
>>>>> @@ -53,3 +53,12 @@ versioned_symbol (libc, __clock_settime,
>>>>> clock_settime, GLIBC_2_17); strong_alias (__clock_settime,
>>>>> __clock_settime_2); compat_symbol (libc, __clock_settime_2,
>>>>> clock_settime, GLIBC_2_2); #endif
>>>>> +
>>>>> +int
>>>>> +__clock_settime64 (clockid_t clock_id, const struct __timespec64
>>>>> *ts64) +{
>>>>> +  struct timespec ts = valid_timespec64_to_timespec (*ts64);
>>>>> +
>>>>> +  return __clock_settime (clock_id, &ts);
>>>>> +}
>>>>> +libc_hidden_def (__clock_settime64)
>>>>>     
>>>>
>>>> As for https://sourceware.org/ml/libc-alpha/2020-01/msg00445.html I
>>>> don't think this patch is really required now.  
>>>
>>> I think that it is required to allow using __clock_settime64() calls
>>> internally in glibc.  
>>
>>
>> Ok, I missed the patch where you adjusted the generic settimeofday.
>> In any case, I would like to avoid mess with generic implementation
>> for time64 since I am not sure how/when Hurd will try to fix it and
>> currently all time64 fixes are compartmentalized on Linux sysdep
>> folder.
>>
>> I think it might be add Linux specific version of settimeofday to 
>> handle the time64 support.
> 
> Please correct me if I'm wrong - you suggest moving the code from this
> patch (to convert settimeofday to __settimeofday64) to
> sysdeps/unix/sysv/linux/settimeofday.c and do not modify
> ./time/settimeofday.c.
> 
> In that way there would be no need to add any code (regarding time64)
> to HURD at all?
> 

Yes, I would like to change the generic files for time64_t support once
Hurd wants to actually pursuit it. What I would like is to eventually
sets the generic interface to assume 64 bit time_t, as currently the
generic bits/typesizes.h still sets __TIME_T_TYPE as __SLONGWORD_TYPE.

Re: [PATCH 2/6] y2038: hurd: Provide __clock_settime64 function

[Samuel Thibault]

Adhemerval Zanella, le mar. 21 janv. 2020 14:20:12 -0300, a ecrit:
> On 21/01/2020 13:34, Lukasz Majewski wrote:
> > In that way there would be no need to add any code (regarding time64)
> > to HURD at all?
> 
> Yes, I would like to change the generic files for time64_t support once
> Hurd wants to actually pursuit it.

Yes, I'd like to work on it indeed.

Samuel

Re: [PATCH 2/6] y2038: hurd: Provide __clock_settime64 function

[Samuel Thibault]

Hello,

Adhemerval Zanella, le mar. 21 janv. 2020 14:20:12 -0300, a ecrit:
> On 21/01/2020 13:34, Lukasz Majewski wrote:
> > In that way there would be no need to add any code (regarding time64)
> > to HURD at all?
> > 
> 
> Yes, I would like to change the generic files for time64_t support once
> Hurd wants to actually pursuit it. What I would like is to eventually
> sets the generic interface to assume 64 bit time_t, as currently the
> generic bits/typesizes.h still sets __TIME_T_TYPE as __SLONGWORD_TYPE.

Yes, please do this for generic, it's simpler if we can get the
migration done together. It is fine to introduce __clock_get/settime64
which just convert between 32 and 64, and we'll plug a 64bit kernel
interface soon.

Samuel

[PATCH 3/6] y2038: Introduce struct __timeval64 - new internal glibc type

[Lukasz Majewski]

This type is a glibc's "internal" type similar to struct timeval but
whose tv_sec field is a __time64_t rather than a time_t, which makes it
Y2038-proof. This struct is NOT supposed to be passed to the kernel -
instead it shall be converted to struct __timespec64 and clock_[sg]ettime
syscalls shall be used (which are now Y2038 safe).

Build tests:
./src/scripts/build-many-glibcs.py glibcs
---
 include/time.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/time.h b/include/time.h
index 047f431a1a..22ecacd0d8 100644
--- a/include/time.h
+++ b/include/time.h
@@ -93,6 +93,20 @@ struct __itimerspec64
 };
 #endif
 
+#if __TIMESIZE == 64
+# define __timeval64 timeval
+#else
+/* The glibc Y2038-proof struct __timeval64 structure for a time value.
+   This structure is NOT supposed to be passed to the Linux kernel.
+   Instead, it shall be converted to struct __timespec64 and time shall
+   be [sg]et via clock_[sg]ettime (which are now Y2038 safe).  */
+struct __timeval64
+{
+  __time64_t tv_sec;         /* Seconds */
+  __suseconds_t tv_usec;       /* Microseconds */
+};
+#endif
+
 #if __TIMESIZE == 64
 # define __ctime64 ctime
 #else
-- 
2.20.1

Re: [PATCH 3/6] y2038: Introduce struct __timeval64 - new internal glibc type

[Alistair Francis]

On Sat, Jan 18, 2020 at 5:21 PM Lukasz Majewski <lukma@denx.de> wrote:
>
> This type is a glibc's "internal" type similar to struct timeval but
> whose tv_sec field is a __time64_t rather than a time_t, which makes it
> Y2038-proof. This struct is NOT supposed to be passed to the kernel -
> instead it shall be converted to struct __timespec64 and clock_[sg]ettime
> syscalls shall be used (which are now Y2038 safe).
>
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs

Looks good, I have the same thing in my tree :)

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  include/time.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/include/time.h b/include/time.h
> index 047f431a1a..22ecacd0d8 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -93,6 +93,20 @@ struct __itimerspec64
>  };
>  #endif
>
> +#if __TIMESIZE == 64
> +# define __timeval64 timeval
> +#else
> +/* The glibc Y2038-proof struct __timeval64 structure for a time value.
> +   This structure is NOT supposed to be passed to the Linux kernel.
> +   Instead, it shall be converted to struct __timespec64 and time shall
> +   be [sg]et via clock_[sg]ettime (which are now Y2038 safe).  */
> +struct __timeval64
> +{
> +  __time64_t tv_sec;         /* Seconds */
> +  __suseconds_t tv_usec;       /* Microseconds */
> +};
> +#endif
> +
>  #if __TIMESIZE == 64
>  # define __ctime64 ctime
>  #else
> --
> 2.20.1
>

Re: [PATCH 3/6] y2038: Introduce struct __timeval64 - new internal glibc type

[Arnd Bergmann]

On Sat, Jan 18, 2020 at 11:48 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Sat, Jan 18, 2020 at 5:21 PM Lukasz Majewski <lukma@denx.de> wrote:
> >
> > This type is a glibc's "internal" type similar to struct timeval but
> > whose tv_sec field is a __time64_t rather than a time_t, which makes it
> > Y2038-proof. This struct is NOT supposed to be passed to the kernel -
> > instead it shall be converted to struct __timespec64 and clock_[sg]ettime
> > syscalls shall be used (which are now Y2038 safe).
> >
> > Build tests:
> > ./src/scripts/build-many-glibcs.py glibcs
>
> Looks good, I have the same thing in my tree :)

I think it's slightly different: __suseconds_t on rv32 is 64 bit wide, while
on all existing 32-bit architectures as well as sparc64 it is 32-bit wide.

While almost all kernel interfaces based on timeval have been replaced
by those based on timespec, there are a small number that need this
to be compatible with the kernel's layout, or to have an explicit conversion:

- Socket timestamps with SO_TIMESTAMP
- Socket timeouts with SO_RCVTIMEO/SO_SNDTIMEO
- pc-style parallel ports with the PPGETTIME/PPSETTIME ioctls
- video4linux with the VIDIOC_QUERYBUF/VIDIOC_QBUF/VIDIOC_DQBUF
  VIDIOC_PREPARE_BUF and VIDIOC_OMAP3ISP_STAT_REQ ioctls

       Arnd

Re: [PATCH 3/6] y2038: Introduce struct __timeval64 - new internal glibc type

[Lukasz Majewski]

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

Hi Arnd,

> On Sat, Jan 18, 2020 at 11:48 PM Alistair Francis
> <alistair23@gmail.com> wrote:
> >
> > On Sat, Jan 18, 2020 at 5:21 PM Lukasz Majewski <lukma@denx.de>
> > wrote:  
> > >
> > > This type is a glibc's "internal" type similar to struct timeval
> > > but whose tv_sec field is a __time64_t rather than a time_t,
> > > which makes it Y2038-proof. This struct is NOT supposed to be
> > > passed to the kernel - instead it shall be converted to struct
> > > __timespec64 and clock_[sg]ettime syscalls shall be used (which
> > > are now Y2038 safe).
> > >
> > > Build tests:
> > > ./src/scripts/build-many-glibcs.py glibcs  
> >
> > Looks good, I have the same thing in my tree :)  
> 
> I think it's slightly different: __suseconds_t on rv32 is 64 bit
> wide, while on all existing 32-bit architectures as well as sparc64
> it is 32-bit wide.
> 
> While almost all kernel interfaces based on timeval have been replaced
> by those based on timespec, there are a small number that need this
> to be compatible with the kernel's layout, or to have an explicit
> conversion:
> 
> - Socket timestamps with SO_TIMESTAMP
> - Socket timeouts with SO_RCVTIMEO/SO_SNDTIMEO
> - pc-style parallel ports with the PPGETTIME/PPSETTIME ioctls
> - video4linux with the VIDIOC_QUERYBUF/VIDIOC_QBUF/VIDIOC_DQBUF
>   VIDIOC_PREPARE_BUF and VIDIOC_OMAP3ISP_STAT_REQ ioctls
> 
>        Arnd

I've kept the __suseconds_t type from the original definition of struct
timeval.

Moreover, as I've noted in the commit message - the struct __timeval64
is NOT supposed to be passed directly to Linux kernel and shall be
explicitly converted if needed (thanks Arnd for pointing out exact
situations where conversion will be needed).

Or maybe somebody has better idea? Comments are more than welcome.

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 3/6] y2038: Introduce struct __timeval64 - new internal glibc type

[Arnd Bergmann]

On Sun, Jan 19, 2020 at 4:22 PM Lukasz Majewski <lukma@denx.de> wrote:
> > On Sat, Jan 18, 2020 at 11:48 PM Alistair Francis
> > <alistair23@gmail.com> wrote:
> >
> > - Socket timestamps with SO_TIMESTAMP
> > - Socket timeouts with SO_RCVTIMEO/SO_SNDTIMEO
> > - pc-style parallel ports with the PPGETTIME/PPSETTIME ioctls
> > - video4linux with the VIDIOC_QUERYBUF/VIDIOC_QBUF/VIDIOC_DQBUF
> >   VIDIOC_PREPARE_BUF and VIDIOC_OMAP3ISP_STAT_REQ ioctls
> >
> >        Arnd
>
> I've kept the __suseconds_t type from the original definition of struct
> timeval.
>
> Moreover, as I've noted in the commit message - the struct __timeval64
> is NOT supposed to be passed directly to Linux kernel and shall be
> explicitly converted if needed (thanks Arnd for pointing out exact
> situations where conversion will be needed).
>
> Or maybe somebody has better idea? Comments are more than welcome.

musl went to a 64-bit suseconds_t, same as the riscv32 port on glibc.

It would certainly help if this could be consistent across architectures
and libraries.

      Arnd

Re: [PATCH 3/6] y2038: Introduce struct __timeval64 - new internal glibc type

[Lukasz Majewski]

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

Hi Arnd,

> On Sun, Jan 19, 2020 at 4:22 PM Lukasz Majewski <lukma@denx.de> wrote:
> > > On Sat, Jan 18, 2020 at 11:48 PM Alistair Francis
> > > <alistair23@gmail.com> wrote:
> > >
> > > - Socket timestamps with SO_TIMESTAMP
> > > - Socket timeouts with SO_RCVTIMEO/SO_SNDTIMEO
> > > - pc-style parallel ports with the PPGETTIME/PPSETTIME ioctls
> > > - video4linux with the VIDIOC_QUERYBUF/VIDIOC_QBUF/VIDIOC_DQBUF
> > >   VIDIOC_PREPARE_BUF and VIDIOC_OMAP3ISP_STAT_REQ ioctls
> > >
> > >        Arnd  
> >
> > I've kept the __suseconds_t type from the original definition of
> > struct timeval.
> >
> > Moreover, as I've noted in the commit message - the struct
> > __timeval64 is NOT supposed to be passed directly to Linux kernel
> > and shall be explicitly converted if needed (thanks Arnd for
> > pointing out exact situations where conversion will be needed).
> >
> > Or maybe somebody has better idea? Comments are more than welcome.  
> 
> musl went to a 64-bit suseconds_t, same as the riscv32 port on glibc.
> 
> It would certainly help if this could be consistent across
> architectures and libraries.

Ok. Thanks for the input.

> 
>       Arnd




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 3/6] y2038: Introduce struct __timeval64 - new internal glibc type

[Lukasz Majewski]

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

Hi Arnd,

> Hi Arnd,
> 
> > On Sun, Jan 19, 2020 at 4:22 PM Lukasz Majewski <lukma@denx.de>
> > wrote:  
> > > > On Sat, Jan 18, 2020 at 11:48 PM Alistair Francis
> > > > <alistair23@gmail.com> wrote:
> > > >
> > > > - Socket timestamps with SO_TIMESTAMP
> > > > - Socket timeouts with SO_RCVTIMEO/SO_SNDTIMEO
> > > > - pc-style parallel ports with the PPGETTIME/PPSETTIME ioctls
> > > > - video4linux with the VIDIOC_QUERYBUF/VIDIOC_QBUF/VIDIOC_DQBUF
> > > >   VIDIOC_PREPARE_BUF and VIDIOC_OMAP3ISP_STAT_REQ ioctls
> > > >
> > > >        Arnd    
> > >
> > > I've kept the __suseconds_t type from the original definition of
> > > struct timeval.
> > >
> > > Moreover, as I've noted in the commit message - the struct
> > > __timeval64 is NOT supposed to be passed directly to Linux kernel
> > > and shall be explicitly converted if needed (thanks Arnd for
> > > pointing out exact situations where conversion will be needed).
> > >
> > > Or maybe somebody has better idea? Comments are more than
> > > welcome.    
> > 
> > musl went to a 64-bit suseconds_t, same as the riscv32 port on
> > glibc.
> > 
> > It would certainly help if this could be consistent across
> > architectures and libraries.  
> 
> Ok. Thanks for the input.

Would it be OK to have:

struct __timeval64
{
  __time64_t tv_sec;         /* Seconds */
  __int64_t tv_usec;       /* Microseconds */
};

I would prefer to avoid changing typedef of suseconds_t as it may
affect struct timeval related operations.

Using explicitly __int64_t for tv_usec seems better option, isn't it?

> 
> > 
> >       Arnd  
> 
> 
> 
> 
> 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 3/6] y2038: Introduce struct __timeval64 - new internal glibc type

[Arnd Bergmann]

On Mon, Jan 20, 2020 at 1:34 PM Lukasz Majewski <lukma@denx.de> wrote:
> > > On Sun, Jan 19, 2020 at 4:22 PM Lukasz Majewski <lukma@denx.de>
> > > wrote:
> > > > > On Sat, Jan 18, 2020 at 11:48 PM Alistair Francis <alistair23@gmail.com> wrote:
> > >
> > > It would certainly help if this could be consistent across
> > > architectures and libraries.
> >
> > Ok. Thanks for the input.
>
> Would it be OK to have:
>
> struct __timeval64
> {
>   __time64_t tv_sec;         /* Seconds */
>   __int64_t tv_usec;       /* Microseconds */
> };

This would not work if you pass it into a kernel interface that expects
a 32-bit suseconds_t on sparc64, but it should work as an internal
type on all 32-bit architectures.

> I would prefer to avoid changing typedef of suseconds_t as it may
> affect struct timeval related operations.
>
> Using explicitly __int64_t for tv_usec seems better option, isn't it?

Maybe a __suseconds64_t that happens to be 32-bit wide on sparc64?

     Arnd

Re: [PATCH 3/6] y2038: Introduce struct __timeval64 - new internal glibc type

[Adhemerval Zanella]

On 20/01/2020 10:00, Arnd Bergmann wrote:
> On Mon, Jan 20, 2020 at 1:34 PM Lukasz Majewski <lukma@denx.de> wrote:
>>>> On Sun, Jan 19, 2020 at 4:22 PM Lukasz Majewski <lukma@denx.de>
>>>> wrote:
>>>>>> On Sat, Jan 18, 2020 at 11:48 PM Alistair Francis <alistair23@gmail.com> wrote:
>>>>
>>>> It would certainly help if this could be consistent across
>>>> architectures and libraries.
>>>
>>> Ok. Thanks for the input.
>>
>> Would it be OK to have:
>>
>> struct __timeval64
>> {
>>   __time64_t tv_sec;         /* Seconds */
>>   __int64_t tv_usec;       /* Microseconds */
>> };
> 
> This would not work if you pass it into a kernel interface that expects
> a 32-bit suseconds_t on sparc64, but it should work as an internal
> type on all 32-bit architectures.
> 
>> I would prefer to avoid changing typedef of suseconds_t as it may
>> affect struct timeval related operations.
>>
>> Using explicitly __int64_t for tv_usec seems better option, isn't it?
> 
> Maybe a __suseconds64_t that happens to be 32-bit wide on sparc64?

If timeval64 kernel ABI expects tv_usec being a different type depending
of the architecture, it is an indication we should parametrize with a 
__suseconds64_t.

As a side note, why tv_usec for timeval64 is a 64-bit value? Is it
related to some alignment constraint?

Re: [PATCH 3/6] y2038: Introduce struct __timeval64 - new internal glibc type

[Arnd Bergmann]

On Mon, Jan 20, 2020 at 5:46 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
> On 20/01/2020 10:00, Arnd Bergmann wrote:
> > On Mon, Jan 20, 2020 at 1:34 PM Lukasz Majewski <lukma@denx.de> wrote:
> >>>> On Sun, Jan 19, 2020 at 4:22 PM Lukasz Majewski <lukma@denx.de>
> >>>> wrote:
> >>>>>> On Sat, Jan 18, 2020 at 11:48 PM Alistair Francis <alistair23@gmail.com> wrote:
> >>>>
> >>>> It would certainly help if this could be consistent across
> >>>> architectures and libraries.
> >>>
> >>> Ok. Thanks for the input.
> >>
> >> Would it be OK to have:
> >>
> >> struct __timeval64
> >> {
> >>   __time64_t tv_sec;         /* Seconds */
> >>   __int64_t tv_usec;       /* Microseconds */
> >> };
> >
> > This would not work if you pass it into a kernel interface that expects
> > a 32-bit suseconds_t on sparc64, but it should work as an internal
> > type on all 32-bit architectures.
> >
> >> I would prefer to avoid changing typedef of suseconds_t as it may
> >> affect struct timeval related operations.
> >>
> >> Using explicitly __int64_t for tv_usec seems better option, isn't it?
> >
> > Maybe a __suseconds64_t that happens to be 32-bit wide on sparc64?
>
> If timeval64 kernel ABI expects tv_usec being a different type depending
> of the architecture, it is an indication we should parametrize with a
> __suseconds64_t.
>
> As a side note, why tv_usec for timeval64 is a 64-bit value? Is it
> related to some alignment constraint?

I think it's mostly for simplicity: we do need the microseconds to be
in the low bytes for compatibility with the 64-bit syscalls in the
kernel, so it's either a 64-bit microseconds value or the same
conditional padding that exists in timespec, with an extra special
case for sparc64.

The 64-bit case is slightly simpler for the few kernel interfaces that
take a timeval as input and then can avoid conditionally zeroing out
the upper 32 bits before checking the lower 32 bits again 1000000.
I think that is only needed for clock_adjtimex, VIDIOC_QBUF and
PPSETTIME though, everything else only passes a timeval
to user space.

       Arnd

Re: [PATCH 3/6] y2038: Introduce struct __timeval64 - new internal glibc type

[Lukasz Majewski]

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

Hi Arnd, Adhemerval,

> On Mon, Jan 20, 2020 at 5:46 PM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
> > On 20/01/2020 10:00, Arnd Bergmann wrote:  
> > > On Mon, Jan 20, 2020 at 1:34 PM Lukasz Majewski <lukma@denx.de>
> > > wrote:  
> > >>>> On Sun, Jan 19, 2020 at 4:22 PM Lukasz Majewski <lukma@denx.de>
> > >>>> wrote:  
> > >>>>>> On Sat, Jan 18, 2020 at 11:48 PM Alistair Francis
> > >>>>>> <alistair23@gmail.com> wrote:  
> > >>>>
> > >>>> It would certainly help if this could be consistent across
> > >>>> architectures and libraries.  
> > >>>
> > >>> Ok. Thanks for the input.  
> > >>
> > >> Would it be OK to have:
> > >>
> > >> struct __timeval64
> > >> {
> > >>   __time64_t tv_sec;         /* Seconds */
> > >>   __int64_t tv_usec;       /* Microseconds */
> > >> };  
> > >
> > > This would not work if you pass it into a kernel interface that
> > > expects a 32-bit suseconds_t on sparc64, but it should work as an
> > > internal type on all 32-bit architectures.
> > >  
> > >> I would prefer to avoid changing typedef of suseconds_t as it may
> > >> affect struct timeval related operations.
> > >>
> > >> Using explicitly __int64_t for tv_usec seems better option,
> > >> isn't it?  
> > >
> > > Maybe a __suseconds64_t that happens to be 32-bit wide on
> > > sparc64?  
> >
> > If timeval64 kernel ABI expects tv_usec being a different type
> > depending of the architecture, it is an indication we should
> > parametrize with a __suseconds64_t.
> >
> > As a side note, why tv_usec for timeval64 is a 64-bit value? Is it
> > related to some alignment constraint?  
> 
> I think it's mostly for simplicity: we do need the microseconds to be
> in the low bytes for compatibility with the 64-bit syscalls in the
> kernel, so it's either a 64-bit microseconds value or the same
> conditional padding that exists in timespec, with an extra special
> case for sparc64.
> 
> The 64-bit case is slightly simpler for the few kernel interfaces that
> take a timeval as input and then can avoid conditionally zeroing out
> the upper 32 bits before checking the lower 32 bits again 1000000.
> I think that is only needed for clock_adjtimex, VIDIOC_QBUF and
> PPSETTIME though, everything else only passes a timeval
> to user space.

I would opt for simpler approach :-)

Is the below code correct?
./include/time.h

struct __timeval64
{
   __time64_t tv_sec;         /* Seconds */
   __suseconds64_t tv_usec;       /* Microseconds */
};

However, I'm a bit confused on the correct place to define
__suseconds64_t.

Shall it be added to posix/bits/types.h?
__STD_TYPE __SUSECONDS_T_TYPE __suseconds_t; /* Signed count of
microseconds.  */

If yes, then wouldn't we break the compatibility by adding new type to
this file?

> 
>        Arnd




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 3/6] y2038: Introduce struct __timeval64 - new internal glibc type

[Adhemerval Zanella]

On 21/01/2020 13:45, Lukasz Majewski wrote:
> Hi Arnd, Adhemerval,
> 
>> On Mon, Jan 20, 2020 at 5:46 PM Adhemerval Zanella
>> <adhemerval.zanella@linaro.org> wrote:
>>> On 20/01/2020 10:00, Arnd Bergmann wrote:  
>>>> On Mon, Jan 20, 2020 at 1:34 PM Lukasz Majewski <lukma@denx.de>
>>>> wrote:  
>>>>>>> On Sun, Jan 19, 2020 at 4:22 PM Lukasz Majewski <lukma@denx.de>
>>>>>>> wrote:  
>>>>>>>>> On Sat, Jan 18, 2020 at 11:48 PM Alistair Francis
>>>>>>>>> <alistair23@gmail.com> wrote:  
>>>>>>>
>>>>>>> It would certainly help if this could be consistent across
>>>>>>> architectures and libraries.  
>>>>>>
>>>>>> Ok. Thanks for the input.  
>>>>>
>>>>> Would it be OK to have:
>>>>>
>>>>> struct __timeval64
>>>>> {
>>>>>   __time64_t tv_sec;         /* Seconds */
>>>>>   __int64_t tv_usec;       /* Microseconds */
>>>>> };  
>>>>
>>>> This would not work if you pass it into a kernel interface that
>>>> expects a 32-bit suseconds_t on sparc64, but it should work as an
>>>> internal type on all 32-bit architectures.
>>>>  
>>>>> I would prefer to avoid changing typedef of suseconds_t as it may
>>>>> affect struct timeval related operations.
>>>>>
>>>>> Using explicitly __int64_t for tv_usec seems better option,
>>>>> isn't it?  
>>>>
>>>> Maybe a __suseconds64_t that happens to be 32-bit wide on
>>>> sparc64?  
>>>
>>> If timeval64 kernel ABI expects tv_usec being a different type
>>> depending of the architecture, it is an indication we should
>>> parametrize with a __suseconds64_t.
>>>
>>> As a side note, why tv_usec for timeval64 is a 64-bit value? Is it
>>> related to some alignment constraint?  
>>
>> I think it's mostly for simplicity: we do need the microseconds to be
>> in the low bytes for compatibility with the 64-bit syscalls in the
>> kernel, so it's either a 64-bit microseconds value or the same
>> conditional padding that exists in timespec, with an extra special
>> case for sparc64.
>>
>> The 64-bit case is slightly simpler for the few kernel interfaces that
>> take a timeval as input and then can avoid conditionally zeroing out
>> the upper 32 bits before checking the lower 32 bits again 1000000.
>> I think that is only needed for clock_adjtimex, VIDIOC_QBUF and
>> PPSETTIME though, everything else only passes a timeval
>> to user space.
> 
> I would opt for simpler approach :-)
> 
> Is the below code correct?
> ./include/time.h
> 
> struct __timeval64
> {
>    __time64_t tv_sec;         /* Seconds */
>    __suseconds64_t tv_usec;       /* Microseconds */
> };
> 
> However, I'm a bit confused on the correct place to define
> __suseconds64_t.
> 
> Shall it be added to posix/bits/types.h?
> __STD_TYPE __SUSECONDS_T_TYPE __suseconds_t; /* Signed count of
> microseconds.  */


Yes, the idea is to add on posix/bits/types.h:

__STD_TYPE __SUSECONDS64_T_TYPE __suseconds64_t;

Then on generic bits bits/typesizes.h:

#define __SUSECONDS_T_TYPE      __SQUAD_TYPE;

And then also define it on each Linux bits/typesizes.h.

> 
> If yes, then wouldn't we break the compatibility by adding new type to
> this file?

What do you mean by break compatibility? It is a new type on an
installed header used by new types.

Re: [PATCH 3/6] y2038: Introduce struct __timeval64 - new internal glibc type

[Lukasz Majewski]

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

Hi Adhemerval,

> On 21/01/2020 13:45, Lukasz Majewski wrote:
> > Hi Arnd, Adhemerval,
> >   
> >> On Mon, Jan 20, 2020 at 5:46 PM Adhemerval Zanella
> >> <adhemerval.zanella@linaro.org> wrote:  
> >>> On 20/01/2020 10:00, Arnd Bergmann wrote:    
> >>>> On Mon, Jan 20, 2020 at 1:34 PM Lukasz Majewski <lukma@denx.de>
> >>>> wrote:    
> >>>>>>> On Sun, Jan 19, 2020 at 4:22 PM Lukasz Majewski
> >>>>>>> <lukma@denx.de> wrote:    
> >>>>>>>>> On Sat, Jan 18, 2020 at 11:48 PM Alistair Francis
> >>>>>>>>> <alistair23@gmail.com> wrote:    
> >>>>>>>
> >>>>>>> It would certainly help if this could be consistent across
> >>>>>>> architectures and libraries.    
> >>>>>>
> >>>>>> Ok. Thanks for the input.    
> >>>>>
> >>>>> Would it be OK to have:
> >>>>>
> >>>>> struct __timeval64
> >>>>> {
> >>>>>   __time64_t tv_sec;         /* Seconds */
> >>>>>   __int64_t tv_usec;       /* Microseconds */
> >>>>> };    
> >>>>
> >>>> This would not work if you pass it into a kernel interface that
> >>>> expects a 32-bit suseconds_t on sparc64, but it should work as an
> >>>> internal type on all 32-bit architectures.
> >>>>    
> >>>>> I would prefer to avoid changing typedef of suseconds_t as it
> >>>>> may affect struct timeval related operations.
> >>>>>
> >>>>> Using explicitly __int64_t for tv_usec seems better option,
> >>>>> isn't it?    
> >>>>
> >>>> Maybe a __suseconds64_t that happens to be 32-bit wide on
> >>>> sparc64?    
> >>>
> >>> If timeval64 kernel ABI expects tv_usec being a different type
> >>> depending of the architecture, it is an indication we should
> >>> parametrize with a __suseconds64_t.
> >>>
> >>> As a side note, why tv_usec for timeval64 is a 64-bit value? Is it
> >>> related to some alignment constraint?    
> >>
> >> I think it's mostly for simplicity: we do need the microseconds to
> >> be in the low bytes for compatibility with the 64-bit syscalls in
> >> the kernel, so it's either a 64-bit microseconds value or the same
> >> conditional padding that exists in timespec, with an extra special
> >> case for sparc64.
> >>
> >> The 64-bit case is slightly simpler for the few kernel interfaces
> >> that take a timeval as input and then can avoid conditionally
> >> zeroing out the upper 32 bits before checking the lower 32 bits
> >> again 1000000. I think that is only needed for clock_adjtimex,
> >> VIDIOC_QBUF and PPSETTIME though, everything else only passes a
> >> timeval to user space.  
> > 
> > I would opt for simpler approach :-)
> > 
> > Is the below code correct?
> > ./include/time.h
> > 
> > struct __timeval64
> > {
> >    __time64_t tv_sec;         /* Seconds */
> >    __suseconds64_t tv_usec;       /* Microseconds */
> > };
> > 
> > However, I'm a bit confused on the correct place to define
> > __suseconds64_t.
> > 
> > Shall it be added to posix/bits/types.h?
> > __STD_TYPE __SUSECONDS_T_TYPE __suseconds_t; /* Signed count of
> > microseconds.  */  
> 
> 
> Yes, the idea is to add on posix/bits/types.h:
> 
> __STD_TYPE __SUSECONDS64_T_TYPE __suseconds64_t;
> 
> Then on generic bits bits/typesizes.h:
> 
> #define __SUSECONDS_T_TYPE      __SQUAD_TYPE;
> 
> And then also define it on each Linux bits/typesizes.h.

Ok.

> 
> > 
> > If yes, then wouldn't we break the compatibility by adding new type
> > to this file?  
> 
> What do you mean by break compatibility? It is a new type on an
> installed header used by new types.

Thanks for the explanation - now it is clear :-)


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