aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMarcoFalke <falke.marco@gmail.com>2021-10-04 15:05:55 +0200
committerMarcoFalke <falke.marco@gmail.com>2021-10-04 15:06:37 +0200
commit42fedb4acd3cfa813059fcc3f96b2a41f78d9074 (patch)
tree9fe2a104daa2bc50d0b7d1d3b2c56edb5a2de779
parentcdb4dfcbf1c82ef4c4eb7bb982b35219d5dbf827 (diff)
parentfa9d72a7947d2cff541794e21e0040c3c1d43b32 (diff)
Merge bitcoin/bitcoin#23156: refactor: Remove unused ParsePrechecks and ParseDouble
fa9d72a7947d2cff541794e21e0040c3c1d43b32 Remove unused ParseDouble and ParsePrechecks (MarcoFalke) fa3cd2853530c86c261ac7266ffe4f1726fe9ce6 refactor: Remove unused ParsePrechecks from ParseIntegral (MarcoFalke) Pull request description: All of the `ParsePrechecks` are already done by `ToIntegral`, so remove them from `ParseIntegral`. Also: * Remove redundant `{}`. See https://github.com/bitcoin/bitcoin/pull/20457#discussion_r720116866 * Add missing failing c-string test case * Add missing failing test cases for non-int32_t integral types ACKs for top commit: laanwj: Code review ACK fa9d72a7947d2cff541794e21e0040c3c1d43b32, good find on ParseDouble not being used at all, and testing for behavior of embedded NULL characters is always a good thing. practicalswift: cr ACK fa9d72a7947d2cff541794e21e0040c3c1d43b32 Tree-SHA512: 3d654dcaebbf312dd57e54241f9aa6d35b1d1d213c37e4c6b8b9a69bcbe8267a397474a8b86b57740fbdd8e3d03b4cdb6a189a9eb8e05cd38035dab195410aa7
-rw-r--r--src/test/fuzz/parse_numbers.cpp3
-rw-r--r--src/test/util_tests.cpp84
-rw-r--r--src/util/strencodings.cpp30
-rw-r--r--src/util/strencodings.h13
4 files changed, 41 insertions, 89 deletions
diff --git a/src/test/fuzz/parse_numbers.cpp b/src/test/fuzz/parse_numbers.cpp
index 6a302e1e06..85fee062f0 100644
--- a/src/test/fuzz/parse_numbers.cpp
+++ b/src/test/fuzz/parse_numbers.cpp
@@ -14,9 +14,6 @@ FUZZ_TARGET(parse_numbers)
(void)ParseMoney(random_string);
- double d;
- (void)ParseDouble(random_string, &d);
-
uint8_t u8;
(void)ParseUInt8(random_string, &u8);
diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
index 5d1ad2ebf6..51707310a2 100644
--- a/src/test/util_tests.cpp
+++ b/src/test/util_tests.cpp
@@ -1474,6 +1474,35 @@ BOOST_AUTO_TEST_CASE(test_ParseInt32)
BOOST_CHECK(!ParseInt32("32482348723847471234", nullptr));
}
+template <typename T>
+static void RunToIntegralTests()
+{
+ BOOST_CHECK(!ToIntegral<T>(STRING_WITH_EMBEDDED_NULL_CHAR));
+ BOOST_CHECK(!ToIntegral<T>(" 1"));
+ BOOST_CHECK(!ToIntegral<T>("1 "));
+ BOOST_CHECK(!ToIntegral<T>("1a"));
+ BOOST_CHECK(!ToIntegral<T>("1.1"));
+ BOOST_CHECK(!ToIntegral<T>("1.9"));
+ BOOST_CHECK(!ToIntegral<T>("+01.9"));
+ BOOST_CHECK(!ToIntegral<T>("-"));
+ BOOST_CHECK(!ToIntegral<T>("+"));
+ BOOST_CHECK(!ToIntegral<T>(" -1"));
+ BOOST_CHECK(!ToIntegral<T>("-1 "));
+ BOOST_CHECK(!ToIntegral<T>(" -1 "));
+ BOOST_CHECK(!ToIntegral<T>("+1"));
+ BOOST_CHECK(!ToIntegral<T>(" +1"));
+ BOOST_CHECK(!ToIntegral<T>(" +1 "));
+ BOOST_CHECK(!ToIntegral<T>("+-1"));
+ BOOST_CHECK(!ToIntegral<T>("-+1"));
+ BOOST_CHECK(!ToIntegral<T>("++1"));
+ BOOST_CHECK(!ToIntegral<T>("--1"));
+ BOOST_CHECK(!ToIntegral<T>(""));
+ BOOST_CHECK(!ToIntegral<T>("aap"));
+ BOOST_CHECK(!ToIntegral<T>("0x1"));
+ BOOST_CHECK(!ToIntegral<T>("-32482348723847471234"));
+ BOOST_CHECK(!ToIntegral<T>("32482348723847471234"));
+}
+
BOOST_AUTO_TEST_CASE(test_ToIntegral)
{
BOOST_CHECK_EQUAL(ToIntegral<int32_t>("1234").value(), 1'234);
@@ -1486,27 +1515,14 @@ BOOST_AUTO_TEST_CASE(test_ToIntegral)
BOOST_CHECK_EQUAL(ToIntegral<int32_t>("-1234").value(), -1'234);
BOOST_CHECK_EQUAL(ToIntegral<int32_t>("-1").value(), -1);
- BOOST_CHECK(!ToIntegral<int32_t>(" 1"));
- BOOST_CHECK(!ToIntegral<int32_t>("1 "));
- BOOST_CHECK(!ToIntegral<int32_t>("1a"));
- BOOST_CHECK(!ToIntegral<int32_t>("1.1"));
- BOOST_CHECK(!ToIntegral<int32_t>("1.9"));
- BOOST_CHECK(!ToIntegral<int32_t>("+01.9"));
- BOOST_CHECK(!ToIntegral<int32_t>(" -1"));
- BOOST_CHECK(!ToIntegral<int32_t>("-1 "));
- BOOST_CHECK(!ToIntegral<int32_t>(" -1 "));
- BOOST_CHECK(!ToIntegral<int32_t>("+1"));
- BOOST_CHECK(!ToIntegral<int32_t>(" +1"));
- BOOST_CHECK(!ToIntegral<int32_t>(" +1 "));
- BOOST_CHECK(!ToIntegral<int32_t>("+-1"));
- BOOST_CHECK(!ToIntegral<int32_t>("-+1"));
- BOOST_CHECK(!ToIntegral<int32_t>("++1"));
- BOOST_CHECK(!ToIntegral<int32_t>("--1"));
- BOOST_CHECK(!ToIntegral<int32_t>(""));
- BOOST_CHECK(!ToIntegral<int32_t>("aap"));
- BOOST_CHECK(!ToIntegral<int32_t>("0x1"));
- BOOST_CHECK(!ToIntegral<int32_t>("-32482348723847471234"));
- BOOST_CHECK(!ToIntegral<int32_t>("32482348723847471234"));
+ RunToIntegralTests<uint64_t>();
+ RunToIntegralTests<int64_t>();
+ RunToIntegralTests<uint32_t>();
+ RunToIntegralTests<int32_t>();
+ RunToIntegralTests<uint16_t>();
+ RunToIntegralTests<int16_t>();
+ RunToIntegralTests<uint8_t>();
+ RunToIntegralTests<int8_t>();
BOOST_CHECK(!ToIntegral<int64_t>("-9223372036854775809"));
BOOST_CHECK_EQUAL(ToIntegral<int64_t>("-9223372036854775808").value(), -9'223'372'036'854'775'807LL - 1LL);
@@ -1785,32 +1801,6 @@ BOOST_AUTO_TEST_CASE(test_ParseUInt64)
BOOST_CHECK(!ParseUInt64("-1234", &n));
}
-BOOST_AUTO_TEST_CASE(test_ParseDouble)
-{
- double n;
- // Valid values
- BOOST_CHECK(ParseDouble("1234", nullptr));
- BOOST_CHECK(ParseDouble("0", &n) && n == 0.0);
- BOOST_CHECK(ParseDouble("1234", &n) && n == 1234.0);
- BOOST_CHECK(ParseDouble("01234", &n) && n == 1234.0); // no octal
- BOOST_CHECK(ParseDouble("2147483647", &n) && n == 2147483647.0);
- BOOST_CHECK(ParseDouble("-2147483648", &n) && n == -2147483648.0);
- BOOST_CHECK(ParseDouble("-1234", &n) && n == -1234.0);
- BOOST_CHECK(ParseDouble("1e6", &n) && n == 1e6);
- BOOST_CHECK(ParseDouble("-1e6", &n) && n == -1e6);
- // Invalid values
- BOOST_CHECK(!ParseDouble("", &n));
- BOOST_CHECK(!ParseDouble(" 1", &n)); // no padding inside
- BOOST_CHECK(!ParseDouble("1 ", &n));
- BOOST_CHECK(!ParseDouble("1a", &n));
- BOOST_CHECK(!ParseDouble("aap", &n));
- BOOST_CHECK(!ParseDouble("0x1", &n)); // no hex
- BOOST_CHECK(!ParseDouble(STRING_WITH_EMBEDDED_NULL_CHAR, &n));
- // Overflow and underflow
- BOOST_CHECK(!ParseDouble("-1e10000", nullptr));
- BOOST_CHECK(!ParseDouble("1e10000", nullptr));
-}
-
BOOST_AUTO_TEST_CASE(test_FormatParagraph)
{
BOOST_CHECK_EQUAL(FormatParagraph("", 79, 0), "");
diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp
index 88fec6740a..15bd07b374 100644
--- a/src/util/strencodings.cpp
+++ b/src/util/strencodings.cpp
@@ -281,16 +281,11 @@ std::string DecodeBase32(const std::string& str, bool* pf_invalid)
return std::string((const char*)vchRet.data(), vchRet.size());
}
-[[nodiscard]] static bool ParsePrechecks(const std::string&);
-
namespace {
template <typename T>
bool ParseIntegral(const std::string& str, T* out)
{
static_assert(std::is_integral<T>::value);
- if (!ParsePrechecks(str)) {
- return false;
- }
// Replicate the exact behavior of strtol/strtoll/strtoul/strtoull when
// handling leading +/- for backwards compatibility.
if (str.length() >= 2 && str[0] == '+' && str[1] == '-') {
@@ -307,17 +302,6 @@ bool ParseIntegral(const std::string& str, T* out)
}
}; // namespace
-[[nodiscard]] static bool ParsePrechecks(const std::string& str)
-{
- if (str.empty()) // No empty string allowed
- return false;
- if (str.size() >= 1 && (IsSpace(str[0]) || IsSpace(str[str.size()-1]))) // No padding allowed
- return false;
- if (!ValidAsCString(str)) // No embedded NUL characters allowed
- return false;
- return true;
-}
-
bool ParseInt32(const std::string& str, int32_t* out)
{
return ParseIntegral<int32_t>(str, out);
@@ -348,20 +332,6 @@ bool ParseUInt64(const std::string& str, uint64_t* out)
return ParseIntegral<uint64_t>(str, out);
}
-bool ParseDouble(const std::string& str, double *out)
-{
- if (!ParsePrechecks(str))
- return false;
- if (str.size() >= 2 && str[0] == '0' && str[1] == 'x') // No hexadecimal floats allowed
- return false;
- std::istringstream text(str);
- text.imbue(std::locale::classic());
- double result;
- text >> result;
- if(out) *out = result;
- return text.eof() && !text.fail();
-}
-
std::string FormatParagraph(const std::string& in, size_t width, size_t indent)
{
std::stringstream out;
diff --git a/src/util/strencodings.h b/src/util/strencodings.h
index 166352c42f..1f7762aeef 100644
--- a/src/util/strencodings.h
+++ b/src/util/strencodings.h
@@ -123,7 +123,9 @@ constexpr inline bool IsSpace(char c) noexcept {
}
/**
- * Convert string to integral type T.
+ * Convert string to integral type T. Leading whitespace, a leading +, or any
+ * trailing character fail the parsing. The required format expressed as regex
+ * is `-?[0-9]+`.
*
* @returns std::nullopt if the entire string could not be parsed, or if the
* parsed value is not in the range representable by the type T.
@@ -137,7 +139,7 @@ std::optional<T> ToIntegral(const std::string& str)
if (first_nonmatching != str.data() + str.size() || error_condition != std::errc{}) {
return std::nullopt;
}
- return {result};
+ return result;
}
/**
@@ -183,13 +185,6 @@ std::optional<T> ToIntegral(const std::string& str)
[[nodiscard]] bool ParseUInt64(const std::string& str, uint64_t *out);
/**
- * Convert string to double with strict parse error feedback.
- * @returns true if the entire string could be parsed as valid double,
- * false if not the entire string could be parsed or when overflow or underflow occurred.
- */
-[[nodiscard]] bool ParseDouble(const std::string& str, double *out);
-
-/**
* Convert a span of bytes to a lower-case hexadecimal string.
*/
std::string HexStr(const Span<const uint8_t> s);