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

[Lukasz Majewski <lukma at denx dot de>]

Hi Joseph, Paul,

> On Mon, 11 Mar 2019, Paul Eggert wrote:
> 
> > On 2/27/19 3:20 AM, Lukasz Majewski wrote:  
> > > +/* Another name for `__mktime64'.  */
> > > +extern __time64_t __timelocal64 (struct tm *__tp) __THROW;  
> > 
> > In hindsight the name 'timelocal' was a mistake: it's not a portable
> > name and its use has not caught on. Although we need to keep
> > 'timelocal' for backwards compatibility, there's no need to define
> > 'timelocal64', as the very few people who need such a function can
> > just call mktime64. So I suggest removing all traces of
> > timelocal64, __timelocal64, etc. from the patch. 
> 
> Neither timelocal64 nor mktime64 will be a public API; the public API
> is to define _TIME_BITS=64 (and _FILE_OFFSET_BITS=64 because we don't
> want to support the combination of 64-bit times with 32-bit offsets)
> before including any system headers, then call the existing function
> names.

I do have a doubt if I understood the above comment(s). 

Please correct me if I'm wrong - this patch makes the
__mktime_internal compatible with __time_64 explicit 64 bit time data
structure.

It's goal is to replace all calls to mktime/timegm/localtime with
__mktime64/__timegm64/__localtime64 (alias to __mktime64) internal
counterparts (another level of abstraction with explicit 64 bit time
storage).

Then, the __mktime64() internal function is used to implement mktime
wrapper for 32-bit time (the same approach is for timegm).

This patch looks like a preparatory patch for further Y2038 conversion
(but doesn't provide one).

> 
> Given timelocal as part of the __USE_MISC API in time.h, it should be
> part of the __USE_MISC __TIME_BITS=64 API there as well - but it's

This patch doesn't introduce/use the _TIME_BITS=64 - instead it
operates on TIMESIZE=64.

> fine for that case to redirect to __mktime64 (and thus not need
> __timelocal64 as another alias).
> 


Do you require following code in include/time.h for timelocal:

#if defined __USE_MISC
# if defined(__REDIRECT)
  extern int __REDIRECT (timelocal, (struct tm *__tp),
                         __mktime64) __THROW);
# else
#  define timelocal __mktime64
# endif
#endif


The problem seems to be that Paul requested to remove {__}timelocal64
from this patch and you pointed out that timelocal is already supported
when __USE_MISC is defined?




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