public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Paul Eggert <eggert@cs.ucla.edu>
To: DJ Delorie <dj@redhat.com>
Cc: libc-alpha@sourceware.org
Subject: Re: [swbz 29035] mktime vs non-DST
Date: Wed, 17 Aug 2022 16:10:22 -0700	[thread overview]
Message-ID: <e4757fd0-0eb5-067f-fed9-db21818c1a16@cs.ucla.edu> (raw)
In-Reply-To: <xnv8qq601w.fsf@greed.delorie.com>

[-- Attachment #1: Type: text/plain, Size: 1268 bytes --]

On 8/17/22 14:18, DJ Delorie wrote:
> 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.

Actually, mktime doesn't and never did that in general, because in 
general there's no such thing as the "local DST offset": within a single 
TZ setting the difference between standard time and DST need not be 
constant. For example, if we are currently in standard time, it's 
possible that the next spring-forward will add 30 minutes, while the 
previous fall-backward subtracted an hour. In such a situation the 
"local DST offset" could reasonably be thought of as 30 minutes, or 60 
minutes, or even something else if there were other transitions of 
varying sizes at other times.

Although I'm not seeing how BZ#29035 led to your diagnosis (Andreas 
didn't reproduce that bug), we did run into a similar problem in Gnulib, 
and I installed into Gnulib a patch to fix that. So I suggest syncing 
with Gnulib. This is a bit fancier than what you suggested, but I expect 
it'll fix BZ#29035 while it's also fixing the bug reported against Gnulib.

Proposed glibc patches attached. Their effect is to sync glibc from 
Gnulib. If these patches don't fix the problem you diagnosed please let 
me know.

[-- Attachment #2: 0001-Assume-HAVE_TZSET-in-time-mktime.c.patch --]
[-- Type: text/x-patch, Size: 709 bytes --]

From 674d4588995d24f8bc5167f73d8661ef5c82c2de Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 17 Aug 2022 15:35:47 -0700
Subject: [PATCH 1/2] Assume HAVE_TZSET in time/mktime.c

This patch does not affect glibc.  It affects only Gnulib-specific
code and is for coordination with Gnulib.
---
 time/mktime.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/time/mktime.c b/time/mktime.c
index 494c89bf54..aa12e28e16 100644
--- a/time/mktime.c
+++ b/time/mktime.c
@@ -94,7 +94,7 @@ my_tzset (void)
   const char *tz = getenv ("TZ");
   if (tz != NULL && strchr (tz, '/') != NULL)
     _putenv ("TZ=");
-# elif HAVE_TZSET
+# else
   tzset ();
 # endif
 }
-- 
2.34.1


[-- Attachment #3: 0002-mktime-improve-heuristic-for-ca-1986-Indiana-DST.patch --]
[-- Type: text/x-patch, Size: 3198 bytes --]

From bfe919ce3bba22178429c5b39db98c26fd83272e Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 17 Aug 2022 15:38:54 -0700
Subject: [PATCH 2/2] mktime: improve heuristic for ca-1986 Indiana DST

This patch syncs mktime.c from Gnulib, fixing a
problem reported by Mark Krenz <https://bugs.gnu.org/48085>,
and it should fix BZ#29035 too.
* time/mktime.c (__mktime_internal): Be more generous about
accepting arguments with the wrong value of tm_isdst, by falling
back to a one-hour DST difference if we find no nearby DST that is
unusual.  This fixes a problem where "1986-04-28 00:00 EDT" was
rejected when TZ="America/Indianapolis" because the nearest DST
timestamp occurred in 1970, a temporal distance too great for the
old heuristic.  This also also narrows the search a bit, which
is a minor performance win.
---
 time/mktime.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/time/mktime.c b/time/mktime.c
index aa12e28e16..7dc9d67ef9 100644
--- a/time/mktime.c
+++ b/time/mktime.c
@@ -429,8 +429,13 @@ __mktime_internal (struct tm *tp,
 	 time with the right value, and use its UTC offset.
 
 	 Heuristic: probe the adjacent timestamps in both directions,
-	 looking for the desired isdst.  This should work for all real
-	 time zone histories in the tz database.  */
+	 looking for the desired isdst.  If none is found within a
+	 reasonable duration bound, assume a one-hour DST difference.
+	 This should work for all real time zone histories in the tz
+	 database.  */
+
+      /* +1 if we wanted standard time but got DST, -1 if the reverse.  */
+      int dst_difference = (isdst == 0) - (tm.tm_isdst == 0);
 
       /* Distance between probes when looking for a DST boundary.  In
 	 tzdata2003a, the shortest period of DST is 601200 seconds
@@ -441,12 +446,14 @@ __mktime_internal (struct tm *tp,
 	 periods when probing.  */
       int stride = 601200;
 
-      /* The longest period of DST in tzdata2003a is 536454000 seconds
-	 (e.g., America/Jujuy starting 1946-10-01 01:00).  The longest
-	 period of non-DST is much longer, but it makes no real sense
-	 to search for more than a year of non-DST, so use the DST
-	 max.  */
-      int duration_max = 536454000;
+      /* In TZDB 2021e, the longest period of DST (or of non-DST), in
+	 which the DST (or adjacent DST) difference is not one hour,
+	 is 457243209 seconds: e.g., America/Cambridge_Bay with leap
+	 seconds, starting 1965-10-31 00:00 in a switch from
+	 double-daylight time (-05) to standard time (-07), and
+	 continuing to 1980-04-27 02:00 in a switch from standard time
+	 (-07) to daylight time (-06).  */
+      int duration_max = 457243209;
 
       /* Search in both directions, so the maximum distance is half
 	 the duration; add the stride to avoid off-by-1 problems.  */
@@ -483,6 +490,11 @@ __mktime_internal (struct tm *tp,
 	      }
 	  }
 
+      /* No unusual DST offset was found nearby.  Assume one-hour DST.  */
+      t += 60 * 60 * dst_difference;
+      if (mktime_min <= t && t <= mktime_max && convert_time (convert, t, &tm))
+	goto offset_found;
+
       __set_errno (EOVERFLOW);
       return -1;
     }
-- 
2.34.1


  parent reply	other threads:[~2022-08-17 23:10 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-17 21:18 DJ Delorie
2022-08-17 21:50 ` Carlos O'Donell
2022-08-17 23:10 ` Paul Eggert [this message]
2022-08-18  1:39   ` DJ Delorie
2022-08-18  2:37     ` Carlos O'Donell
2022-08-18  3:16       ` Paul Eggert
2022-08-18  4:05         ` Carlos O'Donell
2022-08-18 21:17       ` DJ Delorie
2022-08-18 21:57         ` Paul Eggert
2022-08-18 22:40           ` DJ Delorie
2022-08-18 22:58             ` Paul Eggert
2022-08-19 18:15               ` DJ Delorie
2022-08-19 22:04                 ` Paul Eggert
2022-08-18  3:02     ` Paul Eggert
2022-09-08 20:25   ` DJ Delorie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e4757fd0-0eb5-067f-fed9-db21818c1a16@cs.ucla.edu \
    --to=eggert@cs.ucla.edu \
    --cc=dj@redhat.com \
    --cc=libc-alpha@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).