rust-coarsetime icon indicating copy to clipboard operation
rust-coarsetime copied to clipboard

Overflow possibilities in helpers.rs

Open ijackson opened this issue 1 year ago • 1 comments
trafficstars

I was looking at the code in helpers.rs with a view to excluding panics (see #20, also #29).

I noticed that these functions all use << to shift some number of seconds into the top half of the u64. However, if the number of seconds is too large, this will simply discard the top bits.

I think this is all correct for Instant unless the whole program runs for more than 2^32 seconds, so that's fine.

But it's wrong for Duration. For example, currently, Duration::from_secs(4294967296) is zero. (And the conversion from std::time::Duration is similarly troubled.)

Based on your opinion on #29, I guess you think that Duration::from_secs(4294967296) and Duration::from(std::time::Duration::from_secs()) should both panic? Then, presumably, there should be Duration::from_std_saturating?

I think conversions from coarsetime::Duration to std::time::Duration are correct and infallible but I'm not 100% sure. Perhaps there should be some test cases for that.

Please let me know what you think.

ijackson avatar Mar 21 '24 17:03 ijackson

(see also #34)

ijackson avatar Mar 21 '24 17:03 ijackson