diff --git a/PR.md b/PR.md new file mode 100644 index 0000000..e746795 --- /dev/null +++ b/PR.md @@ -0,0 +1,85 @@ +# Fix `no_std` panic in `Byte::from_f64`/`from_f32` on non-finite input + +## Problem + +`Byte::from_f64` and `Byte::from_f32` are documented to return `None` when the +input is "too large" or not `>= 0`. In `std` builds a non-finite input such as +`f64::INFINITY` correctly returns `None`, but in `no_std` builds it panics: + +```rust +// with `--no-default-features --features byte` +byte_unit::Byte::from_f64(f64::INFINITY); +// thread panicked at src/common.rs:39: +// called `Option::unwrap()` on a `None` value +``` + +So the same call behaves differently across the two build configurations and +breaks the documented contract in `no_std`. + +## Root cause + +`from_f64`/`from_f32` guard the input with `if size >= 0.0`, which rejects +`NaN` (since `NaN >= 0.0` is `false`) but lets `+inf` through. The value is then +passed to `ceil_f64`/`ceil_f32` in `src/common.rs`. + +The `std` implementation uses `v.ceil()`, which returns `inf`; the subsequent +`inf as u128`/`as u64` cast saturates to the maximum, and the range check then +returns `None`. + +The `no_std` implementation instead does: + +```rust +Decimal::from_f64(v).unwrap().ceil().to_f64().unwrap() +``` + +`Decimal::from_f64` (and `from_f32`) returns `None` for non-finite values, so +`.unwrap()` panics. The `no_std` helper therefore diverges from the `std` one. + +## Fix + +Make the `no_std` `ceil_f64`/`ceil_f32` fall back to the input value when it is +not representable as a `Decimal` (i.e. non-finite), mirroring `f64::ceil`: + +```rust +match Decimal::from_f64(v) { + Some(d) => d.ceil().to_f64().unwrap_or(v), + None => v, +} +``` + +With this, `from_f64(f64::INFINITY)` flows through the same saturating cast and +range check as in `std`, returning `None`. Finite values are unaffected. + +## Test + +Added `from_non_finite_returns_none` to `tests/byte.rs`, asserting that +`from_f64`/`from_f32` return `None` for both infinity and NaN. It passes under +`--all-features` and under `--no-default-features --features byte` (the config +that previously panicked). + +## Gate + +- `cargo test --all-features` -> ok (exit 0) +- `cargo test --no-default-features --features byte,bit` -> ok (exit 0) +- `cargo build --all-features` -> ok (exit 0) +- `cargo clippy --all-targets --all-features -- -D warnings` -> ok (exit 0) +- `cargo clippy --no-default-features --features byte,bit -- -D warnings` -> ok (exit 0) + +## Counterfactual + +Restoring `Decimal::from_f64(v).unwrap()...` makes the new test panic under the +`no_std` config: + +``` +thread 'from_non_finite_returns_none' panicked at src/common.rs:39:26: +called `Option::unwrap()` on a `None` value +``` + +## Flags for the account owner + +`cargo fmt --check` reports differences, but this is pre-existing and unrelated +to this change: `rustfmt.toml` enables nightly-only options (e.g. +`binop_separator`, `imports_granularity`), so a stable `rustfmt` reformats many +untouched files. On an unmodified checkout `cargo fmt --check` already reports +the same differences. The two files changed here were edited by hand to match +the surrounding style. diff --git a/src/common.rs b/src/common.rs index ee94712..b05c028 100644 --- a/src/common.rs +++ b/src/common.rs @@ -36,7 +36,13 @@ pub(crate) fn ceil_f64(v: f64) -> f64 { pub(crate) fn ceil_f64(v: f64) -> f64 { debug_assert!(v >= 0.0); - Decimal::from_f64(v).unwrap().ceil().to_f64().unwrap() + // `Decimal::from_f64` returns `None` for non-finite values (e.g. infinity), + // so fall back to the input to mirror `f64::ceil` in the `std` path and + // avoid panicking. + match Decimal::from_f64(v) { + Some(d) => d.ceil().to_f64().unwrap_or(v), + None => v, + } } #[cfg(any(feature = "byte", feature = "bit"))] @@ -54,7 +60,13 @@ pub(crate) fn ceil_f32(v: f32) -> f32 { pub(crate) fn ceil_f32(v: f32) -> f32 { debug_assert!(v >= 0.0); - Decimal::from_f32(v).unwrap().ceil().to_f32().unwrap() + // `Decimal::from_f32` returns `None` for non-finite values (e.g. infinity), + // so fall back to the input to mirror `f32::ceil` in the `std` path and + // avoid panicking. + match Decimal::from_f32(v) { + Some(d) => d.ceil().to_f32().unwrap_or(v), + None => v, + } } #[cfg(any(feature = "byte", feature = "bit"))] diff --git a/tests/byte.rs b/tests/byte.rs index 8687ce1..c1429b4 100644 --- a/tests/byte.rs +++ b/tests/byte.rs @@ -170,3 +170,13 @@ fn tests() { assert_eq!(byte, serde_json::from_str::(case.0).unwrap(), "{i}"); } } + +#[test] +fn from_non_finite_returns_none() { + // Non-finite inputs are "too large" and must return None instead of + // panicking, consistently across the `std` and `no_std` builds. + assert_eq!(Byte::from_f64(f64::INFINITY), None); + assert_eq!(Byte::from_f64(f64::NAN), None); + assert_eq!(Byte::from_f32(f32::INFINITY), None); + assert_eq!(Byte::from_f32(f32::NAN), None); +}