Re: [RFC 1/7] y2038: Introduce struct __timespec64

[Lukasz Majewski <lukma at denx dot de>]

On Wed, 10 Apr 2019 15:05:27 +0200
Lukasz Majewski <lukma@denx.de> wrote:

> Hi Paul, Joseph,
> 
> > >> However, as long as __time64_t and __time_t are not visible to
> > >> user code when _TIME_BITS=64, this will address most of my own
> > >> practical concerns and should be OK as a compromise.    
> > > 
> > > To sum up:
> > > ----------
> > > 
> > > 1. __time64_t and __time_t are "exported" (visible
> > > in /usr/include/*) always (similar to __off_t and __off64_t).    
> > 
> > No, the compromise is that these types are "exported" only when
> > _TIME_BITS<64. Exporting them when _TIME_BITS==64 is unnecessary and
> > confusing. (It's also unnecessary and confusing when _TIME_BITS<64,
> > but I'm willing to compromise there because the _TIME_BITS<64 case
> > is obsolescent and will eventually go away.)
> >   
> > > 2. The "exported" struct timespec would look like:
> > > 
> > > struct timespec
> > > {
> > > /* Use the original definition for 64-bit arches
> > >     or when 64-bit-time by default has *not* been requested */
> > > #if __WORDSIZE > 32 || ! defined(__USE_TIME_BITS64)
> > >    __time_t tv_sec; /* Seconds. */
> > > #else
> > >    __time64_t tv_sec;
> > > #end
> > >    __syscall_slong_t tv_nsec;
> > > };    
> > 
> > Sorry, I'm not following this. Why can't this be the natural 'struct
> > timespec { time_t tv_sec; ...; }'? The two forms are equivalent,
> > regardless of whether __WORDSIZE > 32 || !
> > defined(__USE_TIME_BITS64) and regardless of what we do about (1),
> > so why use the more-complicated and more-confusing definition?  
> 
> Ok, time_t shall be used for tv_sec.
> 
> > 
> > As Joseph mentioned, there may need to be ifdeffery around tv_nsec
> > due to the issue of endian-dependent padding around tv_nsec.  
> 
> It is not only the padding. 
> The problem is with the size of this struct passed to the kernel as
> syscall.
> 
> 1. __TIMESIZE = 64 -> No problem with kernel ABI
> 	- sizeof(tv_sec) = 64 bit
> 	- sizeof(tv_nsec) = 64 bit
> 
> 	Kernel looks for tv_sec and tv_nsec, each 64 bit
> 
> 2. __TIMESIZE = 32 (no, Y2038 support) ->  No problem with kernel ABI
> 	- sizeof(tv_sec) = 32 bit
> 	- sizeof(tv_nsec) = 32 bit
> 
> 	Kernel looks for tv_sec and tv_nsec, each 32 bit (those
> 	syscalls would be removed from the kernel)
> 
> 3. __TIMESIZE = 32 (Y2038 support) -> ABI problem 
> 	- sizeof(tv_sec) = 64 bit (time_t)
> 	- sizeof(tv_nsec) = 32 bit
> 	- needed padding (tv_pad) 32 bit
> 
> 	Kernel expects tv_sec and tv_nsec to be both 64 bits.
> 
> Exported (installed at /usr/include) struct timespec (to work in above
> three cases):
> 
> struct timespec
>  {
>    time_t tv_sec;   /* Seconds.  */
> #ifdef __USE_TIME_BITS64
> # if BYTE_ORDER == BIG_ENDIAN
>    int tv_pad: 32;  /* Padding named for checking/setting */
>    __syscall_slong_t tv_nsec; /* Nanoseconds */ 
> # else
>    __syscall_slong_t tv_nsec; /* Nanoseconds */
>    int tv_pad: 32;  /* Padding named for checking/setting */ 
> # endif
> #else
>    __syscall_slong_t tv_nsec;	/* Nanoseconds.  */
> #endif
>  };
> 
> 
> and internal (in ./include/time.h) definition of struct __timespec64:
> 
> #if __TIMESIZE == 64
> # define __timespec64 timespec
> #else
> struct __timespec64
> {
> 	__time64_t tv_sec;         /* Seconds */
> 	__time64_t tv_nsec;         /* Nanoseconds */
> };
> #endif
> 

I've tested it a bit more and the internal representation also needs
padding to be safely passed to Linux kernel.

Hence, it would look like:

struct __timespec64
{
	__time64_t tv_sec;         /* Seconds */
# if BYTE_ORDER == BIG_ENDIAN
	int tv_pad: 32;            /* Padding named for
	__int32_t tv_nsec;         /* Nanoseconds */
# else
	__int32_t tv_nsec;         /* Nanoseconds */
	int tv_pad: 32;            /* Padding named for
# endif
};
#endif


> 
> 
> > However,
> > there does not need to be ifdeffery around tv_sec. Just use time_t
> > there; problem solved.
> >   
> > > 3. Put the internal struct __timespec64 ...
> > > into include/time.h and don't introduce separate
> > > include/bits/types/struct___timespec64.h
> > > 
> > > The same idea would be applied to struct timeval    
> > 
> > Yes.  
> 
> 
> 
> 
> 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: pgpFluqeOO95b.pgp
Description: OpenPGP digital signature