Skip to content

Commit 26af5da

Browse files
committed
libstd: Limit Duration range to i64 milliseconds.
This enables `num_milliseconds` to return an `i64` again instead of `Option<i64>`, because it is guaranteed not to overflow. The Duration range is now rougly 300e6 years (positive and negative), whereas it was 300e9 years previously. To put these numbers in perspective, 300e9 years is about 21 times the age of the universe (according to Wolfram|Alpha). 300e6 years is about 1/15 of the age of the earth (according to Wolfram|Alpha).
1 parent 39133ef commit 26af5da

File tree

4 files changed

+52
-54
lines changed

4 files changed

+52
-54
lines changed

src/libstd/io/net/tcp.rs

+1-7
Original file line numberDiff line numberDiff line change
@@ -105,23 +105,17 @@ impl TcpStream {
105105
///
106106
/// If a `timeout` with zero or negative duration is specified then
107107
/// the function returns `Err`, with the error kind set to `TimedOut`.
108-
/// If the timeout is larger than 2^63 milliseconds, the function also
109-
/// returns `Err` with the error kind set to `TimedOut`.
110108
#[experimental = "the timeout argument may eventually change types"]
111109
pub fn connect_timeout(addr: SocketAddr,
112110
timeout: Duration) -> IoResult<TcpStream> {
113111
if timeout <= Duration::milliseconds(0) {
114112
return Err(standard_error(TimedOut));
115113
}
116-
let timeout_ms = timeout.num_milliseconds().map(|x| { x as u64 });
117-
if timeout_ms.is_none() {
118-
return Err(standard_error(TimedOut));
119-
}
120114

121115
let SocketAddr { ip, port } = addr;
122116
let addr = rtio::SocketAddr { ip: super::to_rtio(ip), port: port };
123117
LocalIo::maybe_raise(|io| {
124-
io.tcp_connect(addr, timeout_ms).map(TcpStream::new)
118+
io.tcp_connect(addr, Some(timeout.num_milliseconds() as u64)).map(TcpStream::new)
125119
}).map_err(IoError::from_rtio_error)
126120
}
127121

src/libstd/io/net/unix.rs

+1-7
Original file line numberDiff line numberDiff line change
@@ -66,21 +66,15 @@ impl UnixStream {
6666
///
6767
/// If a `timeout` with zero or negative duration is specified then
6868
/// the function returns `Err`, with the error kind set to `TimedOut`.
69-
/// If the timeout is larger than 2^63 milliseconds, the function also
70-
/// returns `Err` with the error kind set to `TimedOut`.
7169
#[experimental = "the timeout argument is likely to change types"]
7270
pub fn connect_timeout<P: ToCStr>(path: &P,
7371
timeout: Duration) -> IoResult<UnixStream> {
7472
if timeout <= Duration::milliseconds(0) {
7573
return Err(standard_error(TimedOut));
7674
}
77-
let timeout_ms = timeout.num_milliseconds().map(|x| { x as u64 });
78-
if timeout_ms.is_none() {
79-
return Err(standard_error(TimedOut));
80-
}
8175

8276
LocalIo::maybe_raise(|io| {
83-
let s = io.unix_connect(&path.to_c_str(), timeout_ms);
77+
let s = io.unix_connect(&path.to_c_str(), Some(timeout.num_milliseconds() as u64));
8478
s.map(|p| UnixStream { obj: p })
8579
}).map_err(IoError::from_rtio_error)
8680
}

src/libstd/io/timer.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ impl Callback for TimerCallback {
225225
}
226226

227227
fn in_ms_u64(d: Duration) -> u64 {
228-
let ms = d.num_milliseconds().unwrap_or(0);
228+
let ms = d.num_milliseconds();
229229
if ms < 0 { return 0 };
230230
return ms as u64;
231231
}

src/libstd/time/duration.rs

+49-39
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,17 @@ pub struct Duration {
5151
nanos: i32, // Always 0 <= nanos < NANOS_PER_SEC
5252
}
5353

54-
/// The minimum possible `Duration`.
55-
pub static MIN: Duration = Duration { secs: i64::MIN, nanos: 0 };
56-
/// The maximum possible `Duration`.
57-
pub static MAX: Duration = Duration { secs: i64::MAX, nanos: NANOS_PER_SEC - 1 };
54+
/// The minimum possible `Duration`: `i64::MIN` milliseconds.
55+
pub static MIN: Duration = Duration {
56+
secs: i64::MIN / MILLIS_PER_SEC - 1,
57+
nanos: NANOS_PER_SEC + (i64::MIN % MILLIS_PER_SEC) as i32 * NANOS_PER_MILLI
58+
};
59+
60+
/// The maximum possible `Duration`: `i64::MAX` milliseconds.
61+
pub static MAX: Duration = Duration {
62+
secs: i64::MAX / MILLIS_PER_SEC,
63+
nanos: (i64::MAX % MILLIS_PER_SEC) as i32 * NANOS_PER_MILLI
64+
};
5865

5966
impl Duration {
6067
/// Makes a new `Duration` with given number of weeks.
@@ -94,9 +101,15 @@ impl Duration {
94101
}
95102

96103
/// Makes a new `Duration` with given number of seconds.
104+
/// Fails when the duration is more than `i64::MAX` milliseconds
105+
/// or less than `i64::MIN` milliseconds.
97106
#[inline]
98107
pub fn seconds(seconds: i64) -> Duration {
99-
Duration { secs: seconds, nanos: 0 }
108+
let d = Duration { secs: seconds, nanos: 0 };
109+
if d < MIN || d > MAX {
110+
fail!("Duration::seconds out of bounds");
111+
}
112+
d
100113
}
101114

102115
/// Makes a new `Duration` with given number of milliseconds.
@@ -167,11 +180,12 @@ impl Duration {
167180
}
168181

169182
/// Returns the total number of whole milliseconds in the duration,
170-
/// or `None` on overflow (exceeding 2^63 milliseconds in either direction).
171-
pub fn num_milliseconds(&self) -> Option<i64> {
172-
let secs_part = try_opt!(self.num_seconds().checked_mul(&MILLIS_PER_SEC));
183+
pub fn num_milliseconds(&self) -> i64 {
184+
// A proper Duration will not overflow, because MIN and MAX are defined
185+
// such that the range is exactly i64 milliseconds.
186+
let secs_part = self.num_seconds() * MILLIS_PER_SEC;
173187
let nanos_part = self.nanos_mod_sec() / NANOS_PER_MILLI;
174-
secs_part.checked_add(&(nanos_part as i64))
188+
secs_part + nanos_part as i64
175189
}
176190

177191
/// Returns the total number of whole microseconds in the duration,
@@ -211,13 +225,7 @@ impl num::Zero for Duration {
211225
impl Neg<Duration> for Duration {
212226
#[inline]
213227
fn neg(&self) -> Duration {
214-
if self.secs == i64::MIN && self.nanos == 0 {
215-
// The minimum value cannot be negated due to overflow. Use the
216-
// maximum value, which is one nanosecond less than the negated minimum.
217-
MAX
218-
} else if self.secs == i64::MIN {
219-
Duration { secs: i64::MAX, nanos: NANOS_PER_SEC - self.nanos }
220-
} else if self.nanos == 0 {
228+
if self.nanos == 0 {
221229
Duration { secs: -self.secs, nanos: 0 }
222230
} else {
223231
Duration { secs: -self.secs - 1, nanos: NANOS_PER_SEC - self.nanos }
@@ -245,7 +253,10 @@ impl num::CheckedAdd for Duration {
245253
nanos -= NANOS_PER_SEC;
246254
secs = try_opt!(secs.checked_add(&1));
247255
}
248-
Some(Duration { secs: secs, nanos: nanos })
256+
let d = Duration { secs: secs, nanos: nanos };
257+
// Even if d is within the bounds of i64 seconds,
258+
// it might still overflow i64 milliseconds.
259+
if d < MIN || d > MAX { None } else { Some(d) }
249260
}
250261
}
251262

@@ -269,7 +280,10 @@ impl num::CheckedSub for Duration {
269280
nanos += NANOS_PER_SEC;
270281
secs = try_opt!(secs.checked_sub(&1));
271282
}
272-
Some(Duration { secs: secs, nanos: nanos })
283+
let d = Duration { secs: secs, nanos: nanos };
284+
// Even if d is within the bounds of i64 seconds,
285+
// it might still overflow i64 milliseconds.
286+
if d < MIN || d > MAX { None } else { Some(d) }
273287
}
274288
}
275289

@@ -407,26 +421,22 @@ mod tests {
407421
assert_eq!(Duration::milliseconds(1001).num_seconds(), 1);
408422
assert_eq!(Duration::milliseconds(-999).num_seconds(), 0);
409423
assert_eq!(Duration::milliseconds(-1001).num_seconds(), -1);
410-
assert_eq!(Duration::seconds(i64::MAX).num_seconds(), i64::MAX);
411-
assert_eq!(Duration::seconds(i64::MIN).num_seconds(), i64::MIN);
412-
assert_eq!(MAX.num_seconds(), i64::MAX);
413-
assert_eq!(MIN.num_seconds(), i64::MIN);
414424
}
415425

416426
#[test]
417427
fn test_duration_num_milliseconds() {
418428
let d: Duration = Zero::zero();
419-
assert_eq!(d.num_milliseconds(), Some(0));
420-
assert_eq!(Duration::milliseconds(1).num_milliseconds(), Some(1));
421-
assert_eq!(Duration::milliseconds(-1).num_milliseconds(), Some(-1));
422-
assert_eq!(Duration::microseconds(999).num_milliseconds(), Some(0));
423-
assert_eq!(Duration::microseconds(1001).num_milliseconds(), Some(1));
424-
assert_eq!(Duration::microseconds(-999).num_milliseconds(), Some(0));
425-
assert_eq!(Duration::microseconds(-1001).num_milliseconds(), Some(-1));
426-
assert_eq!(Duration::milliseconds(i64::MAX).num_milliseconds(), Some(i64::MAX));
427-
assert_eq!(Duration::milliseconds(i64::MIN).num_milliseconds(), Some(i64::MIN));
428-
assert_eq!(MAX.num_milliseconds(), None);
429-
assert_eq!(MIN.num_milliseconds(), None);
429+
assert_eq!(d.num_milliseconds(), 0);
430+
assert_eq!(Duration::milliseconds(1).num_milliseconds(), 1);
431+
assert_eq!(Duration::milliseconds(-1).num_milliseconds(), -1);
432+
assert_eq!(Duration::microseconds(999).num_milliseconds(), 0);
433+
assert_eq!(Duration::microseconds(1001).num_milliseconds(), 1);
434+
assert_eq!(Duration::microseconds(-999).num_milliseconds(), 0);
435+
assert_eq!(Duration::microseconds(-1001).num_milliseconds(), -1);
436+
assert_eq!(Duration::milliseconds(i64::MAX).num_milliseconds(), i64::MAX);
437+
assert_eq!(Duration::milliseconds(i64::MIN).num_milliseconds(), i64::MIN);
438+
assert_eq!(MAX.num_milliseconds(), i64::MAX);
439+
assert_eq!(MIN.num_milliseconds(), i64::MIN);
430440
}
431441

432442
#[test]
@@ -477,13 +487,13 @@ mod tests {
477487

478488
#[test]
479489
fn test_duration_checked_ops() {
480-
assert_eq!(Duration::seconds(i64::MAX).checked_add(&Duration::milliseconds(999)),
481-
Some(Duration::seconds(i64::MAX - 1) + Duration::milliseconds(1999)));
482-
assert!(Duration::seconds(i64::MAX).checked_add(&Duration::milliseconds(1000)).is_none());
490+
assert_eq!(Duration::milliseconds(i64::MAX - 1).checked_add(&Duration::microseconds(999)),
491+
Some(Duration::milliseconds(i64::MAX - 2) + Duration::microseconds(1999)));
492+
assert!(Duration::milliseconds(i64::MAX).checked_add(&Duration::microseconds(1000)).is_none());
483493

484-
assert_eq!(Duration::seconds(i64::MIN).checked_sub(&Duration::seconds(0)),
485-
Some(Duration::seconds(i64::MIN)));
486-
assert!(Duration::seconds(i64::MIN).checked_sub(&Duration::seconds(1)).is_none());
494+
assert_eq!(Duration::milliseconds(i64::MIN).checked_sub(&Duration::milliseconds(0)),
495+
Some(Duration::milliseconds(i64::MIN)));
496+
assert!(Duration::milliseconds(i64::MIN).checked_sub(&Duration::milliseconds(1)).is_none());
487497
}
488498

489499
#[test]

0 commit comments

Comments
 (0)