From: Douglas Bagnall Date: Wed, 16 Jul 2014 10:19:11 +0000 (+1200) Subject: opt/helpers: fix out-of-range check in opt_set_floatval() X-Git-Url: https://git.ozlabs.org/?p=ccan;a=commitdiff_plain;h=106eab33eac437bdde11280e5f05f194a8fdb8c8 opt/helpers: fix out-of-range check in opt_set_floatval() 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 --- diff --git a/ccan/opt/helpers.c b/ccan/opt/helpers.c index 43b86d7c..f247301c 100644 --- a/ccan/opt/helpers.c +++ b/ccan/opt/helpers.c @@ -9,6 +9,7 @@ #include #include #include "private.h" +#include /* Upper bound to sprintf this simple type? Each 3 bits < 1 digit. */ #define CHAR_SIZE(type) (((sizeof(type)*CHAR_BIT + 2) / 3) + 1) @@ -126,8 +127,14 @@ char *opt_set_floatval(const char *arg, float *f) return err; *f = d; - if (*f != d) - return arg_bad("'%s' is out of range", arg); + + /*allow true infinity via --foo=INF, while avoiding isinf() from math.h + because it wasn't standard 25 years ago.*/ + double inf = 1e300 * 1e300; /*direct 1e600 annoys -Woverflow*/ + if ((d > FLT_MAX || d < -FLT_MAX) && d != inf && d != -inf) + return arg_bad("'%s' is out of range for a 32 bit float", arg); + if (d != 0 && *f == 0) + return arg_bad("'%s' is out of range (truncated to zero)", arg); return NULL; } diff --git a/ccan/opt/test/run-helpers.c b/ccan/opt/test/run-helpers.c index 49fb2062..6ec17f58 100644 --- a/ccan/opt/test/run-helpers.c +++ b/ccan/opt/test/run-helpers.c @@ -5,6 +5,7 @@ #include #include #include "utils.h" +#include /* We don't actually want it to exit... */ static jmp_buf exited; @@ -77,7 +78,7 @@ static void set_args(int *argc, char ***argv, ...) /* Test helpers. */ int main(int argc, char *argv[]) { - plan_tests(476); + plan_tests(500); /* opt_set_bool */ { @@ -215,9 +216,25 @@ int main(int argc, char *argv[]) ok1(arg == 9999); ok1(parse_args(&argc, &argv, "-a", "-9999", NULL)); ok1(arg == -9999); + ok1(parse_args(&argc, &argv, "-a", "1e33", NULL)); + ok1(arg == 1e33f); + /*overflows should fail */ + ok1(!parse_args(&argc, &argv, "-a", "1e39", NULL)); + ok1(!parse_args(&argc, &argv, "-a", "-1e40", NULL)); + /*low numbers lose precision but work */ + ok1(parse_args(&argc, &argv, "-a", "1e-39", NULL)); + ok1(arg == 1e-39f); + ok1(parse_args(&argc, &argv, "-a", "-1e-45", NULL)); + ok1(arg == -1e-45f); + ok1(!parse_args(&argc, &argv, "-a", "1e-99", NULL)); ok1(parse_args(&argc, &argv, "-a", "0", NULL)); ok1(arg == 0); + ok1(parse_args(&argc, &argv, "-a", "1.111111111111", NULL)); + ok1(arg == 1.1111112f); + ok1(parse_args(&argc, &argv, "-a", "INF", NULL)); + ok1(isinf(arg)); ok1(!parse_args(&argc, &argv, "-a", "100crap", NULL)); + ok1(!parse_args(&argc, &argv, "-a", "1e7crap", NULL)); } /* opt_set_doubleval */ { @@ -228,9 +245,19 @@ int main(int argc, char *argv[]) ok1(arg == 9999); ok1(parse_args(&argc, &argv, "-a", "-9999", NULL)); ok1(arg == -9999); + ok1(parse_args(&argc, &argv, "-a", "1e-299", NULL)); + ok1(arg == 1e-299); + ok1(parse_args(&argc, &argv, "-a", "-1e-305", NULL)); + ok1(arg == -1e-305); + ok1(!parse_args(&argc, &argv, "-a", "1e-499", NULL)); ok1(parse_args(&argc, &argv, "-a", "0", NULL)); ok1(arg == 0); + ok1(parse_args(&argc, &argv, "-a", "1.1111111111111111111", NULL)); + ok1(arg == 1.1111111111111112); + ok1(parse_args(&argc, &argv, "-a", "INF", NULL)); + ok1(isinf(arg)); ok1(!parse_args(&argc, &argv, "-a", "100crap", NULL)); + ok1(!parse_args(&argc, &argv, "-a", "1e7crap", NULL)); } {