Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3937)

Unified Diff: base/time/time_posix.cc

Issue 27472003: android: fix base::Time::FromLocalExploded() crash. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Use new code paths on all platforms. Created 7 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | base/time/time_unittest.cc » ('j') | base/time/time_unittest.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/time/time_posix.cc
diff --git a/base/time/time_posix.cc b/base/time/time_posix.cc
index 5e06ce93cf039fd420549a345915e2d0ffd62bea..c1edb7a27298a4e7ec5a31ad2383e11879336bbe 100644
--- a/base/time/time_posix.cc
+++ b/base/time/time_posix.cc
@@ -214,9 +214,41 @@ Time Time::FromExploded(bool is_local, const Exploded& exploded) {
timestruct.tm_zone = NULL; // not a POSIX field, so mktime/timegm ignore
#endif
- SysTime seconds = SysTimeFromTimeStruct(&timestruct, is_local);
int64 milliseconds;
+ SysTime seconds;
+
+ // Certain exploded dates do not really exist due to daylight saving times,
+ // and this causes mktime() to return implementation-defined values when
+ // tm_isdst is set to -1. On Android, the function will return -1, while the
+ // C libraries of other platforms typically return a liberally-chosen value.
+ // Handling this requires the special code below.
+
+ // SysTimeFromTimeStruct() modifies the input structure, save current value.
+ struct tm timestruct0 = timestruct;
+
+ seconds = SysTimeFromTimeStruct(&timestruct, is_local);
+ if (seconds == -1) {
+ // Get the time values with tm_isdst == 0 and 1, then select the closest one
+ // to UTC 00:00:00 that isn't -1.
+ timestruct = timestruct0;
+ timestruct.tm_isdst = 0;
+ int64 seconds_isdst0 = SysTimeFromTimeStruct(&timestruct, is_local);
+
+ timestruct = timestruct0;
+ timestruct.tm_isdst = 1;
+ int64 seconds_isdst1 = SysTimeFromTimeStruct(&timestruct, is_local);
+
+ // seconds_isdst0 or seconds_isdst1 can be -1 for some timezones.
+ // E.g. "CLST" (Chile Summer Time) returns -1 for 'tm_isdt == 1'.
+ if (seconds_isdst0 < 0)
+ seconds = seconds_isdst1;
+ else if (seconds_isdst1 < 0)
+ seconds = seconds_isdst0;
+ else
+ seconds = std::min(seconds_isdst0, seconds_isdst1);
+ }
+
// Handle overflow. Clamping the range to what mktime and timegm might
// return is the best that can be done here. It's not ideal, but it's better
// than failing here or ignoring the overflow case and treating each time
@@ -237,14 +269,19 @@ Time Time::FromExploded(bool is_local, const Exploded& exploded) {
// When representing the most distant time in the future, add in an extra
// 999ms to avoid the time being less than any other possible value that
// this function can return.
+
+ // On Android, SysTime is int64, special care must be taken to avoid
+ // overflows.
+ const int64 min_seconds = (sizeof(SysTime) < sizeof(int64))
+ ? std::numeric_limits<SysTime>::min()
+ : std::numeric_limits<int32_t>::min();
+ const int64 max_seconds = (sizeof(SysTime) < sizeof(int64))
+ ? std::numeric_limits<SysTime>::max()
+ : std::numeric_limits<int32_t>::max();
if (exploded.year < 1969) {
- CHECK(sizeof(SysTime) < sizeof(int64)) << "integer overflow";
- milliseconds = std::numeric_limits<SysTime>::min();
- milliseconds *= kMillisecondsPerSecond;
+ milliseconds = min_seconds * kMillisecondsPerSecond;
} else {
- CHECK(sizeof(SysTime) < sizeof(int64)) << "integer overflow";
- milliseconds = std::numeric_limits<SysTime>::max();
- milliseconds *= kMillisecondsPerSecond;
+ milliseconds = max_seconds * kMillisecondsPerSecond;
milliseconds += (kMillisecondsPerSecond - 1);
}
} else {
« no previous file with comments | « no previous file | base/time/time_unittest.cc » ('j') | base/time/time_unittest.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698