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

[Albert ARIBAUD <albert dot aribaud at 3adev dot fr>]

Hi Paul,

On Fri, 9 Nov 2018 18:00:25 -0800, Paul Eggert <eggert@cs.ucla.edu>
wrote :

> On 11/7/18 6:48 AM, Albert ARIBAUD wrote:
> 
> > 1. If the for-loop reaches remaining_probes==0, then it really should
> >     set errno = EOVERFLOW before returning -1, because remaining_probes
> >     is only decremented in the else clause inside the for-loop, and that
> >     only happens (or should only happen) when there were no failures so
> >     far, so if we fail then, we have to set errno.  
> 
> Thanks for the diagnosis. Revised patches attached, which set errno in 
> that case as you suggested.
> 
> 
> > 2. It is not normal that t, gt, t1 and t2 remain the same for all six
> >     iterations of the for-loop. That should be investigated and fixed.  
> 
> Long ago I came up with weird scenarios involving odd combinations of 
> leap seconds and DST transitions all near the maximum convertible time_t 
> values that could involve that many iterations. None of these scenarios 
> will ever happen, really; the number is that large just to be safe. It 
> could be that I overestimated the number, but that's no big deal.
> 
> 
> > I don't know why ranged_convert alters an argument which should be
> > a pure imput. In fact, I don't know why it does not just copy this
> > argument into a local time_t. Any known reason?  
> Because it communicates back to the caller the nearest long_int value 
> that is in range. This value is not obvious because it can depend on 
> timezone and leap second information.
> 
> After looking at the mktime implementation again I see some other things 
> that need fixing. These are mostly for Gnulib (when we can't assume that 
> localtime_r fails only due to EOVERFLOW), but there are some code 
> cleanups and fixes for very unlikely bugs. Proposed glibc patches attached.

I've applied the series above current master (with the ChangeLog date
adapted) and the make check stats are unchanged by the series, which
means the added test indeed returns ok with these patches. If I can get
the adequate permissions for Bugzilla.

Cordialement,
Albert ARIBAUD
3ADEV