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

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

Hi Joseph,

On Mon, 29 Oct 2018 15:15:40 +0000, Joseph Myers
<joseph@codesourcery.com> wrote :

> On Sun, 28 Oct 2018, Albert ARIBAUD (3ADEV) wrote:
> 
> > +#define TEST_FUNCTION do_test ()
> > +#include "../test-skeleton.c"  
> 
> New tests should use <support/test-driver.c>, not the old test-skeleton.

Ok.

> > +  if (result == -1 && errno == 0)
> > +    return result;  
> 
> It is never OK for a library function to set errno to 0 (return with errno 
> set to 0 when it wasn't 0 on entry); if there is no error you can restore 
> the value that was set on entry to mktime, or set errno to some spurious 
> other value despite the lack of an error (maybe not a good idea in this 
> case, because it prevents callers from distinguishing error and non-error 
> -1), but not set it to 0 if it wasn't 0 on entry to the library function.  
> (See ISO C: "The value of errno in the initial thread is zero at program 
> startup (the initial value of errno in other threads is an indeterminate 
> value), but is never set to zero by any library function.".)

Noted [but then, if some application code, for some reason, has errno
already set to EOVERFLOW by the time it calls mktime, and if mktime
returns -1 without setting errno (i.e., returns a valid "one second
before the Epoch" time), the application will falsely believe mktime
returned a failure (-1 and errno==EOVERFLOW). OTOH, -1 is not 100%
Posix as a time elapsed since the Epoch, so that's not really an issue].

> > +    __set_errno(errno_from_tzset);  
> 
> Missing space before '(', here and elsewhere in this patch.

Will be "fixed" in v3 as errno handling will be removed.

Cordialement,
Albert ARIBAUD
3ADEV