dualfroz
bf130b54f8
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.
86 lines
2.9 KiB
Markdown
86 lines
2.9 KiB
Markdown
# 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.
|