fix: avoid no_std panic in ceil helpers on non-finite input
Byte::from_f64/from_f32 let +inf past the size >= 0.0 guard and then called the no_std ceil helpers, which used Decimal::from_f64(v).unwrap(). Decimal conversion returns None for non-finite values, so this panicked in no_std builds while std returned None. Fall back to the input value for non-finite input so no_std matches std and the documented None contract is upheld.
This commit is contained in:
1 parent
85f79fa753
commit
bf130b54f8
3 files changed
+109
-2
No files matched your search
@@ -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.
|
||||||
+14
-2
@@ -36,7 +36,13 @@ pub(crate) fn ceil_f64(v: f64) -> f64 {
|
|||||||
pub(crate) fn ceil_f64(v: f64) -> f64 {
|
pub(crate) fn ceil_f64(v: f64) -> f64 {
|
||||||
debug_assert!(v >= 0.0);
|
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"))]
|
#[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 {
|
pub(crate) fn ceil_f32(v: f32) -> f32 {
|
||||||
debug_assert!(v >= 0.0);
|
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"))]
|
#[cfg(any(feature = "byte", feature = "bit"))]
|
||||||
|
|||||||
@@ -170,3 +170,13 @@ fn tests() {
|
|||||||
assert_eq!(byte, serde_json::from_str::<Byte>(case.0).unwrap(), "{i}");
|
assert_eq!(byte, serde_json::from_str::<Byte>(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);
|
||||||
|
}
|
||||||
Reference in new issue
Block a user