diff mbox

tzset did not catch changes tolocaltime  [BZ #21060 ]

Message ID 201812172006103612868@zte.com.cn
State New, archived
Headers

Commit Message

ma.jiang@zte.com.cn Dec. 17, 2018, 12:06 p.m. UTC 
  Hi schwab,
  Thanks for the quick reply.
  I have changed the patch into the following one, as you suggested.   Is it OK now?





------------------原始邮件------------------
发件人：AndreasSchwab <schwab@suse.de>
收件人：马江10100629;
抄送人：carlos@redhat.com <carlos@redhat.com>;libc-alpha@sourceware.org <libc-alpha@sourceware.org>;
日 期 ：2018年12月17日 17:40
主 题 ：Re: [PATCH]   tzset did not catch changes tolocaltime  [BZ #21060 ]
On Dez 17 2018, <ma.jiang@zte.com.cn> wrote:

> +  /* Check whether the tz value changed since the last run.  */
> +  int tz_changed = old_tz ? strcmp (tz, old_tz) : 1;

This should be written as old_tz == NULL || strcmp (tz, old_tz) != 0,
and tz_changed should be bool.

> +  if ((old_tz == NULL)
> +      || (old_tz != NULL && tz_changed))

This is the same as tz_changed.

> +  {
> +    /* Save the value of `tz'. */
> +    free (old_tz);
> +    old_tz = __strdup (tz);
> +  }

Wrong indentation.

Andreas.

--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Comments

Paul Eggert Dec. 17, 2018, 10:23 p.m. UTC | #1 
I followed up in the bug report, here:

https://sourceware.org/bugzilla/show_bug.cgi?id=21060#c1
Carlos O'Donell Dec. 18, 2018, 4:24 a.m. UTC | #2 
On 12/17/18 5:23 PM, Paul Eggert wrote:
> I followed up in the bug report, here:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=21060#c1
> 

In the case of reloading if TZ or /etc/localtime has changed we are in
the same realm of problem as /etc/resolv.conf reloading, it's something
we would like to be able to do, but you're going to need more cleverness
to avoid the costly operations, otherwise it's lost performance for
little to no gain. Most developers don't change /etc/localtime, and in
many countries it is a stable set of daylight savings transitions (and
if they change you need to restart).

The naive implementation is not going to be sufficient.
diff mbox

Patch

diff --git a/time/tzset.c b/time/tzset.c
index b517867..1431097 100644
--- a/time/tzset.c
+++ b/time/tzset.c
@@ -386,22 +386,27 @@  tzset_internal (int always)
   if (tz && *tz == ':')
     ++tz;

-  /* Check whether the value changed since the last run.  */
-  if (old_tz != NULL && tz != NULL && strcmp (tz, old_tz) == 0)
-    /* No change, simply return.  */
-    return;
-
   if (tz == NULL)
     /* No user specification; use the site-wide default.  */
     tz = TZDEFAULT;

+  /* Check whether the tz value changed since the last run.  */
+  bool tz_changed = (old_tz == NULL) || (strcmp (tz, old_tz) != 0);
+  if (old_tz == NULL || tz_changed)
+    {
+      /* Save the value of `tz'. */
+      free (old_tz);
+      old_tz = __strdup (tz);
+    }
+  
+  /* When using tzfile, must check whether it changed in  __tzfile_read.  */
+  if (!__use_tzfile && !tz_changed)
+    return;
+
+  /* Going to recaculate values. */
   tz_rules[0].name = NULL;
   tz_rules[1].name = NULL;

-  /* Save the value of `tz'.  */
-  free (old_tz);
-  old_tz = tz ? __strdup (tz) : NULL;
-
   /* Try to read a data file.  */
   __tzfile_read (tz, 0, NULL);
   if (__use_tzfile)