From 39133efebf49823002977f0ad0ac12edf27b4d06 Mon Sep 17 00:00:00 2001 From: Ruud van Asseldonk Date: Tue, 19 Aug 2014 14:32:20 +0200 Subject: libstd: Refactor Duration. This changes the internal representation of `Duration` from days: i32, secs: i32, nanos: u32 to secs: i64, nanos: i32 This resolves #16466. Some methods now take `i64` instead of `i32` due to the increased range. Some methods, like `num_milliseconds`, now return an `Option` instead of `i64`, because the range of `Duration` is now larger than e.g. 2^63 milliseconds. --- src/libstd/io/net/tcp.rs | 12 +++++++++--- src/libstd/io/net/unix.rs | 10 ++++++++-- src/libstd/io/timer.rs | 2 +- 3 files changed, 18 insertions(+), 6 deletions(-) (limited to 'src/libstd/io') diff --git a/src/libstd/io/net/tcp.rs b/src/libstd/io/net/tcp.rs index 7055b9d7a47..52d3a04432a 100644 --- a/src/libstd/io/net/tcp.rs +++ b/src/libstd/io/net/tcp.rs @@ -97,25 +97,31 @@ impl TcpStream { /// the specified duration. /// /// This is the same as the `connect` method, except that if the timeout - /// specified (in milliseconds) elapses before a connection is made an error - /// will be returned. The error's kind will be `TimedOut`. + /// specified elapses before a connection is made an error will be + /// returned. The error's kind will be `TimedOut`. /// /// Note that the `addr` argument may one day be split into a separate host /// and port, similar to the API seen in `connect`. /// /// If a `timeout` with zero or negative duration is specified then /// the function returns `Err`, with the error kind set to `TimedOut`. + /// If the timeout is larger than 2^63 milliseconds, the function also + /// returns `Err` with the error kind set to `TimedOut`. #[experimental = "the timeout argument may eventually change types"] pub fn connect_timeout(addr: SocketAddr, timeout: Duration) -> IoResult { if timeout <= Duration::milliseconds(0) { return Err(standard_error(TimedOut)); } + let timeout_ms = timeout.num_milliseconds().map(|x| { x as u64 }); + if timeout_ms.is_none() { + return Err(standard_error(TimedOut)); + } let SocketAddr { ip, port } = addr; let addr = rtio::SocketAddr { ip: super::to_rtio(ip), port: port }; LocalIo::maybe_raise(|io| { - io.tcp_connect(addr, Some(timeout.num_milliseconds() as u64)).map(TcpStream::new) + io.tcp_connect(addr, timeout_ms).map(TcpStream::new) }).map_err(IoError::from_rtio_error) } diff --git a/src/libstd/io/net/unix.rs b/src/libstd/io/net/unix.rs index eb251075418..179855003f9 100644 --- a/src/libstd/io/net/unix.rs +++ b/src/libstd/io/net/unix.rs @@ -61,20 +61,26 @@ impl UnixStream { /// Connect to a pipe named by `path`, timing out if the specified number of /// milliseconds. /// - /// This function is similar to `connect`, except that if `timeout_ms` + /// This function is similar to `connect`, except that if `timeout` /// elapses the function will return an error of kind `TimedOut`. /// /// If a `timeout` with zero or negative duration is specified then /// the function returns `Err`, with the error kind set to `TimedOut`. + /// If the timeout is larger than 2^63 milliseconds, the function also + /// returns `Err` with the error kind set to `TimedOut`. #[experimental = "the timeout argument is likely to change types"] pub fn connect_timeout(path: &P, timeout: Duration) -> IoResult { if timeout <= Duration::milliseconds(0) { return Err(standard_error(TimedOut)); } + let timeout_ms = timeout.num_milliseconds().map(|x| { x as u64 }); + if timeout_ms.is_none() { + return Err(standard_error(TimedOut)); + } LocalIo::maybe_raise(|io| { - let s = io.unix_connect(&path.to_c_str(), Some(timeout.num_milliseconds() as u64)); + let s = io.unix_connect(&path.to_c_str(), timeout_ms); s.map(|p| UnixStream { obj: p }) }).map_err(IoError::from_rtio_error) } diff --git a/src/libstd/io/timer.rs b/src/libstd/io/timer.rs index 39c6c74e45e..205132aca1d 100644 --- a/src/libstd/io/timer.rs +++ b/src/libstd/io/timer.rs @@ -225,7 +225,7 @@ impl Callback for TimerCallback { } fn in_ms_u64(d: Duration) -> u64 { - let ms = d.num_milliseconds(); + let ms = d.num_milliseconds().unwrap_or(0); if ms < 0 { return 0 }; return ms as u64; } -- cgit 1.4.1-3-g733a5 From 26af5da6d4ca7ee3e7cec0333b2465a1c65e1171 Mon Sep 17 00:00:00 2001 From: Ruud van Asseldonk Date: Thu, 21 Aug 2014 09:37:30 +0200 Subject: libstd: Limit Duration range to i64 milliseconds. This enables `num_milliseconds` to return an `i64` again instead of `Option`, 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). --- src/libstd/io/net/tcp.rs | 8 +---- src/libstd/io/net/unix.rs | 8 +---- src/libstd/io/timer.rs | 2 +- src/libstd/time/duration.rs | 88 +++++++++++++++++++++++++-------------------- 4 files changed, 52 insertions(+), 54 deletions(-) (limited to 'src/libstd/io') diff --git a/src/libstd/io/net/tcp.rs b/src/libstd/io/net/tcp.rs index 52d3a04432a..8128c4466ce 100644 --- a/src/libstd/io/net/tcp.rs +++ b/src/libstd/io/net/tcp.rs @@ -105,23 +105,17 @@ impl TcpStream { /// /// If a `timeout` with zero or negative duration is specified then /// the function returns `Err`, with the error kind set to `TimedOut`. - /// If the timeout is larger than 2^63 milliseconds, the function also - /// returns `Err` with the error kind set to `TimedOut`. #[experimental = "the timeout argument may eventually change types"] pub fn connect_timeout(addr: SocketAddr, timeout: Duration) -> IoResult { if timeout <= Duration::milliseconds(0) { return Err(standard_error(TimedOut)); } - let timeout_ms = timeout.num_milliseconds().map(|x| { x as u64 }); - if timeout_ms.is_none() { - return Err(standard_error(TimedOut)); - } let SocketAddr { ip, port } = addr; let addr = rtio::SocketAddr { ip: super::to_rtio(ip), port: port }; LocalIo::maybe_raise(|io| { - io.tcp_connect(addr, timeout_ms).map(TcpStream::new) + io.tcp_connect(addr, Some(timeout.num_milliseconds() as u64)).map(TcpStream::new) }).map_err(IoError::from_rtio_error) } diff --git a/src/libstd/io/net/unix.rs b/src/libstd/io/net/unix.rs index 179855003f9..33235bd6b03 100644 --- a/src/libstd/io/net/unix.rs +++ b/src/libstd/io/net/unix.rs @@ -66,21 +66,15 @@ impl UnixStream { /// /// If a `timeout` with zero or negative duration is specified then /// the function returns `Err`, with the error kind set to `TimedOut`. - /// If the timeout is larger than 2^63 milliseconds, the function also - /// returns `Err` with the error kind set to `TimedOut`. #[experimental = "the timeout argument is likely to change types"] pub fn connect_timeout(path: &P, timeout: Duration) -> IoResult { if timeout <= Duration::milliseconds(0) { return Err(standard_error(TimedOut)); } - let timeout_ms = timeout.num_milliseconds().map(|x| { x as u64 }); - if timeout_ms.is_none() { - return Err(standard_error(TimedOut)); - } LocalIo::maybe_raise(|io| { - let s = io.unix_connect(&path.to_c_str(), timeout_ms); + let s = io.unix_connect(&path.to_c_str(), Some(timeout.num_milliseconds() as u64)); s.map(|p| UnixStream { obj: p }) }).map_err(IoError::from_rtio_error) } diff --git a/src/libstd/io/timer.rs b/src/libstd/io/timer.rs index 205132aca1d..39c6c74e45e 100644 --- a/src/libstd/io/timer.rs +++ b/src/libstd/io/timer.rs @@ -225,7 +225,7 @@ impl Callback for TimerCallback { } fn in_ms_u64(d: Duration) -> u64 { - let ms = d.num_milliseconds().unwrap_or(0); + let ms = d.num_milliseconds(); if ms < 0 { return 0 }; return ms as u64; } diff --git a/src/libstd/time/duration.rs b/src/libstd/time/duration.rs index c2030ccceb0..e8fefebc963 100644 --- a/src/libstd/time/duration.rs +++ b/src/libstd/time/duration.rs @@ -51,10 +51,17 @@ pub struct Duration { nanos: i32, // Always 0 <= nanos < NANOS_PER_SEC } -/// The minimum possible `Duration`. -pub static MIN: Duration = Duration { secs: i64::MIN, nanos: 0 }; -/// The maximum possible `Duration`. -pub static MAX: Duration = Duration { secs: i64::MAX, nanos: NANOS_PER_SEC - 1 }; +/// The minimum possible `Duration`: `i64::MIN` milliseconds. +pub static MIN: Duration = Duration { + secs: i64::MIN / MILLIS_PER_SEC - 1, + nanos: NANOS_PER_SEC + (i64::MIN % MILLIS_PER_SEC) as i32 * NANOS_PER_MILLI +}; + +/// The maximum possible `Duration`: `i64::MAX` milliseconds. +pub static MAX: Duration = Duration { + secs: i64::MAX / MILLIS_PER_SEC, + nanos: (i64::MAX % MILLIS_PER_SEC) as i32 * NANOS_PER_MILLI +}; impl Duration { /// Makes a new `Duration` with given number of weeks. @@ -94,9 +101,15 @@ impl Duration { } /// Makes a new `Duration` with given number of seconds. + /// Fails when the duration is more than `i64::MAX` milliseconds + /// or less than `i64::MIN` milliseconds. #[inline] pub fn seconds(seconds: i64) -> Duration { - Duration { secs: seconds, nanos: 0 } + let d = Duration { secs: seconds, nanos: 0 }; + if d < MIN || d > MAX { + fail!("Duration::seconds out of bounds"); + } + d } /// Makes a new `Duration` with given number of milliseconds. @@ -167,11 +180,12 @@ impl Duration { } /// Returns the total number of whole milliseconds in the duration, - /// or `None` on overflow (exceeding 2^63 milliseconds in either direction). - pub fn num_milliseconds(&self) -> Option { - let secs_part = try_opt!(self.num_seconds().checked_mul(&MILLIS_PER_SEC)); + pub fn num_milliseconds(&self) -> i64 { + // A proper Duration will not overflow, because MIN and MAX are defined + // such that the range is exactly i64 milliseconds. + let secs_part = self.num_seconds() * MILLIS_PER_SEC; let nanos_part = self.nanos_mod_sec() / NANOS_PER_MILLI; - secs_part.checked_add(&(nanos_part as i64)) + secs_part + nanos_part as i64 } /// Returns the total number of whole microseconds in the duration, @@ -211,13 +225,7 @@ impl num::Zero for Duration { impl Neg for Duration { #[inline] fn neg(&self) -> Duration { - if self.secs == i64::MIN && self.nanos == 0 { - // The minimum value cannot be negated due to overflow. Use the - // maximum value, which is one nanosecond less than the negated minimum. - MAX - } else if self.secs == i64::MIN { - Duration { secs: i64::MAX, nanos: NANOS_PER_SEC - self.nanos } - } else if self.nanos == 0 { + if self.nanos == 0 { Duration { secs: -self.secs, nanos: 0 } } else { Duration { secs: -self.secs - 1, nanos: NANOS_PER_SEC - self.nanos } @@ -245,7 +253,10 @@ impl num::CheckedAdd for Duration { nanos -= NANOS_PER_SEC; secs = try_opt!(secs.checked_add(&1)); } - Some(Duration { secs: secs, nanos: nanos }) + let d = Duration { secs: secs, nanos: nanos }; + // Even if d is within the bounds of i64 seconds, + // it might still overflow i64 milliseconds. + if d < MIN || d > MAX { None } else { Some(d) } } } @@ -269,7 +280,10 @@ impl num::CheckedSub for Duration { nanos += NANOS_PER_SEC; secs = try_opt!(secs.checked_sub(&1)); } - Some(Duration { secs: secs, nanos: nanos }) + let d = Duration { secs: secs, nanos: nanos }; + // Even if d is within the bounds of i64 seconds, + // it might still overflow i64 milliseconds. + if d < MIN || d > MAX { None } else { Some(d) } } } @@ -407,26 +421,22 @@ mod tests { assert_eq!(Duration::milliseconds(1001).num_seconds(), 1); assert_eq!(Duration::milliseconds(-999).num_seconds(), 0); assert_eq!(Duration::milliseconds(-1001).num_seconds(), -1); - assert_eq!(Duration::seconds(i64::MAX).num_seconds(), i64::MAX); - assert_eq!(Duration::seconds(i64::MIN).num_seconds(), i64::MIN); - assert_eq!(MAX.num_seconds(), i64::MAX); - assert_eq!(MIN.num_seconds(), i64::MIN); } #[test] fn test_duration_num_milliseconds() { let d: Duration = Zero::zero(); - assert_eq!(d.num_milliseconds(), Some(0)); - assert_eq!(Duration::milliseconds(1).num_milliseconds(), Some(1)); - assert_eq!(Duration::milliseconds(-1).num_milliseconds(), Some(-1)); - assert_eq!(Duration::microseconds(999).num_milliseconds(), Some(0)); - assert_eq!(Duration::microseconds(1001).num_milliseconds(), Some(1)); - assert_eq!(Duration::microseconds(-999).num_milliseconds(), Some(0)); - assert_eq!(Duration::microseconds(-1001).num_milliseconds(), Some(-1)); - assert_eq!(Duration::milliseconds(i64::MAX).num_milliseconds(), Some(i64::MAX)); - assert_eq!(Duration::milliseconds(i64::MIN).num_milliseconds(), Some(i64::MIN)); - assert_eq!(MAX.num_milliseconds(), None); - assert_eq!(MIN.num_milliseconds(), None); + assert_eq!(d.num_milliseconds(), 0); + assert_eq!(Duration::milliseconds(1).num_milliseconds(), 1); + assert_eq!(Duration::milliseconds(-1).num_milliseconds(), -1); + assert_eq!(Duration::microseconds(999).num_milliseconds(), 0); + assert_eq!(Duration::microseconds(1001).num_milliseconds(), 1); + assert_eq!(Duration::microseconds(-999).num_milliseconds(), 0); + assert_eq!(Duration::microseconds(-1001).num_milliseconds(), -1); + assert_eq!(Duration::milliseconds(i64::MAX).num_milliseconds(), i64::MAX); + assert_eq!(Duration::milliseconds(i64::MIN).num_milliseconds(), i64::MIN); + assert_eq!(MAX.num_milliseconds(), i64::MAX); + assert_eq!(MIN.num_milliseconds(), i64::MIN); } #[test] @@ -477,13 +487,13 @@ mod tests { #[test] fn test_duration_checked_ops() { - assert_eq!(Duration::seconds(i64::MAX).checked_add(&Duration::milliseconds(999)), - Some(Duration::seconds(i64::MAX - 1) + Duration::milliseconds(1999))); - assert!(Duration::seconds(i64::MAX).checked_add(&Duration::milliseconds(1000)).is_none()); + assert_eq!(Duration::milliseconds(i64::MAX - 1).checked_add(&Duration::microseconds(999)), + Some(Duration::milliseconds(i64::MAX - 2) + Duration::microseconds(1999))); + assert!(Duration::milliseconds(i64::MAX).checked_add(&Duration::microseconds(1000)).is_none()); - assert_eq!(Duration::seconds(i64::MIN).checked_sub(&Duration::seconds(0)), - Some(Duration::seconds(i64::MIN))); - assert!(Duration::seconds(i64::MIN).checked_sub(&Duration::seconds(1)).is_none()); + assert_eq!(Duration::milliseconds(i64::MIN).checked_sub(&Duration::milliseconds(0)), + Some(Duration::milliseconds(i64::MIN))); + assert!(Duration::milliseconds(i64::MIN).checked_sub(&Duration::milliseconds(1)).is_none()); } #[test] -- cgit 1.4.1-3-g733a5