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

[Paul Eggert <eggert at cs dot ucla dot edu>]

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.


> +/* Check whether a time64_t value fits in a time_t.  */
> +static inline bool
> +fits_in_time_t (__time64_t t64)
> +{
> +  time_t t = t64;
> +  return t == t64;
> +}
> +

This function is used only in time/mktime.c, and so should be defined
there. This will help sharing with Gnulib, which doesn't have
include/time.h.

The function's name should not end with "_t" since that's in the
reserved namespace (important for Gnulib).


> -verify (TYPE_IS_INTEGER (time_t));
> +verify (TYPE_IS_INTEGER (__time64_t));

Please remove this line instead. It dates back to old POSIX, which
allowed time_t to be a floating-point type. POSIX no longer allows this
and we needn't worry about that possibility any more.


> +  __time64_t t64;
> +  time_t t;
> +  struct tm tp0 = *tp;
There is no need for both t and t64. Just declare "__time64_t t;" and
replace all uses of t64 with t. Also, please consistently rename tp0 to
tm0, since it's not a pointer.


> +  t = t64;
> +  if (t != t64)

Replace this with "if (! fits_in_time_t (t))" (but rename the function).


> +/* The 32-bit-time wrapper.  */
> +time_t
> +mktime (struct tm *tp)
> +{
> +  __time64_t t64 = __mktime64 (tp);
> +  if (fits_in_time_t (t64))
> +    return t64;
> +  __set_errno (EOVERFLOW);
> +  return -1;
> +}

Fix this so that it doesn not modify *TP when failing due to EOVERFLOW.
The manual says mktime doesn't change *TP on failure.


Similar comments apply to the implementation of timegm.