Skip to content

Commit 42fedb4

Browse files
author
MarcoFalke
committed
Merge bitcoin/bitcoin#23156: refactor: Remove unused ParsePrechecks and ParseDouble
fa9d72a Remove unused ParseDouble and ParsePrechecks (MarcoFalke) fa3cd28 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 bitcoin/bitcoin#20457 (comment) * 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 fa9d72a, 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 fa9d72a Tree-SHA512: 3d654dcaebbf312dd57e54241f9aa6d35b1d1d213c37e4c6b8b9a69bcbe8267a397474a8b86b57740fbdd8e3d03b4cdb6a189a9eb8e05cd38035dab195410aa7
2 parents cdb4dfc + fa9d72a commit 42fedb4

File tree

4 files changed

+41
-89
lines changed

4 files changed

+41
-89
lines changed

‎src/test/fuzz/parse_numbers.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,6 @@ FUZZ_TARGET(parse_numbers)
1414

1515
(void)ParseMoney(random_string);
1616

17-
double d;
18-
(void)ParseDouble(random_string, &d);
19-
2017
uint8_t u8;
2118
(void)ParseUInt8(random_string, &u8);
2219

‎src/test/util_tests.cpp

Lines changed: 37 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1474,6 +1474,35 @@ BOOST_AUTO_TEST_CASE(test_ParseInt32)
14741474
BOOST_CHECK(!ParseInt32("32482348723847471234", nullptr));
14751475
}
14761476

1477+
template <typename T>
1478+
static void RunToIntegralTests()
1479+
{
1480+
BOOST_CHECK(!ToIntegral<T>(STRING_WITH_EMBEDDED_NULL_CHAR));
1481+
BOOST_CHECK(!ToIntegral<T>(" 1"));
1482+
BOOST_CHECK(!ToIntegral<T>("1 "));
1483+
BOOST_CHECK(!ToIntegral<T>("1a"));
1484+
BOOST_CHECK(!ToIntegral<T>("1.1"));
1485+
BOOST_CHECK(!ToIntegral<T>("1.9"));
1486+
BOOST_CHECK(!ToIntegral<T>("+01.9"));
1487+
BOOST_CHECK(!ToIntegral<T>("-"));
1488+
BOOST_CHECK(!ToIntegral<T>("+"));
1489+
BOOST_CHECK(!ToIntegral<T>(" -1"));
1490+
BOOST_CHECK(!ToIntegral<T>("-1 "));
1491+
BOOST_CHECK(!ToIntegral<T>(" -1 "));
1492+
BOOST_CHECK(!ToIntegral<T>("+1"));
1493+
BOOST_CHECK(!ToIntegral<T>(" +1"));
1494+
BOOST_CHECK(!ToIntegral<T>(" +1 "));
1495+
BOOST_CHECK(!ToIntegral<T>("+-1"));
1496+
BOOST_CHECK(!ToIntegral<T>("-+1"));
1497+
BOOST_CHECK(!ToIntegral<T>("++1"));
1498+
BOOST_CHECK(!ToIntegral<T>("--1"));
1499+
BOOST_CHECK(!ToIntegral<T>(""));
1500+
BOOST_CHECK(!ToIntegral<T>("aap"));
1501+
BOOST_CHECK(!ToIntegral<T>("0x1"));
1502+
BOOST_CHECK(!ToIntegral<T>("-32482348723847471234"));
1503+
BOOST_CHECK(!ToIntegral<T>("32482348723847471234"));
1504+
}
1505+
14771506
BOOST_AUTO_TEST_CASE(test_ToIntegral)
14781507
{
14791508
BOOST_CHECK_EQUAL(ToIntegral<int32_t>("1234").value(), 1'234);
@@ -1486,27 +1515,14 @@ BOOST_AUTO_TEST_CASE(test_ToIntegral)
14861515
BOOST_CHECK_EQUAL(ToIntegral<int32_t>("-1234").value(), -1'234);
14871516
BOOST_CHECK_EQUAL(ToIntegral<int32_t>("-1").value(), -1);
14881517

1489-
BOOST_CHECK(!ToIntegral<int32_t>(" 1"));
1490-
BOOST_CHECK(!ToIntegral<int32_t>("1 "));
1491-
BOOST_CHECK(!ToIntegral<int32_t>("1a"));
1492-
BOOST_CHECK(!ToIntegral<int32_t>("1.1"));
1493-
BOOST_CHECK(!ToIntegral<int32_t>("1.9"));
1494-
BOOST_CHECK(!ToIntegral<int32_t>("+01.9"));
1495-
BOOST_CHECK(!ToIntegral<int32_t>(" -1"));
1496-
BOOST_CHECK(!ToIntegral<int32_t>("-1 "));
1497-
BOOST_CHECK(!ToIntegral<int32_t>(" -1 "));
1498-
BOOST_CHECK(!ToIntegral<int32_t>("+1"));
1499-
BOOST_CHECK(!ToIntegral<int32_t>(" +1"));
1500-
BOOST_CHECK(!ToIntegral<int32_t>(" +1 "));
1501-
BOOST_CHECK(!ToIntegral<int32_t>("+-1"));
1502-
BOOST_CHECK(!ToIntegral<int32_t>("-+1"));
1503-
BOOST_CHECK(!ToIntegral<int32_t>("++1"));
1504-
BOOST_CHECK(!ToIntegral<int32_t>("--1"));
1505-
BOOST_CHECK(!ToIntegral<int32_t>(""));
1506-
BOOST_CHECK(!ToIntegral<int32_t>("aap"));
1507-
BOOST_CHECK(!ToIntegral<int32_t>("0x1"));
1508-
BOOST_CHECK(!ToIntegral<int32_t>("-32482348723847471234"));
1509-
BOOST_CHECK(!ToIntegral<int32_t>("32482348723847471234"));
1518+
RunToIntegralTests<uint64_t>();
1519+
RunToIntegralTests<int64_t>();
1520+
RunToIntegralTests<uint32_t>();
1521+
RunToIntegralTests<int32_t>();
1522+
RunToIntegralTests<uint16_t>();
1523+
RunToIntegralTests<int16_t>();
1524+
RunToIntegralTests<uint8_t>();
1525+
RunToIntegralTests<int8_t>();
15101526

15111527
BOOST_CHECK(!ToIntegral<int64_t>("-9223372036854775809"));
15121528
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)
17851801
BOOST_CHECK(!ParseUInt64("-1234", &n));
17861802
}
17871803

1788-
BOOST_AUTO_TEST_CASE(test_ParseDouble)
1789-
{
1790-
double n;
1791-
// Valid values
1792-
BOOST_CHECK(ParseDouble("1234", nullptr));
1793-
BOOST_CHECK(ParseDouble("0", &n) && n == 0.0);
1794-
BOOST_CHECK(ParseDouble("1234", &n) && n == 1234.0);
1795-
BOOST_CHECK(ParseDouble("01234", &n) && n == 1234.0); // no octal
1796-
BOOST_CHECK(ParseDouble("2147483647", &n) && n == 2147483647.0);
1797-
BOOST_CHECK(ParseDouble("-2147483648", &n) && n == -2147483648.0);
1798-
BOOST_CHECK(ParseDouble("-1234", &n) && n == -1234.0);
1799-
BOOST_CHECK(ParseDouble("1e6", &n) && n == 1e6);
1800-
BOOST_CHECK(ParseDouble("-1e6", &n) && n == -1e6);
1801-
// Invalid values
1802-
BOOST_CHECK(!ParseDouble("", &n));
1803-
BOOST_CHECK(!ParseDouble(" 1", &n)); // no padding inside
1804-
BOOST_CHECK(!ParseDouble("1 ", &n));
1805-
BOOST_CHECK(!ParseDouble("1a", &n));
1806-
BOOST_CHECK(!ParseDouble("aap", &n));
1807-
BOOST_CHECK(!ParseDouble("0x1", &n)); // no hex
1808-
BOOST_CHECK(!ParseDouble(STRING_WITH_EMBEDDED_NULL_CHAR, &n));
1809-
// Overflow and underflow
1810-
BOOST_CHECK(!ParseDouble("-1e10000", nullptr));
1811-
BOOST_CHECK(!ParseDouble("1e10000", nullptr));
1812-
}
1813-
18141804
BOOST_AUTO_TEST_CASE(test_FormatParagraph)
18151805
{
18161806
BOOST_CHECK_EQUAL(FormatParagraph("", 79, 0), "");

‎src/util/strencodings.cpp

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -281,16 +281,11 @@ std::string DecodeBase32(const std::string& str, bool* pf_invalid)
281281
return std::string((const char*)vchRet.data(), vchRet.size());
282282
}
283283

284-
[[nodiscard]] static bool ParsePrechecks(const std::string&);
285-
286284
namespace {
287285
template <typename T>
288286
bool ParseIntegral(const std::string& str, T* out)
289287
{
290288
static_assert(std::is_integral<T>::value);
291-
if (!ParsePrechecks(str)) {
292-
return false;
293-
}
294289
// Replicate the exact behavior of strtol/strtoll/strtoul/strtoull when
295290
// handling leading +/- for backwards compatibility.
296291
if (str.length() >= 2 && str[0] == '+' && str[1] == '-') {
@@ -307,17 +302,6 @@ bool ParseIntegral(const std::string& str, T* out)
307302
}
308303
}; // namespace
309304

310-
[[nodiscard]] static bool ParsePrechecks(const std::string& str)
311-
{
312-
if (str.empty()) // No empty string allowed
313-
return false;
314-
if (str.size() >= 1 && (IsSpace(str[0]) || IsSpace(str[str.size()-1]))) // No padding allowed
315-
return false;
316-
if (!ValidAsCString(str)) // No embedded NUL characters allowed
317-
return false;
318-
return true;
319-
}
320-
321305
bool ParseInt32(const std::string& str, int32_t* out)
322306
{
323307
return ParseIntegral<int32_t>(str, out);
@@ -348,20 +332,6 @@ bool ParseUInt64(const std::string& str, uint64_t* out)
348332
return ParseIntegral<uint64_t>(str, out);
349333
}
350334

351-
bool ParseDouble(const std::string& str, double *out)
352-
{
353-
if (!ParsePrechecks(str))
354-
return false;
355-
if (str.size() >= 2 && str[0] == '0' && str[1] == 'x') // No hexadecimal floats allowed
356-
return false;
357-
std::istringstream text(str);
358-
text.imbue(std::locale::classic());
359-
double result;
360-
text >> result;
361-
if(out) *out = result;
362-
return text.eof() && !text.fail();
363-
}
364-
365335
std::string FormatParagraph(const std::string& in, size_t width, size_t indent)
366336
{
367337
std::stringstream out;

‎src/util/strencodings.h

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,9 @@ constexpr inline bool IsSpace(char c) noexcept {
123123
}
124124

125125
/**
126-
* Convert string to integral type T.
126+
* Convert string to integral type T. Leading whitespace, a leading +, or any
127+
* trailing character fail the parsing. The required format expressed as regex
128+
* is `-?[0-9]+`.
127129
*
128130
* @returns std::nullopt if the entire string could not be parsed, or if the
129131
* parsed value is not in the range representable by the type T.
@@ -137,7 +139,7 @@ std::optional<T> ToIntegral(const std::string& str)
137139
if (first_nonmatching != str.data() + str.size() || error_condition != std::errc{}) {
138140
return std::nullopt;
139141
}
140-
return {result};
142+
return result;
141143
}
142144

143145
/**
@@ -182,13 +184,6 @@ std::optional<T> ToIntegral(const std::string& str)
182184
*/
183185
[[nodiscard]] bool ParseUInt64(const std::string& str, uint64_t *out);
184186

185-
/**
186-
* Convert string to double with strict parse error feedback.
187-
* @returns true if the entire string could be parsed as valid double,
188-
* false if not the entire string could be parsed or when overflow or underflow occurred.
189-
*/
190-
[[nodiscard]] bool ParseDouble(const std::string& str, double *out);
191-
192187
/**
193188
* Convert a span of bytes to a lower-case hexadecimal string.
194189
*/

0 commit comments

Comments
 (0)