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

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: 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') | no next file with comments »
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..e2879e7440ba98645bacf752b4b9ea38b3354c1d 100644
--- a/base/time/time_posix.cc
+++ b/base/time/time_posix.cc
@@ -214,9 +214,29 @@ 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;
+
+// On older releases of Android, mktime() will sometimes return -1 when given
+// any second after midnight in certain timezones. For more details, see:
+// http://b.android.com/61137
+#if defined(OS_ANDROID)
+ // SysTimeFromTimeStruct() modifies the input structure, save current value.
+ struct tm timestruct0 = timestruct;
+
+ seconds = SysTimeFromTimeStruct(&timestruct, is_local);
+ if (seconds == -1 && exploded.hour == 0) {
+ // Work-around this by trying to add one hour.
+ timestruct0.tm_hour = 1;
+ seconds = SysTimeFromTimeStruct(&timestruct0, is_local);
+ if (seconds != -1)
+ seconds -= 60 * 60;
+ }
+#else
+ seconds = SysTimeFromTimeStruct(&timestruct, is_local);
+#endif
+
// 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 +257,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();
digit1 2013/10/17 14:00:27 I'm still not sure that using INT32_MAX here is su
+ 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') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698