Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Lukasz Majewski <lukma at denx dot de>]

Hi Paul,

> On 3/23/19 4:59 AM, Lukasz Majewski wrote:
> > https://github.com/lmajewski/y2038_glibc/commits/mktime_v3_fixes
> >
> > In short:
> >
> > - The __time64_t needs to be exported to user space as it is a
> > building block for Y2038 safe 32 bit systems' structures (like
> > struct __timeval64). In the above use case the "normal" timeval is
> >   implicitly replaced with __timeval64.  
> 
> I looked into that URL, and I don't see any explanation of why
> __time64_t needs to be visible to user code.

On top of this patchset there is an ongoing work for Y2038 support:
https://github.com/lmajewski/y2038_glibc/commits/Y2038-2.29-glibc-__clock_settime64_v1-27-03-2019

The idea till now was to introduce new "installed" glibc type:

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

Which uses explicit __time64_t to store tv_sec.

The above structure on 32bit Y2038 safe devices would replace in user
code struct timeval.


> 
> Surely struct timeval ought to be consistent with time_t. That is, it
> ought to continue to be defined like this:
> 
> struct timeval
> {
>   __time_t tv_sec;        /* Seconds.  */
>   __suseconds_t tv_usec;    /* Microseconds.  */
> };
> 
> where __time_t is merely an alias for time_t and so is 64 bits if
> _TIME_BITS=64 and is the current size (32 or 64) otherwise.

In other words - I shall not introduce new "installed" type for struct
timeval and just in posix/bits/types.h define:

#ifndef __USE_TIME_BITS64
  __STD_TYPE __TIME_T_TYPE __time_t; /* Seconds since the Epoch. */
#else
  __STD_TYPE·__TIME64_T_TYPE __time_t;
#endif

In that way all structures which use __time_t are Y2038 safe.

ONE NOTE:
I also guess that the above change keeps those structs posix compliant
for 32 bit machines ?

> 
> The only reason I can see why __time64_t needs to be visible to user
> code, would be if a single user source file accesses both time_t and
> __time64_t (or needs to access both struct timeval and struct
> timeval64, or similarly for struct timespec etc.).

The only supported use case would be that user defines -D_TIME_BITS=64
during compilation of the SW (in OE/yocto) and use Y2038 safe types.

> However, we should
> not support a complicated API like that, as it's typically not useful
> in practice and its mere availability causes more confusion than it's
> worth - as I've discovered with _FILE_OFFSET_BITS and __off64_t.


If I may ask - what were the biggest problems?


I would appreciate if we could make the decision/agreement on this -

if the 64bit time support shall be done by above redefinition of
__time_t and not "exporting" __time64_t (and struct __timeval64,
__timespec64, etc).

> Instead, glibc should have a simple user API where _TIME_BITS=64
> merely says "I want 64-bit time_t everywhere" and a module either
> uses this option or it doesn't.
> 

So according to above I shall only introduce glibc _internal_ struct
__timespec64/__timeval64 in include/bits/types/ :

#if __WORDSIZE > 32 || ! defined(__USE_TIME_BITS64)
# define __timespec64 timespec;
#else
struct __timespec64 {
	__time64_t tv_sec;
	__int64_t tv_nsec;
}
#endif

and rewrite the relevant functions/syscalls (like clock_settime() in
this particular case) to use it in glibc ?

PROBLEM(S) with internal struct __timespec64:

- Would be misleading for 32 bit architectures (minor issue)

- Needs to met specific Linux kernel's ABI as it is passed as an
  argument to Linux syscalls (like clock_settime64).


> I did see a fix in that URL, to use "#elif defined TIME_T_IS_SIGNED"
> instead of "#elif TIME_T_IS_SIGNED" for the benefit of picky glibc
> compiles, and that fix is incorporated in the revised patch attached.
> 

Thanks for the revised patch.


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: pgpBeFW7s_iWk.pgp
Description: OpenPGP digital signature