Skip to contentSkip to conversation

qemu_timedate_diff() shouldn't modify its argument.

PATCH Applied Gleb Natapov 4 participants 5 messages Nov 6, 2011 – Nov 8, 2011

lore
Share & tools
lore

Gleb Natapov

author
#0 lore raw

PATCH 1/1 qemu_timedate_diff() shouldn't modify its argument.

The caller of qemu_timedate_diff() does not expect that tm it passes to the function will be modified, but mktime() is destructive and modifies its argument. Pass a copy of tm to it and set tm_isdst so that mktime() will not rely on it since its value may be outdated.

Signed-off-by Gleb Natapov <[email protected]>
vl.c +5 -2
index 624da0f..641629b 100644
@@ -460,8 +460,11 @@ int qemu_timedate_diff(struct tm *tm)
     if (rtc_date_offset == -1)
         if (rtc_utc)
             seconds = mktimegm(tm);
-        else
-            seconds = mktime(tm);
+        else {
+            struct tm tmp = *tm;
+            tmp.tm_isdst = -1; /* use timezone to figure it out */
+            seconds = mktime(&tmp);
+	}
     else
         seconds = mktimegm(tm) + rtc_date_offset;

--
Gleb.

Acked-by on v1 1/1
#1 lore raw
Quoted 4 lines · Gleb Natapov

The caller of qemu_timedate_diff() does not expect that tm it passes to the function will be modified, but mktime() is destructive and modifies its argument. Pass a copy of tm to it and set tm_isdst so that mktime() will not rely on it since its value may be outdated.

Ohhh, nice catch.

Quoted 1 line
Signed-off-by Gleb Natapov<[email protected]>
Acked-by Rik van Riel <[email protected]>
Signature · 1 line
All rights reversed
Acked-by on v1 1/1
#2 lore raw
In reply to Gleb Natapov
Quoted 4 lines · Gleb Natapov

The caller of qemu_timedate_diff() does not expect that tm it passes to the function will be modified, but mktime() is destructive and modifies its argument. Pass a copy of tm to it and set tm_isdst so that mktime() will not rely on it since its value may be outdated.

I believe that the original issue was not related to outdated data at the moment of the daylight saving time transition.
using tmp.tm_isdst = -1 sounds good, but why use a copy of tm? The only significant field that will change in the tm is the tm_isdst itself that will be set to 0/1 (correctly).

Acked-by Ronen Hod <[email protected]>
Quoted 22 lines
Signed-off-by Gleb Natapov<[email protected]>
vl.c +5 -3
index 624da0f..641629b 100644
@@ -460,8 +460,11 @@ int qemu_timedate_diff(struct tm *tm)
      if (rtc_date_offset == -1)
          if (rtc_utc)
              seconds = mktimegm(tm);
-        else
-            seconds = mktime(tm);
+        else {
+            struct tm tmp = *tm;
+            tmp.tm_isdst = -1; /* use timezone to figure it out */
+            seconds = mktime(&tmp);
+	}
      else
          seconds = mktimegm(tm) + rtc_date_offset;
 
--

Gleb.

author
#3 lore raw
Quoted 8 lines · Ronen Hod
Quoted 4 lines · Gleb Natapov

The caller of qemu_timedate_diff() does not expect that tm it passes to the function will be modified, but mktime() is destructive and modifies its argument. Pass a copy of tm to it and set tm_isdst so that mktime() will not rely on it since its value may be outdated.

I believe that the original issue was not related to outdated data at the moment of the daylight saving time transition.

Don't know what do you mean here. The problem is definitely related to incorrect data about daylight saving time.

Quoted 1 line

using tmp.tm_isdst = -1 sounds good, but why use a copy of tm? The

Hmm, what is not clear about "qemu_timedate_diff() shouldn't modify its
argument"? :)

Quoted 2 lines

only significant field that will change in the tm is the tm_isdst itself that will be set to 0/1 (correctly).

This is incorrect (read man), but event if it would breed a new kind of especially handsome ponies, if this is what caller wants it should call mktime() by itself and not be a side effect of some random function call.

Quoted 22 lines
Acked-by Ronen Hod <[email protected]>
Quoted 19 lines
Signed-off-by Gleb Natapov<[email protected]>
vl.c +5 -2
index 624da0f..641629b 100644
@@ -460,8 +460,11 @@ int qemu_timedate_diff(struct tm *tm)
     if (rtc_date_offset == -1)
         if (rtc_utc)
             seconds = mktimegm(tm);
-        else
-            seconds = mktime(tm);
+        else {
+            struct tm tmp = *tm;
+            tmp.tm_isdst = -1; /* use timezone to figure it out */
+            seconds = mktime(&tmp);
+	}
     else
         seconds = mktimegm(tm) + rtc_date_offset;
Quoted 3 lines
Quoted 3 lines

--
Gleb.

--
Gleb.

Applied
#4 lore raw
In reply to Gleb Natapov
Quoted 6 lines · Gleb Natapov

The caller of qemu_timedate_diff() does not expect that tm it passes to the function will be modified, but mktime() is destructive and modifies its argument. Pass a copy of tm to it and set tm_isdst so that mktime() will not rely on it since its value may be outdated.

Signed-off-by Gleb Natapov<[email protected]>

Applied. Thanks.

Regards,

Anthony Liguori

Quoted 22 lines
vl.c +5 -3
index 624da0f..641629b 100644
@@ -460,8 +460,11 @@ int qemu_timedate_diff(struct tm *tm)
      if (rtc_date_offset == -1)
          if (rtc_utc)
              seconds = mktimegm(tm);
-        else
-            seconds = mktime(tm);
+        else {
+            struct tm tmp = *tm;
+            tmp.tm_isdst = -1; /* use timezone to figure it out */
+            seconds = mktime(&tmp);
+	}
      else
          seconds = mktimegm(tm) + rtc_date_offset;
 
--

Gleb.