Re: [PATCH v2 2/2] Y2038: make __tz_convert compatible with 64-bit-time

[Albert ARIBAUD <albert.aribaud@3adev.fr>]

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