On Mon, Jun 6, 2022 at 11:27 AM Paul Eggert via tz <tz@iana.org> wrote:
On 6/6/22 10:35, Almaz Mingaleev wrote:
1) According to the man page mktime sets errno if tm is invalid. Why is it not safe to just check errno value (saving and restoring it if needed)?
Because mktime can set errno if tm is valid. mktime is like most C syscalls - when they fail, they set errno to a value that tells you what the error was, but when they succeed they set errno to an unspecified value.
you can still int saved_errno = errno; errno = 0; if (mktime() == -1 && errno != 0) { return NULL; } errno = saved_errno; though? shouldn't that be cheaper than an extra localtime() call? (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 https://android.googlesource.com/platform/bionic/+/master/libc/bionic/time64... 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.)
2) man page says that in error case mktime "returns (time_t) -1 and does not alter the members of the broken-down time structure". Wouldn't it be enough to check that tm.tm_yday is >= 0?
That's the man page for TZDB mktime; unfortunately the C standard does not guarantee this and strftime.c is attempting to be portable to non-TZDB mktime.
ah, yeah, looks like POSIX historically said "may" set errno (though 2018 seems to provide the guarantee). that's unfortunate...
Now that I look at the code again, I see that the heuristic was still too trusting of non-TZDB mktime implementations, so I installed the attached to tighten things up. It's just a heuristic, so it can still do the wrong thing on even weirder implementations (though not TZDB).
Perhaps some day we can optimize strftime.c if we know that TZDB-compatible mktime is being used.