Fixing Time::Max()'s behavior with Time::ToTimeT() and friends.

base/time.cc

Index: base/time.cc
diff --git a/base/time.cc b/base/time.cc
index 7055311e80e7d53df843b61dd63a67b816a74faa..e59de53d44718b4dd0e429a29e3e3d31108cd8d8 100644
--- a/base/time.cc
+++ b/base/time.cc
@@ -74,12 +74,18 @@ Time Time::Max() {
 Time Time::FromTimeT(time_t tt) {
   if (tt == 0)
     return Time();  // Preserve 0 so we can tell it doesn't exist.
+  if (tt == std::numeric_limits<time_t>::max())
+    return Max();
   return Time((tt * kMicrosecondsPerSecond) + kTimeTToMicrosecondsOffset);
 }
 
 time_t Time::ToTimeT() const {
-  if (us_ == 0)
+  if (is_null())
     return 0;  // Preserve 0 so we can tell it doesn't exist.
+  if (is_max()) {
+    // Preserve max without offset to prevent overflow.
+    return std::numeric_limits<time_t>::max();
+  }
   return (us_ - kTimeTToMicrosecondsOffset) / kMicrosecondsPerSecond;

[msarda, 2012/09/06 15:04:34]

There is still a risk of overflow here: us_ might still be big enough without
actually being equal to std::numeric_limits<int64>::max().
I would add a DLOG(WARNING) in case there is an overflow and return
std::numeric_limits<time_t>::max().

[Mike West, 2012/09/06 19:51:16]

Good point. I've added the log. This one looks like the only one that can
overflow based on the offset. The rest of the conversions are doubles, or on
platforms where the offset is 0.

 }
 
@@ -87,14 +93,20 @@ time_t Time::ToTimeT() const {
 Time Time::FromDoubleT(double dt) {
   if (dt == 0 || isnan(dt))
     return Time();  // Preserve 0 so we can tell it doesn't exist.
+  if (dt == std::numeric_limits<double>::max())
+    return Max();
   return Time(static_cast<int64>((dt *
                                   static_cast<double>(kMicrosecondsPerSecond)) +
                                  kTimeTToMicrosecondsOffset));
 }
 
 double Time::ToDoubleT() const {
-  if (us_ == 0)
+  if (is_null())
     return 0;  // Preserve 0 so we can tell it doesn't exist.
+  if (is_max()) {
+    // Preserve max without offset to prevent overflow.
+    return std::numeric_limits<double>::max();
+  }
   return (static_cast<double>(us_ - kTimeTToMicrosecondsOffset) /
           static_cast<double>(kMicrosecondsPerSecond));
 }
@@ -103,15 +115,21 @@ double Time::ToDoubleT() const {
 Time Time::FromJsTime(double ms_since_epoch) {
   // The epoch is a valid time, so this constructor doesn't interpret
   // 0 as the null time.
+  if (ms_since_epoch == std::numeric_limits<double>::max())
+    return Max();
   return Time(static_cast<int64>(ms_since_epoch * kMicrosecondsPerMillisecond) +
               kTimeTToMicrosecondsOffset);
 }
 
 double Time::ToJsTime() const {
-  if (us_ == 0) {
+  if (is_null()) {
     // Preserve 0 so the invalid result doesn't depend on the platform.
     return 0;
   }
+  if (is_max()) {
+    // Preserve max without offset to prevent overflow.
+    return std::numeric_limits<double>::max();
+  }
   return (static_cast<double>(us_ - kTimeTToMicrosecondsOffset) /
           kMicrosecondsPerMillisecond);
 }