<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jun 6, 2022 at 11:27 AM Paul Eggert via tz <<a href="mailto:tz@iana.org">tz@iana.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 6/6/22 10:35, Almaz Mingaleev wrote:<br>
<br>
> 1) According to the man page mktime sets errno if tm is invalid. Why is it<br>
> not safe to just check errno value (saving and restoring it if needed)?<br>
<br>
Because mktime can set errno if tm is valid. mktime is like most C <br>
syscalls - when they fail, they set errno to a value that tells you what <br>
the error was, but when they succeed they set errno to an unspecified value.<br></blockquote><div><br></div><div>you can still</div><div><br></div><div>int saved_errno = errno;</div><div>errno = 0;</div><div>if (mktime() == -1 && errno != 0) {</div><div>  return NULL;</div><div>}</div><div>errno = saved_errno;</div><div><br></div><div>though? shouldn't that be cheaper than an extra localtime() call?</div><div><br></div><div>(the specific issue on 32-bit Android [where time_t is 32-bit] is that this ends up using mktime64() which takes a *const* struct tm for historical reasons. that _will_ set errno -- because it just calls tzcode's mktime() behind the scenes -- but doesn't copy the 32-bit struct tm back into the input struct tm. see <a href="https://android.googlesource.com/platform/bionic/+/master/libc/bionic/time64.c#486">https://android.googlesource.com/platform/bionic/+/master/libc/bionic/time64.c#486</a> for the gory details. _changing_ 32-bit-only behavior that's visible to apps in 2023 [we're too late for 2022 already] isn't super.)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> 2) man page says that in error case mktime "returns (time_t) -1 and does<br>
> not alter the members of the broken-down time structure". Wouldn't it be<br>
> enough to check that tm.tm_yday is >= 0?<br>
<br>
That's the man page for TZDB mktime; unfortunately the C standard does <br>
not guarantee this and strftime.c is attempting to be portable to <br>
non-TZDB mktime.<br></blockquote><div><br></div><div>ah, yeah, looks like POSIX historically said "may" set errno (though 2018 seems to provide the guarantee). that's unfortunate...</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Now that I look at the code again, I see that the heuristic was still <br>
too trusting of non-TZDB mktime implementations, so I installed the <br>
attached to tighten things up. It's just a heuristic, so it can still do <br>
the wrong thing on even weirder implementations (though not TZDB).<br>
<br>
Perhaps some day we can optimize strftime.c if we know that <br>
TZDB-compatible mktime is being used.</blockquote></div></div>