[ccan] [PATCH] opt/helpers: fix out-of-range check in opt_set_floatval()

[Rusty Russell]

Douglas Bagnall <douglas at halo.gen.nz> writes:
> opt_set_floatval() uses opt_set_doubleval() to do initial conversion
> and bounds checking before casting the result to float.  Previously
> the out of bounds check compared the original and float values for
> equality and declared anything unequal to be out of bounds. While this
> trick works well in the orderly integer world, it can backfire with
> floating point. For example, 3.1 resolves to the double more precisely
> known as 3.100000000000000088817841970012523233890533447265625, while
> 3.1f resolves to 3.099999904632568359375.  These are not equal, though
> 3.1 is generally regarded as being in bounds for a float.  There are
> around 8 billion other doubles (i.e. 3.1 +/- a little bit) that map to
> the same 3.1f value, of which only one is strictly equal to it.
>
> Why wasn't this discovered by the tests? It turns out that neither
> set_floatval nor set_doubleval were tested against non-integral
> numbers.  This is slightly improved here.
>
> This patch uses the arguably more reasonable definition of bounds that
> is found in opt_set_doubleval(): it excludes numbers that would get
> rounded to zero or an infinity. One subtlety is that the double
> version allows `--foo=INF` for an explicit infinity without overflow.
> This is possibly useful, and there is some fiddling to allow this for
> floats. Likewise an explicit zero is allowed, as you would expect.
>
> It is perhaps worth noting that the `*f = d` cast/assignment at the
> heart of it all can crash and core dump on overflow or underflow if
> the floating point exception flags are in an unexpected state.
>
> Signed-off-by: Douglas Bagnall <douglas at halo.gen.nz>

Much nicer, thanks.

I've never used the floating point arg versions, so I shouldn't have
written them in the first place.

Applied!
Rusty.
PS. Am thinking of relicensing ccan/opt under LGPLv2+, would that be an issue?