Re: [PATCH] Ensure mktime sets errno on error (bug 23789)

[Zack Weinberg <zackw at panix dot com>]

On Thu, Oct 25, 2018 at 12:34 AM Albert ARIBAUD <albert.aribaud@3adev.fr> wrote:
> > > +  result = __mktime_internal (tp, __localtime_r, &localtime_offset);
...
> > > +  if (result == -1)
> > > +    {
> > > +      __set_errno(EOVERFLOW);
> > > +    }
> > > +  return result;
> >
> > Why do you make this change in the `mktime` wrapper instead of
> > `__mktime_internal`?
>
> The change is about errno, which is part of the public interface, so I
> considered it was more logical to put it in the wrapper which
> implements the public interface than in an internal function.

The proper question to ask is whether the other callers of
__mktime_internal should also set errno according to the same rules.
In this case, the only other caller (within glibc; I haven't checked
gnulib) is timegm. timegm is not part of any standard, but I agree
with Andreas that it should have the same public semantics as mktime,
for consistency's sake.

Also, as Paul pointed out, the wrapper can't tell whether
__mktime_internal returned -1 due to a true overflow, or because the
result of the conversion is correctly (time_t)-1.  __mktime_internal
itself, on the other hand, knows which case it's in and can set errno
only when appropriate.

zw