From 129ae52c594de3b52b5ded652a99309543b9da62 Mon Sep 17 00:00:00 2001
From: Edgar Bonet <linux@edgar-bonet.org>
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 <StreamMock.h>
+#include <float.h>
+
/**************************************************************************************
* 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 <linux@edgar-bonet.org>
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 <linux@edgar-bonet.org>
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 <cto@lxrobotics.com>
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 <math.h>
#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 <cto@lxrobotics.com>
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 <math.h>
#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 <linux@edgar-bonet.org>
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 <linux@edgar-bonet.org>
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;
}