From 129ae52c594de3b52b5ded652a99309543b9da62 Mon Sep 17 00:00:00 2001 From: Edgar Bonet Date: Thu, 10 Dec 2020 20:51:08 +0100 Subject: [PATCH 1/7] Test Stream::parseFloat() with many digits --- test/src/Stream/test_parseFloat.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/src/Stream/test_parseFloat.cpp b/test/src/Stream/test_parseFloat.cpp index 610f96a0..a750efc7 100644 --- a/test/src/Stream/test_parseFloat.cpp +++ b/test/src/Stream/test_parseFloat.cpp @@ -10,6 +10,8 @@ #include +#include + /************************************************************************************** * TEST CODE **************************************************************************************/ @@ -43,6 +45,11 @@ TEST_CASE ("Testing parseFloat(LookaheadMode lookahead = SKIP_ALL, char ignore = mock << "\r\n\t 12.34"; REQUIRE(mock.parseFloat() == 12.34f); } + WHEN ("A float is provided with too many digits") + { + mock << "3.1415926535897932384"; + REQUIRE(abs(mock.parseFloat() - 3.141592654f) < 8 * FLT_EPSILON); + } } TEST_CASE ("Testing parseFloat(LookaheadMode lookahead = SKIP_NONE, char ignore = NO_IGNORE_CHAR)", "[Stream-parseFloat-02]") From 8eb4013bb230670948ea015189c6775a51665d33 Mon Sep 17 00:00:00 2001 From: Edgar Bonet Date: Fri, 11 Dec 2020 10:14:17 +0100 Subject: [PATCH 2/7] Use Approx() macro for fuzzy comparison --- test/src/Stream/test_parseFloat.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/src/Stream/test_parseFloat.cpp b/test/src/Stream/test_parseFloat.cpp index a750efc7..64a49c98 100644 --- a/test/src/Stream/test_parseFloat.cpp +++ b/test/src/Stream/test_parseFloat.cpp @@ -48,7 +48,7 @@ TEST_CASE ("Testing parseFloat(LookaheadMode lookahead = SKIP_ALL, char ignore = WHEN ("A float is provided with too many digits") { mock << "3.1415926535897932384"; - REQUIRE(abs(mock.parseFloat() - 3.141592654f) < 8 * FLT_EPSILON); + REQUIRE(mock.parseFloat() == Approx(3.141592654f)); } } From 5445db39345f62acbdf51c3835ea9c0b512fb5dc Mon Sep 17 00:00:00 2001 From: Edgar Bonet Date: Fri, 11 Dec 2020 14:16:22 +0100 Subject: [PATCH 3/7] Test Stream::parseFloat() with a large number --- test/src/Stream/test_parseFloat.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/src/Stream/test_parseFloat.cpp b/test/src/Stream/test_parseFloat.cpp index 64a49c98..0317b99b 100644 --- a/test/src/Stream/test_parseFloat.cpp +++ b/test/src/Stream/test_parseFloat.cpp @@ -45,11 +45,16 @@ TEST_CASE ("Testing parseFloat(LookaheadMode lookahead = SKIP_ALL, char ignore = mock << "\r\n\t 12.34"; REQUIRE(mock.parseFloat() == 12.34f); } - WHEN ("A float is provided with too many digits") + WHEN ("A float is provided with too many digits after the decimal point") { mock << "3.1415926535897932384"; REQUIRE(mock.parseFloat() == Approx(3.141592654f)); } + WHEN ("A float is larger than LONG_MAX") + { + mock << "602200000000000000000000.00"; + REQUIRE(mock.parseFloat() == Approx(6.022e23f)); + } } TEST_CASE ("Testing parseFloat(LookaheadMode lookahead = SKIP_NONE, char ignore = NO_IGNORE_CHAR)", "[Stream-parseFloat-02]") From fae13e5f5324295118cf57104c69c3b087f7d64b Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Mon, 14 Dec 2020 10:11:31 +0100 Subject: [PATCH 4/7] Exchanging the type of 'value' from 'long' to 'double' prevents overflow when parsing float values. However, we still need to ensure against too large values contained in streams. This should be possible because the maximum length of a float value pre-comma is known to be 38 digits (FLT_MAX_10_EXP). --- api/Stream.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/api/Stream.cpp b/api/Stream.cpp index a5d8b076..6eb1bdf2 100644 --- a/api/Stream.cpp +++ b/api/Stream.cpp @@ -24,6 +24,7 @@ #include "Common.h" #include "Stream.h" +#include #define PARSE_TIMEOUT 1000 // default number of milli-seconds to wait @@ -162,9 +163,9 @@ float Stream::parseFloat(LookaheadMode lookahead, char ignore) { bool isNegative = false; bool isFraction = false; - long value = 0; + double value = 0.0; int c; - float fraction = 1.0; + unsigned int digits_post_comma = 0; c = peekNextDigit(lookahead, true); // ignore non numeric leading characters @@ -181,7 +182,7 @@ float Stream::parseFloat(LookaheadMode lookahead, char ignore) else if(c >= '0' && c <= '9') { // is c a digit? value = value * 10 + c - '0'; if(isFraction) - fraction *= 0.1; + digits_post_comma++; } read(); // consume the character we got with peek c = timedPeek(); @@ -190,10 +191,11 @@ float Stream::parseFloat(LookaheadMode lookahead, char ignore) if(isNegative) value = -value; + if(isFraction) - return value * fraction; - else - return value; + value /= pow(10, digits_post_comma); + + return value; } // read characters from stream into buffer From 1266b08d30bc4cd71952504dd9830fedd40ac68f Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Mon, 14 Dec 2020 10:23:58 +0100 Subject: [PATCH 5/7] Replacing computational expensive pow call with result of accumulated multiplication. --- api/Stream.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/api/Stream.cpp b/api/Stream.cpp index 6eb1bdf2..506e2a7f 100644 --- a/api/Stream.cpp +++ b/api/Stream.cpp @@ -24,7 +24,6 @@ #include "Common.h" #include "Stream.h" -#include #define PARSE_TIMEOUT 1000 // default number of milli-seconds to wait @@ -165,7 +164,7 @@ float Stream::parseFloat(LookaheadMode lookahead, char ignore) bool isFraction = false; double value = 0.0; int c; - unsigned int digits_post_comma = 0; + double fraction = 1.0; c = peekNextDigit(lookahead, true); // ignore non numeric leading characters @@ -182,7 +181,7 @@ float Stream::parseFloat(LookaheadMode lookahead, char ignore) else if(c >= '0' && c <= '9') { // is c a digit? value = value * 10 + c - '0'; if(isFraction) - digits_post_comma++; + fraction *= 0.1; } read(); // consume the character we got with peek c = timedPeek(); @@ -193,7 +192,7 @@ float Stream::parseFloat(LookaheadMode lookahead, char ignore) value = -value; if(isFraction) - value /= pow(10, digits_post_comma); + value *= fraction; return value; } From 91c5e2985335c6cf88c5977cbbc311c292bbf7b8 Mon Sep 17 00:00:00 2001 From: Edgar Bonet Date: Mon, 25 Jan 2021 13:03:20 +0100 Subject: [PATCH 6/7] Test Stream::parseFloat() with a huge number of digits Give a 311-digit number to Stream::parseFloat(). This makes the local variable `value' overflow to infinity. With so many digits, the number cannot be parsed into an integer, not even into an integer stored as a `double'. Note that 40 digits would be enough to unveil this issue on AVR. --- test/src/Stream/test_parseFloat.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/src/Stream/test_parseFloat.cpp b/test/src/Stream/test_parseFloat.cpp index 0317b99b..19d2ce86 100644 --- a/test/src/Stream/test_parseFloat.cpp +++ b/test/src/Stream/test_parseFloat.cpp @@ -47,7 +47,7 @@ TEST_CASE ("Testing parseFloat(LookaheadMode lookahead = SKIP_ALL, char ignore = } WHEN ("A float is provided with too many digits after the decimal point") { - mock << "3.1415926535897932384"; + mock << "3.1415926535897932384626433832795028841971693993751058209749445923078164062862089986280348253421170679821480865132823066470938446095505822317253594081284811174502841027019385211055596446229489549303819644288109756659334461284756482337867831652712019091456485669234603486104543266482133936072602491412737245870064"; REQUIRE(mock.parseFloat() == Approx(3.141592654f)); } WHEN ("A float is larger than LONG_MAX") From 61975114e344a74bba4aa643fd6dd246175fde98 Mon Sep 17 00:00:00 2001 From: Edgar Bonet Date: Mon, 25 Jan 2021 13:06:35 +0100 Subject: [PATCH 7/7] Avoid overflowing parseFloat()'s internal 'value' If more than 309 digits are provided to Stream::parseFloat() (more than 39 on AVR), the internal variable 'value' would overflow to infinity. We avoid this by not storing the parsed number as an integer-in-a-float. --- api/Stream.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/api/Stream.cpp b/api/Stream.cpp index 506e2a7f..f6f9bda6 100644 --- a/api/Stream.cpp +++ b/api/Stream.cpp @@ -179,9 +179,12 @@ float Stream::parseFloat(LookaheadMode lookahead, char ignore) else if (c == '.') isFraction = true; else if(c >= '0' && c <= '9') { // is c a digit? - value = value * 10 + c - '0'; - if(isFraction) - fraction *= 0.1; + if(isFraction) { + fraction *= 0.1; + value = value + fraction * (c - '0'); + } else { + value = value * 10 + c - '0'; + } } read(); // consume the character we got with peek c = timedPeek(); @@ -191,9 +194,6 @@ float Stream::parseFloat(LookaheadMode lookahead, char ignore) if(isNegative) value = -value; - if(isFraction) - value *= fraction; - return value; }