[v2,2/5] y2038: Introduce struct __timeval64 - new internal glibc type

  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

---
Changes for v2:
- Replace __suseconds_t with __suseconds64_t
---
 include/time.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
  

On Mon, Jan 27, 2020 at 12:53 AM 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
>
> ---
> Changes for v2:
> - Replace __suseconds_t with __suseconds64_t
> ---
>  include/time.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/include/time.h b/include/time.h
> index 047f431a1a..99492a1577 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 */
> +  __suseconds64_t tv_usec;       /* Microseconds */
> +};
> +#endif

Can this be moved to time/bits/types/struct_timeval.h?

The reason is that we need it in include/bits/types/struct_rusage.h
and including time.h results in exposed symbols (from memory I see
glibc test failures when trying to do this).

The other problem is that we need __timeval64 in time/sys/time.h for
__getitimer64() and __setitimer64() and include/time.h isn't included
in there.

Alistair

> +
>  #if __TIMESIZE == 64
>  # define __ctime64 ctime
>  #else
> --
> 2.20.1
>
  

On Tue, 28 Jan 2020, Alistair Francis wrote:

> Can this be moved to time/bits/types/struct_timeval.h?

That's an installed header.  Internal types don't belong in installed 
headers unless unavoidable.  Note that the __USE_TIME_BITS64 conditionals 
we eventually get in installed headers will (for example) change the 
contents of "struct timeval" depending on __USE_TIME_BITS64, rather than 
adding a new type with an internal name.

> The reason is that we need it in include/bits/types/struct_rusage.h
> and including time.h results in exposed symbols (from memory I see
> glibc test failures when trying to do this).

include/bits/types/struct_rusage.h is a *wrapper* round an installed 
header.  Although such wrappers can contain internal declarations under a 
!_ISOMAC condition, it's best not to in this case because of the rule that 
including <bits/types/foo.h> should always get a definition of the type 
foo and nothing else beyond foo's dependencies, and breaking that rule 
would be confusing.

> The other problem is that we need __timeval64 in time/sys/time.h for
> __getitimer64() and __setitimer64() and include/time.h isn't included
> in there.

But time/sys/time.h is an installed header.  So it shouldn't get internal 
types either.  __getitimer64 and __setitimer64 are not public interfaces 
(nor, as APIs, should they ever be, though eventually __USE_TIME_BITS64 
may cause redirection to such names as ABIs).

The installed header changes for _TIME_BITS=64 support aren't until all 
the internal pieces are ready.  Until then, to a first approximation, you 
should only be changing *internal, non-installed* headers and 
implementation files, not installed headers.
  

On Mon, Jan 27, 2020 at 4:47 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Tue, 28 Jan 2020, Alistair Francis wrote:
>
> > Can this be moved to time/bits/types/struct_timeval.h?
>
> That's an installed header.  Internal types don't belong in installed
> headers unless unavoidable.  Note that the __USE_TIME_BITS64 conditionals
> we eventually get in installed headers will (for example) change the
> contents of "struct timeval" depending on __USE_TIME_BITS64, rather than
> adding a new type with an internal name.
>
> > The reason is that we need it in include/bits/types/struct_rusage.h
> > and including time.h results in exposed symbols (from memory I see
> > glibc test failures when trying to do this).
>
> include/bits/types/struct_rusage.h is a *wrapper* round an installed
> header.  Although such wrappers can contain internal declarations under a
> !_ISOMAC condition, it's best not to in this case because of the rule that
> including <bits/types/foo.h> should always get a definition of the type
> foo and nothing else beyond foo's dependencies, and breaking that rule
> would be confusing.

How should I add a __rusage64 type then? Right now I have modified
include/bits/types/struct_rusage.h to include the header twice with
different definitions based on macros.

>
> > The other problem is that we need __timeval64 in time/sys/time.h for
> > __getitimer64() and __setitimer64() and include/time.h isn't included
> > in there.
>
> But time/sys/time.h is an installed header.  So it shouldn't get internal
> types either.  __getitimer64 and __setitimer64 are not public interfaces
> (nor, as APIs, should they ever be, though eventually __USE_TIME_BITS64
> may cause redirection to such names as ABIs).

From memory I was seeing compiler failures, as something doesn't
include time.h. I can look into this more though.

Alistair

>
> The installed header changes for _TIME_BITS=64 support aren't until all
> the internal pieces are ready.  Until then, to a first approximation, you
> should only be changing *internal, non-installed* headers and
> implementation files, not installed headers.
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
  

On Tue, 28 Jan 2020, Alistair Francis wrote:

> How should I add a __rusage64 type then? Right now I have modified

Define it in an *internal, non-installed* header, just like __timespec64 
is defined in include/time.h (the internal wrapper for the installed 
header).  For example, inside the "#ifndef _ISOMAC" section of 
include/sys/resource.h (if using a wrapper header like that, such internal 
definitions must be inside #ifndef _ISOMAC and the include of the public 
header outside that conditional).
  

On Mon, Jan 27, 2020 at 5:42 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Tue, 28 Jan 2020, Alistair Francis wrote:
>
> > How should I add a __rusage64 type then? Right now I have modified
>
> Define it in an *internal, non-installed* header, just like __timespec64
> is defined in include/time.h (the internal wrapper for the installed
> header).  For example, inside the "#ifndef _ISOMAC" section of
> include/sys/resource.h (if using a wrapper header like that, such internal
> definitions must be inside #ifndef _ISOMAC and the include of the public
> header outside that conditional).

Thanks for the help Joseph. I think I have changed it all around so
it's in non-installed headers.

Alistair

>
> --
> Joseph S. Myers
> joseph@codesourcery.com