Replace use of locale dependent atoi(…) with locale-independent std::from_chars(…) (C++17)

test: Add test cases for LocaleIndependentAtoi

fuzz: Assert legacy atoi(s) == LocaleIndependentAtoi<int>(s)

fuzz: Assert legacy atoi64(s) == LocaleIndependentAtoi<int64_t>(s)

4 files changed, 38 insertions, 29 deletions

diff --git a/src/util/moneystr.cpp b/src/util/moneystr.cpp
index d3f4029607..95a919ad12 100644
--- a/src/util/moneystr.cpp
+++ b/src/util/moneystr.cpp
@@ -77,8 +77,7 @@ std::optional<CAmount> ParseMoney(const std::string& money_string)
         return std::nullopt;
     if (nUnits < 0 || nUnits > COIN)
         return std::nullopt;
-    int64_t nWhole = atoi64(strWhole);
-
+    int64_t nWhole = LocaleIndependentAtoi<int64_t>(strWhole);
     CAmount value = nWhole * COIN + nUnits;
 
     if (!MoneyRange(value)) {
diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp
index 0aa80ea0ae..88fec6740a 100644
--- a/src/util/strencodings.cpp
+++ b/src/util/strencodings.cpp
@@ -403,20 +403,6 @@ std::string FormatParagraph(const std::string& in, size_t width, size_t indent)
     return out.str();
 }
 
-int64_t atoi64(const std::string& str)
-{
-#ifdef _MSC_VER
-    return _atoi64(str.c_str());
-#else
-    return strtoll(str.c_str(), nullptr, 10);
-#endif
-}
-
-int atoi(const std::string& str)
-{
-    return atoi(str.c_str());
-}
-
 /** Upper bound for mantissa.
  * 10^18-1 is the largest arbitrary decimal that will fit in a signed 64-bit integer.
  * Larger integers cannot consist of arbitrary combinations of 0-9:
diff --git a/src/util/strencodings.h b/src/util/strencodings.h
index 1217572c45..166352c42f 100644
--- a/src/util/strencodings.h
+++ b/src/util/strencodings.h
@@ -11,6 +11,7 @@
 
 #include <attributes.h>
 #include <span.h>
+#include <util/string.h>
 
 #include <charconv>
 #include <cstdint>
@@ -68,8 +69,33 @@ std::string EncodeBase32(Span<const unsigned char> input, bool pad = true);
 std::string EncodeBase32(const std::string& str, bool pad = true);
 
 void SplitHostPort(std::string in, uint16_t& portOut, std::string& hostOut);
-int64_t atoi64(const std::string& str);
-int atoi(const std::string& str);
+
+// LocaleIndependentAtoi is provided for backwards compatibility reasons.
+//
+// New code should use the ParseInt64/ParseUInt64/ParseInt32/ParseUInt32 functions
+// which provide parse error feedback.
+//
+// The goal of LocaleIndependentAtoi is to replicate the exact defined behaviour
+// of atoi and atoi64 as they behave under the "C" locale.
+template <typename T>
+T LocaleIndependentAtoi(const std::string& str)
+{
+    static_assert(std::is_integral<T>::value);
+    T result;
+    // Emulate atoi(...) handling of white space and leading +/-.
+    std::string s = TrimString(str);
+    if (!s.empty() && s[0] == '+') {
+        if (s.length() >= 2 && s[1] == '-') {
+            return 0;
+        }
+        s = s.substr(1);
+    }
+    auto [_, error_condition] = std::from_chars(s.data(), s.data() + s.size(), result);
+    if (error_condition != std::errc{}) {
+        return 0;
+    }
+    return result;
+}
 
 /**
  * Tests if the given character is a decimal digit.
diff --git a/src/util/system.cpp b/src/util/system.cpp
index 4defeed4ce..79c08816fa 100644
--- a/src/util/system.cpp
+++ b/src/util/system.cpp
@@ -158,16 +158,14 @@ std::streampos GetFileSize(const char* path, std::streamsize max) {
 /**
  * Interpret a string argument as a boolean.
  *
- * The definition of atoi() requires that non-numeric string values like "foo",
- * return 0. This means that if a user unintentionally supplies a non-integer
- * argument here, the return value is always false. This means that -foo=false
- * does what the user probably expects, but -foo=true is well defined but does
- * not do what they probably expected.
+ * The definition of LocaleIndependentAtoi<int>() requires that non-numeric string values
+ * like "foo", return 0. This means that if a user unintentionally supplies a
+ * non-integer argument here, the return value is always false. This means that
+ * -foo=false does what the user probably expects, but -foo=true is well defined
+ * but does not do what they probably expected.
  *
- * The return value of atoi() is undefined when given input not representable as
- * an int. On most systems this means string value between "-2147483648" and
- * "2147483647" are well defined (this method will return true). Setting
- * -txindex=2147483648 on most systems, however, is probably undefined.
+ * The return value of LocaleIndependentAtoi<int>(...) is zero when given input not
+ * representable as an int.
  *
  * For a more extensive discussion of this topic (and a wide range of opinions
  * on the Right Way to change this code), see PR12713.
@@ -176,7 +174,7 @@ static bool InterpretBool(const std::string& strValue)
 {
     if (strValue.empty())
         return true;
-    return (atoi(strValue) != 0);
+    return (LocaleIndependentAtoi<int>(strValue) != 0);
 }
 
 static std::string SettingName(const std::string& arg)
@@ -594,7 +592,7 @@ std::string ArgsManager::GetArg(const std::string& strArg, const std::string& st
 int64_t ArgsManager::GetIntArg(const std::string& strArg, int64_t nDefault) const
 {
     const util::SettingsValue value = GetSetting(strArg);
-    return value.isNull() ? nDefault : value.isFalse() ? 0 : value.isTrue() ? 1 : value.isNum() ? value.get_int64() : atoi64(value.get_str());
+    return value.isNull() ? nDefault : value.isFalse() ? 0 : value.isTrue() ? 1 : value.isNum() ? value.get_int64() : LocaleIndependentAtoi<int64_t>(value.get_str());
 }
 
 bool ArgsManager::GetBoolArg(const std::string& strArg, bool fDefault) const