Skip to content

Commit

Permalink
Revert of Base: Make TimeDelta constructors deal with overflow. (patc…
Browse files Browse the repository at this point in the history 
…hset #2 id:20001 of https://codereview.chromium.org/1229853004/)

Reason for revert:
Reverting CL: it seems to cause regression in number of static initializers.
For details see https://code.google.com/p/chromium/issues/detail?id=512348

Original issue's description:
> Base: Make TimeDelta constructors deal with overflow.
>
> BUG=466445
>
> Committed: https://crrev.com/15462e858ebf34721e28a023b0b670c2011183ac
> Cr-Commit-Position: refs/heads/master@{#339605}

[email protected],[email protected]
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=466445

Review URL: https://codereview.chromium.org/1250453004

Cr-Commit-Position: refs/heads/master@{#339641}
  • Loading branch information
@JulienIsorce
antrim authored and JulienIsorce committed Jul 27, 2015
1 parent 65126a5 commit 964e2f0
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 100 deletions.
14 changes: 10 additions & 4 deletions base/time/time.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ Time Time::FromTimeT(time_t tt) {
return Time(); // Preserve 0 so we can tell it doesn't exist.
if (tt == std::numeric_limits<time_t>::max())
return Max();
return Time(kTimeTToMicrosecondsOffset) + TimeDelta::FromSeconds(tt);
return Time((tt * kMicrosecondsPerSecond) + kTimeTToMicrosecondsOffset);
}

time_t Time::ToTimeT() const {
Expand All @@ -166,7 +166,11 @@ time_t Time::ToTimeT() const {
Time Time::FromDoubleT(double dt) {
if (dt == 0 || std::isnan(dt))
return Time(); // Preserve 0 so we can tell it doesn't exist.
return Time(kTimeTToMicrosecondsOffset) + TimeDelta::FromSecondsD(dt);
if (dt == std::numeric_limits<double>::infinity())
return Max();
return Time(static_cast<int64>((dt *
static_cast<double>(kMicrosecondsPerSecond)) +
kTimeTToMicrosecondsOffset));
}

double Time::ToDoubleT() const {
Expand All @@ -193,8 +197,10 @@ Time Time::FromTimeSpec(const timespec& ts) {
Time Time::FromJsTime(double ms_since_epoch) {
// The epoch is a valid time, so this constructor doesn't interpret
// 0 as the null time.
return Time(kTimeTToMicrosecondsOffset) +
TimeDelta::FromMillisecondsD(ms_since_epoch);
if (ms_since_epoch == std::numeric_limits<double>::infinity())
return Max();
return Time(static_cast<int64>(ms_since_epoch * kMicrosecondsPerMillisecond) +
kTimeTToMicrosecondsOffset);
}

double Time::ToJsTime() const {
Expand Down
40 changes: 22 additions & 18 deletions base/time/time.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,6 @@ class BASE_EXPORT TimeDelta {
explicit TimeDelta(int64 delta_us) : delta_(delta_us) {
}

// Private method to build a delta from a double.
static TimeDelta FromDouble(double value);

// Delta in microseconds.
int64 delta_;
};
Expand Down Expand Up @@ -600,61 +597,68 @@ class BASE_EXPORT Time : public time_internal::TimeBase<Time> {

// static
inline TimeDelta TimeDelta::FromDays(int days) {
// Preserve max to prevent overflow.
if (days == std::numeric_limits<int>::max())
return Max();
return TimeDelta(days * Time::kMicrosecondsPerDay);
}

// static
inline TimeDelta TimeDelta::FromHours(int hours) {
// Preserve max to prevent overflow.
if (hours == std::numeric_limits<int>::max())
return Max();
return TimeDelta(hours * Time::kMicrosecondsPerHour);
}

// static
inline TimeDelta TimeDelta::FromMinutes(int minutes) {
// Preserve max to prevent overflow.
if (minutes == std::numeric_limits<int>::max())
return Max();
return TimeDelta(minutes * Time::kMicrosecondsPerMinute);
}

// static
inline TimeDelta TimeDelta::FromSeconds(int64 secs) {
return TimeDelta(secs) * Time::kMicrosecondsPerSecond;
// Preserve max to prevent overflow.
if (secs == std::numeric_limits<int64>::max())
return Max();
return TimeDelta(secs * Time::kMicrosecondsPerSecond);
}

// static
inline TimeDelta TimeDelta::FromMilliseconds(int64 ms) {
return TimeDelta(ms) * Time::kMicrosecondsPerMillisecond;
// Preserve max to prevent overflow.
if (ms == std::numeric_limits<int64>::max())
return Max();
return TimeDelta(ms * Time::kMicrosecondsPerMillisecond);
}

// static
inline TimeDelta TimeDelta::FromSecondsD(double secs) {
return FromDouble(secs * Time::kMicrosecondsPerSecond);
// Preserve max to prevent overflow.
if (secs == std::numeric_limits<double>::infinity())
return Max();
return TimeDelta(static_cast<int64>(secs * Time::kMicrosecondsPerSecond));
}

// static
inline TimeDelta TimeDelta::FromMillisecondsD(double ms) {
return FromDouble(ms * Time::kMicrosecondsPerMillisecond);
// Preserve max to prevent overflow.
if (ms == std::numeric_limits<double>::infinity())
return Max();
return TimeDelta(static_cast<int64>(ms * Time::kMicrosecondsPerMillisecond));
}

// static
inline TimeDelta TimeDelta::FromMicroseconds(int64 us) {
// Preserve max to prevent overflow.
if (us == std::numeric_limits<int64>::max())
return Max();
return TimeDelta(us);
}

// static
inline TimeDelta TimeDelta::FromDouble(double value) {
double max_magnitude = std::numeric_limits<int64>::max();
TimeDelta delta = TimeDelta(static_cast<int64>(value));
if (value > max_magnitude)
delta = Max();
else if (value < -max_magnitude)
delta = -Max();
return delta;
}

// For logging use only.
BASE_EXPORT std::ostream& operator<<(std::ostream& os, Time time);

Expand Down
130 changes: 52 additions & 78 deletions base/time/time_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,52 @@ TEST_F(TimeTest, ExplodeBeforeUnixEpoch) {
EXPECT_EQ(1, exploded.millisecond);
}

TEST_F(TimeTest, TimeDeltaMax) {
TimeDelta max = TimeDelta::Max();
EXPECT_TRUE(max.is_max());
EXPECT_EQ(max, TimeDelta::Max());
EXPECT_GT(max, TimeDelta::FromDays(100 * 365));
EXPECT_GT(max, TimeDelta());
}

TEST_F(TimeTest, TimeDeltaMaxConversions) {
TimeDelta t = TimeDelta::Max();
EXPECT_EQ(std::numeric_limits<int64>::max(), t.ToInternalValue());

EXPECT_EQ(std::numeric_limits<int>::max(), t.InDays());
EXPECT_EQ(std::numeric_limits<int>::max(), t.InHours());
EXPECT_EQ(std::numeric_limits<int>::max(), t.InMinutes());
EXPECT_EQ(std::numeric_limits<double>::infinity(), t.InSecondsF());
EXPECT_EQ(std::numeric_limits<int64>::max(), t.InSeconds());
EXPECT_EQ(std::numeric_limits<double>::infinity(), t.InMillisecondsF());
EXPECT_EQ(std::numeric_limits<int64>::max(), t.InMilliseconds());
EXPECT_EQ(std::numeric_limits<int64>::max(), t.InMillisecondsRoundedUp());

t = TimeDelta::FromDays(std::numeric_limits<int>::max());
EXPECT_TRUE(t.is_max());

t = TimeDelta::FromHours(std::numeric_limits<int>::max());
EXPECT_TRUE(t.is_max());

t = TimeDelta::FromMinutes(std::numeric_limits<int>::max());
EXPECT_TRUE(t.is_max());

t = TimeDelta::FromSeconds(std::numeric_limits<int64>::max());
EXPECT_TRUE(t.is_max());

t = TimeDelta::FromMilliseconds(std::numeric_limits<int64>::max());
EXPECT_TRUE(t.is_max());

t = TimeDelta::FromSecondsD(std::numeric_limits<double>::infinity());
EXPECT_TRUE(t.is_max());

t = TimeDelta::FromMillisecondsD(std::numeric_limits<double>::infinity());
EXPECT_TRUE(t.is_max());

t = TimeDelta::FromMicroseconds(std::numeric_limits<int64>::max());
EXPECT_TRUE(t.is_max());
}

TEST_F(TimeTest, Max) {
Time max = Time::Max();
EXPECT_TRUE(max.is_max());
Expand Down Expand Up @@ -749,7 +795,6 @@ TEST(TimeDelta, FromAndIn) {
EXPECT_EQ(13, TimeDelta::FromMillisecondsD(13.3).InMilliseconds());
EXPECT_EQ(13.3, TimeDelta::FromMillisecondsD(13.3).InMillisecondsF());
EXPECT_EQ(13, TimeDelta::FromMicroseconds(13).InMicroseconds());
EXPECT_EQ(3.456, TimeDelta::FromMillisecondsD(3.45678).InMillisecondsF());
}

#if defined(OS_POSIX)
Expand Down Expand Up @@ -823,83 +868,6 @@ TEST(TimeDelta, Magnitude) {
TimeDelta::FromMicroseconds(min_int64_plus_two).magnitude());
}

TEST(TimeDelta, Max) {
TimeDelta max = TimeDelta::Max();
EXPECT_TRUE(max.is_max());
EXPECT_EQ(max, TimeDelta::Max());
EXPECT_GT(max, TimeDelta::FromDays(100 * 365));
EXPECT_GT(max, TimeDelta());
}

bool IsMin(TimeDelta delta) {
return (-delta).is_max();
}

TEST(TimeDelta, MaxConversions) {
TimeDelta t = TimeDelta::Max();
EXPECT_EQ(std::numeric_limits<int64>::max(), t.ToInternalValue());

EXPECT_EQ(std::numeric_limits<int>::max(), t.InDays());
EXPECT_EQ(std::numeric_limits<int>::max(), t.InHours());
EXPECT_EQ(std::numeric_limits<int>::max(), t.InMinutes());
EXPECT_EQ(std::numeric_limits<double>::infinity(), t.InSecondsF());
EXPECT_EQ(std::numeric_limits<int64>::max(), t.InSeconds());
EXPECT_EQ(std::numeric_limits<double>::infinity(), t.InMillisecondsF());
EXPECT_EQ(std::numeric_limits<int64>::max(), t.InMilliseconds());
EXPECT_EQ(std::numeric_limits<int64>::max(), t.InMillisecondsRoundedUp());

t = TimeDelta::FromDays(std::numeric_limits<int>::max());
EXPECT_TRUE(t.is_max());

t = TimeDelta::FromHours(std::numeric_limits<int>::max());
EXPECT_TRUE(t.is_max());

t = TimeDelta::FromMinutes(std::numeric_limits<int>::max());
EXPECT_TRUE(t.is_max());

int64 max_int = std::numeric_limits<int64>::max();

t = TimeDelta::FromSeconds(max_int / Time::kMicrosecondsPerSecond + 1);
EXPECT_TRUE(t.is_max());

t = TimeDelta::FromMilliseconds(max_int / Time::kMillisecondsPerSecond + 1);
EXPECT_TRUE(t.is_max());

t = TimeDelta::FromMicroseconds(max_int);
EXPECT_TRUE(t.is_max());

t = TimeDelta::FromSeconds(-max_int / Time::kMicrosecondsPerSecond - 1);
EXPECT_TRUE(IsMin(t));

t = TimeDelta::FromMilliseconds(-max_int / Time::kMillisecondsPerSecond - 1);
EXPECT_TRUE(IsMin(t));

t = TimeDelta::FromMicroseconds(-max_int);
EXPECT_TRUE(IsMin(t));

t = -TimeDelta::FromMicroseconds(std::numeric_limits<int64>::min());
EXPECT_FALSE(IsMin(t));

t = TimeDelta::FromSecondsD(std::numeric_limits<double>::infinity());
EXPECT_TRUE(t.is_max());

double max_d = max_int;

t = TimeDelta::FromSecondsD(max_d / Time::kMicrosecondsPerSecond + 1);
EXPECT_TRUE(t.is_max());

t = TimeDelta::FromMillisecondsD(std::numeric_limits<double>::infinity());
EXPECT_TRUE(t.is_max());

t = TimeDelta::FromMillisecondsD(max_d / Time::kMillisecondsPerSecond * 2);
EXPECT_TRUE(t.is_max());

t = TimeDelta::FromSecondsD(-max_d / Time::kMicrosecondsPerSecond - 1);
EXPECT_TRUE(IsMin(t));

t = TimeDelta::FromMillisecondsD(-max_d / Time::kMillisecondsPerSecond * 2);
EXPECT_TRUE(IsMin(t));
}

TEST(TimeDelta, NumericOperators) {
double d = 0.5;
Expand All @@ -926,6 +894,7 @@ TEST(TimeDelta, NumericOperators) {
EXPECT_EQ(TimeDelta::FromMilliseconds(500),
f * TimeDelta::FromMilliseconds(1000));


int i = 2;
EXPECT_EQ(TimeDelta::FromMilliseconds(2000),
TimeDelta::FromMilliseconds(1000) * i);
Expand All @@ -950,6 +919,7 @@ TEST(TimeDelta, NumericOperators) {
EXPECT_EQ(TimeDelta::FromMilliseconds(2000),
i64 * TimeDelta::FromMilliseconds(1000));


EXPECT_EQ(TimeDelta::FromMilliseconds(500),
TimeDelta::FromMilliseconds(1000) * 0.5);
EXPECT_EQ(TimeDelta::FromMilliseconds(2000),
Expand All @@ -973,6 +943,10 @@ TEST(TimeDelta, NumericOperators) {
2 * TimeDelta::FromMilliseconds(1000));
}

bool IsMin(TimeDelta delta) {
return (-delta).is_max();
}

TEST(TimeDelta, Overflows) {
// Some sanity checks.
EXPECT_TRUE(TimeDelta::Max().is_max());
Expand Down

0 comments on commit 964e2f0

Please sign in to comment.