This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v8 1/2] Y2038: Add 64-bit time for all architectures
Hi Florian,
On Wed, 26 Sep 2018 10:39:59 +0200, Florian Weimer <fweimer@redhat.com>
wrote :
> * Albert ARIBAUD:
>
> > diff --git a/posix/bits/types.h b/posix/bits/types.h
> > index 5e22ce41bf..cda0a70dd8 100644
> > --- a/posix/bits/types.h
> > +++ b/posix/bits/types.h
>
> > @@ -211,6 +213,12 @@ __STD_TYPE __U32_TYPE __socklen_t;
> > It is not currently necessary for this to be machine-specific. */
> > typedef int __sig_atomic_t;
> >
> > +#if __TIMESIZE == 64
> > +# define __time64_t __time_t
> > +#else
> > +__STD_TYPE __TIME64_T_TYPE __time64_t; /* Seconds since the
> > Epoch. */ +#endif
>
> Should the __TIMESIZE == 64 case use typedef as well?
It was requested that the Y038 code cause as little change as possible
at the object level in the TIMESIZE == 64 case, hence the #define which
ensures that after the preprocessor stage, the compiler can only see
'time_t', not 'time64_t'.
> > diff --git a/sysdeps/unix/sysv/linux/x86/bits/time64.h b/sysdeps/unix/sysv/linux/x86/bits/time64.h
> > new file mode 100644
> > index 0000000000..81de09e23f
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/x86/bits/time64.h
>
> > +#if defined __x86_64__ && defined __ILP32__
> > +/* For x32, time is 64-bit even though word size is 32-bit. */
> > +# define __TIME64_T_TYPE __SQUAD_TYPE
> > +#elif __TIMESIZE == 64
> > +/* If we already have 64-bit time then use it. */
> > +# define __TIME64_T_TYPE __TIME_T_TYPE
> > +#else
> > +/* Define a 64-bit type alongsize the 32-bit one. */
> > +# define __TIME64_T_TYPE __SQUAD_TYPE
> > +#endif
>
> Perhaps I'm missing something, but I think this should be put into a
> generic header and could be written like this:
>
> #if __TIMESIZE == 64
> # define __TIME64_T_TYPE __TIME_T_TYPE
> #else
> # if __WORDSIZE != 32
> # error "32-bit word size expected for non-64-bit time_t"
> # endif
> # define __TIME64_T_TYPE __SQUAD_TYPE
> #endif
>
> I don't think there is a 64-bit port with a 32-bit time_t.
The patch does provide definitions for __TIMESIZE and __TIME64_T_TYPE
in generic headers (bits/timesize and bits/time64.h respectively)
which cover all cases for all 64-bit architectures and all 32-bit
architectures.
Since the only exception is X32, it makes sense that the definitions
which take this exception into account be put in the most X32 specific
header, which is the x86 one.
This organization stems from previous comments by Joseph Myers:
https://patchwork.ozlabs.org/patch/930876/#1935670
> > diff --git a/sysdeps/unix/sysv/linux/x86/bits/timesize.h b/sysdeps/unix/sysv/linux/x86/bits/timesize.h
> > new file mode 100644
> > index 0000000000..8b88ab84b0
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/x86/bits/timesize.h
>
> > +#if defined __x86_64__ && defined __ILP32__
> > +/* For x32, time is 64-bit even though word size is 32-bit. */
> > +# define __TIMESIZE 64
> > +#else
> > +/* For others, time size is word size. */
> > +# define __TIMESIZE __WORDSIZE
> > +#endif
>
> I think writing this as
>
> #ifdef __x86_64__
> /* This includes x32, where time_t is 64-bit even though the word size
> is 32-bit. */
> # define __TIMESIZE 64
> #else
> # define __TIMESIZE 32
> #endif
>
> is much clearer. It's not that there's going to be a different x86-64
> API with yet another time_t size any time soon.
I prefer the #if/#else condition to explicitly involve x32
(through the symbol __ILP32__) rather than implicitly.
> > index 72ef75f074..844a68de8c 100644
> > --- a/time/tzfile.c
> > +++ b/time/tzfile.c
>
> The tzfile.c changes look okay to me.
Thanks!
> Thanks,
> Florian
Cordialement,
Albert ARIBAUD
3ADEV