This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2 2/2] Y2038: make __tz_convert compatible with 64-bit-time
Hi Paul,
On Thu, 14 Jun 2018 10:07:18 -0700, Paul Eggert <eggert@cs.ucla.edu>
wrote :
> On 06/14/2018 06:51 AM, Albert ARIBAUD (3ADEV) wrote:
> > +#if __TIMESIZE == 64
> > +# define __ctime64 ctime
> > +# define __ctime64_r ctime_r
> > +#endif
> > +
> > +#if __TIMESIZE == 64
> > +# define __localtime64 localtime
> > +#else
> > +extern struct tm *__localtime64 (const __time64_t *__timer);
> > +#endif
>
> These don't seem symmetric. Why don't we need an extern declaration for
> __ctime_64 or for __ctime_64_r? If the code is correct, maybe a comment why?
The asymmetry (that ctime and ctime_r are only declared in time/time.h
whereas localtime, localtime_r, gmtime and gmtime_r are declared both in
include/time.h as well as time/time.h) existed before my patches, and I
do not know the reason.
I felt I should not add declarations for ctime / ctime_r in
include/time as obviously glibc builds fine without these, and my
patches are not about adding 32-bit declarations but about adding
64-bit support.
However, I can add them if requested.
> > +#if (__TIMESIZE != 64)
> No parentheses here (and similarly elsewhere).
Fixed everywhere.
> +/* The C Standard says ctime (t) is equivalent to asctime (localtime (t)).
> + In particular, ctime and asctime must yield the same pointer. */
> + return asctime (__localtime64 (t));
>
> Please indent the comment to be like what it was before.
Done.
> Otherwise looks good; thanks.
Thanks to you for the reviews!
Cordialement,
Albert ARIBAUD
3ADEV