[swbz 29035] mktime vs non-DST

[DJ Delorie <dj@redhat.com>]

https://sourceware.org/bugzilla/show_bug.cgi?id=29035

TL;DR - requesting a partial reversion of 86aece3 to become
bug-compatible with older releases.

Long version:

In investigating this, I did a deep-dive on how tm_isdst works in
mktime().  It seems to be less of a hint and more of an override, in
that, if you set tm_isdst=1 you're going to get a result that seems an
hour off if you're in the middle of a standard time period.  Same for
tm_isdst=0.  Setting tm_isdst=-1 is the only way to let mktime use the
dst-in-effect for the time period specified.  Note: I'm not
considering the time duplication that happens at period boundaries
(i.e. the "fall back" that causes an hour of clock time to repeat each
fall).

So if you set tm_isdst=1 in a call to mktime(), it figures out the
local DST offset and applies it regardless of timezone rules.

In the BZ case, however, the zoneinfo in effect does not have a DST
defined (or, as we'll see later, hasn't had DST in a long time).  If
there's no DST, and you set tm_isdst=1, what happens?

Well, prior to 2.29, mktime just overrode tm_isdst and returned a
suitable time according to the current zoneinfo, as if you had passed
tm_isdst=0 or -1 instead.

As of 2.29, we have commit 86aece3bfbd44538ba4fdc947872c81d4c5e6e61
by Paul which includes:

    (__mktime_internal): Set errno to EOVERFLOW if the spring-forward
    gap code fails.

   /* We have a match.  Check whether tm.tm_isdst has the requested
      value, if any.  */
   if (isdst_differ (isdst, tm.tm_isdst))
     {
       . . .
+      __set_errno (EOVERFLOW);
+      return -1;
     }

With this change, tm_isdst becomes a hard requirement, and if the
current zone doesn't have a DST defined, you get a failure, where we
used to succeed (but with a non-DST result).

The relevent standards are pretty quiet on this topic; what little
they say can be interpreted either way - tm_isdst is a requirement, or
tm_isdst is a hint to be corrected by mktime() like other fields.

This breaks the logic down into three categories:

1. You're in a transition period where clock time repeats, and you
   need tm_isdst to decide which to return.

2. You're not in a transition period, and you might as well set
   tm_isdst=-1 unless you want an off-by-an-hour result.

3. Your zone doesn't have dst and setting tm_isdst=1 is meaningless.

I can't see an obvious way to detect case 1 from 2, so this seems to
be a useless set of categories.  A better breakdown would be:

1. You set tm_isdst=-1 by default.  Most of the time, this works.

2. If the time is ambiguous due to a transition, case 1 returns EAGAIN
   and you try again with tm_isdst=0 or 1.

3. If you set tm_isdst=0 or 1 outside of a transition, it returns
   EINVAL if it's incorrect for that time.

But that would be a BIG world-breaking change.  One can dream :-)

Meanwhile, I would like us to consider reverting the commit mentioned
above (not the whole commit, just the two lines I included).  This
will have the effect of making the current code bug-compatible with
older code, in that, setting tm_isdst=1 in a no-dst zone returns a
non-dst (but otherwise valid) time, and updates tm_idst to 0.
Returning EOVERFLOW in these new cases is not useful.