merge bitcoin#20452: Replace use of locale dependent atoi(…) with locale-independent std::from_chars(…)

This commit is contained in:
Kittywhiskers Van Gogh 2022-12-20 21:48:10 +05:30 committed by PastaPastaPasta
parent bb0a52907d
commit cae9d1791c
20 changed files with 153 additions and 59 deletions

View File

@ -21,7 +21,7 @@ static void int_atoi(benchmark::Bench& bench)
{
int value;
bench.run([&] {
value = atoi("1");
value = LocaleIndependentAtoi<int>("1");
});
}

View File

@ -58,7 +58,7 @@ CScript ParseScript(const std::string& s)
(w->front() == '-' && w->size() > 1 && std::all_of(w->begin()+1, w->end(), ::IsDigit)))
{
// Number
int64_t n = atoi64(*w);
int64_t n = LocaleIndependentAtoi<int64_t>(*w);
//limit the range of numbers ParseScript accepts in decimal
//since numbers outside -0xFFFFFFFF...0xFFFFFFFF are illegal in scripts

View File

@ -57,6 +57,7 @@
#include <util/asmap.h>
#include <util/error.h>
#include <util/moneystr.h>
#include <util/strencodings.h>
#include <util/system.h>
#include <util/threadnames.h>
#include <util/translation.h>
@ -878,7 +879,7 @@ static void CleanupBlockRevFiles()
// start removing block files.
int nContigCounter = 0;
for (const std::pair<const std::string, fs::path>& item : mapBlockFiles) {
if (atoi(item.first) == nContigCounter) {
if (LocaleIndependentAtoi<int>(item.first) == nContigCounter) {
nContigCounter++;
continue;
}

View File

@ -227,7 +227,7 @@ bool RPCConsole::RPCParseCommandLine(interfaces::Node* node, std::string &strRes
for(char argch: curarg)
if (!IsDigit(argch))
throw std::runtime_error("Invalid result query");
subelement = lastResult[atoi(curarg.c_str())];
subelement = lastResult[LocaleIndependentAtoi<int>(curarg)];
}
else if (lastResult.isObject())
subelement = find_value(lastResult, curarg);

View File

@ -18,6 +18,7 @@
#include <rpc/blockchain.h>
#include <rpc/server.h>
#include <rpc/util.h>
#include <util/strencodings.h>
#include <util/system.h>
#include <validation.h>
#include <wallet/rpcwallet.h>
@ -950,7 +951,7 @@ static UniValue gobject_getcurrentvotes(const JSONRPCRequest& request)
if (!request.params[1].isNull() && !request.params[2].isNull()) {
uint256 txid = ParseHashV(request.params[1], "Masternode Collateral hash");
std::string strVout = request.params[2].get_str();
mnCollateralOutpoint = COutPoint(txid, (uint32_t)atoi(strVout));
mnCollateralOutpoint = COutPoint(txid, LocaleIndependentAtoi<uint32_t>(strVout));
}
// FIND OBJECT USER IS LOOKING FOR

View File

@ -16,6 +16,7 @@
#include <rpc/server.h>
#include <rpc/util.h>
#include <univalue.h>
#include <util/strencodings.h>
#include <spork.h>
#include <validation.h>
#include <wallet/coincontrol.h>
@ -322,7 +323,7 @@ static UniValue masternode_winners(const JSONRPCRequest& request)
std::string strFilter = "";
if (!request.params[0].isNull()) {
nCount = atoi(request.params[0].get_str());
nCount = LocaleIndependentAtoi<int>(request.params[0].get_str());
}
if (!request.params[1].isNull()) {

View File

@ -736,7 +736,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)
std::string lpstr = lpval.get_str();
hashWatchedChain.SetHex(lpstr.substr(0, 64));
nTransactionsUpdatedLastLP = atoi64(lpstr.substr(64));
nTransactionsUpdatedLastLP = LocaleIndependentAtoi<int64_t>(lpstr.substr(64));
}
else
{

View File

@ -10,6 +10,7 @@
#include <script/signingprovider.h>
#include <tinyformat.h>
#include <util/system.h>
#include <util/string.h>
#include <util/strencodings.h>
#include <util/translation.h>

View File

@ -51,8 +51,6 @@ FUZZ_TARGET(locale)
const bool parseint32_without_locale = ParseInt32(random_string, &parseint32_out_without_locale);
int64_t parseint64_out_without_locale;
const bool parseint64_without_locale = ParseInt64(random_string, &parseint64_out_without_locale);
const int64_t atoi64_without_locale = atoi64(random_string);
const int atoi_without_locale = atoi(random_string);
const int64_t random_int64 = fuzzed_data_provider.ConsumeIntegral<int64_t>();
const std::string tostring_without_locale = ToString(random_int64);
const int32_t random_int32 = fuzzed_data_provider.ConsumeIntegral<int32_t>();
@ -75,10 +73,6 @@ FUZZ_TARGET(locale)
if (parseint64_without_locale) {
assert(parseint64_out_without_locale == parseint64_out_with_locale);
}
const int64_t atoi64_with_locale = atoi64(random_string);
assert(atoi64_without_locale == atoi64_with_locale);
const int atoi_with_locale = atoi(random_string);
assert(atoi_without_locale == atoi_with_locale);
const std::string tostring_with_locale = ToString(random_int64);
assert(tostring_without_locale == tostring_with_locale);
const std::string strprintf_int_with_locale = strprintf("%d", random_int64);

View File

@ -22,13 +22,13 @@ FUZZ_TARGET(parse_numbers)
int32_t i32;
(void)ParseInt32(random_string, &i32);
(void)atoi(random_string);
(void)LocaleIndependentAtoi<int>(random_string);
uint32_t u32;
(void)ParseUInt32(random_string, &u32);
int64_t i64;
(void)atoi64(random_string);
(void)LocaleIndependentAtoi<int64_t>(random_string);
(void)ParseFixedPoint(random_string, 3, &i64);
(void)ParseInt64(random_string, &i64);

View File

@ -120,6 +120,12 @@ bool LegacyParseUInt64(const std::string& str, uint64_t* out)
return endp && *endp == 0 && !errno &&
n <= std::numeric_limits<uint64_t>::max();
}
// For backwards compatibility checking.
int64_t atoi64_legacy(const std::string& str)
{
return strtoll(str.c_str(), nullptr, 10);
}
}; // namespace
FUZZ_TARGET(string)
@ -262,4 +268,22 @@ FUZZ_TARGET(string)
assert(u8 == u8_legacy);
}
}
{
const int atoi_result = atoi(random_string_1.c_str());
const int locale_independent_atoi_result = LocaleIndependentAtoi<int>(random_string_1);
const int64_t atoi64_result = atoi64_legacy(random_string_1);
const bool out_of_range = atoi64_result < std::numeric_limits<int>::min() || atoi64_result > std::numeric_limits<int>::max();
if (out_of_range) {
assert(locale_independent_atoi_result == 0);
} else {
assert(atoi_result == locale_independent_atoi_result);
}
}
{
const int64_t atoi64_result = atoi64_legacy(random_string_1);
const int64_t locale_independent_atoi_result = LocaleIndependentAtoi<int64_t>(random_string_1);
assert(atoi64_result == locale_independent_atoi_result || locale_independent_atoi_result == 0);
}
}

View File

@ -8,6 +8,7 @@
#include <evo/specialtx.h>
#include <primitives/transaction.h>
#include <uint256.h>
#include <util/string.h>
#include <util/strencodings.h>
#include <boost/test/unit_test.hpp>

View File

@ -1368,6 +1368,77 @@ BOOST_AUTO_TEST_CASE(test_ToIntegral)
BOOST_CHECK(!ToIntegral<uint8_t>("256"));
}
BOOST_AUTO_TEST_CASE(test_LocaleIndependentAtoi)
{
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("1234"), 1'234);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("0"), 0);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("01234"), 1'234);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("-1234"), -1'234);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>(" 1"), 1);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("1 "), 1);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("1a"), 1);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("1.1"), 1);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("1.9"), 1);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("+01.9"), 1);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("-1"), -1);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>(" -1"), -1);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("-1 "), -1);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>(" -1 "), -1);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("+1"), 1);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>(" +1"), 1);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>(" +1 "), 1);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("+-1"), 0);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("-+1"), 0);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("++1"), 0);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("--1"), 0);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>(""), 0);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("aap"), 0);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("0x1"), 0);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("-32482348723847471234"), 0);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("32482348723847471234"), 0);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int64_t>("-9223372036854775809"), 0);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int64_t>("-9223372036854775808"), -9'223'372'036'854'775'807LL - 1LL);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int64_t>("9223372036854775807"), 9'223'372'036'854'775'807);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int64_t>("9223372036854775808"), 0);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint64_t>("-1"), 0U);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint64_t>("0"), 0U);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint64_t>("18446744073709551615"), 18'446'744'073'709'551'615ULL);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint64_t>("18446744073709551616"), 0U);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("-2147483649"), 0);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("-2147483648"), -2'147'483'648LL);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("2147483647"), 2'147'483'647);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("2147483648"), 0);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint32_t>("-1"), 0U);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint32_t>("0"), 0U);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint32_t>("4294967295"), 4'294'967'295U);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint32_t>("4294967296"), 0U);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int16_t>("-32769"), 0);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int16_t>("-32768"), -32'768);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int16_t>("32767"), 32'767);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int16_t>("32768"), 0);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint16_t>("-1"), 0U);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint16_t>("0"), 0U);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint16_t>("65535"), 65'535U);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint16_t>("65536"), 0U);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int8_t>("-129"), 0);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int8_t>("-128"), -128);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int8_t>("127"), 127);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int8_t>("128"), 0);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint8_t>("-1"), 0U);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint8_t>("0"), 0U);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint8_t>("255"), 255U);
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint8_t>("256"), 0U);
}
BOOST_AUTO_TEST_CASE(test_ParseInt64)
{
int64_t n;

View File

@ -147,7 +147,7 @@ void TorControlConnection::readcb(struct bufferevent *bev, void *ctx)
if (s.size() < 4) // Short line
continue;
// <status>(-|+| )<data><CRLF>
self->message.code = atoi(s.substr(0,3));
self->message.code = LocaleIndependentAtoi<int>(s.substr(0,3));
self->message.lines.push_back(s.substr(4));
char ch = s[3]; // '-','+' or ' '
if (ch == ' ') {

View File

@ -73,8 +73,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)) {

View File

@ -404,20 +404,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:

View File

@ -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.

View File

@ -164,16 +164,14 @@ bool CheckDiskSpace(const fs::path& dir, uint64_t additional_bytes)
/**
* 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.
@ -182,7 +180,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)
@ -511,7 +509,7 @@ std::string ArgsManager::GetArg(const std::string& strArg, const std::string& st
int64_t ArgsManager::GetArg(const std::string& strArg, int64_t nDefault) const
{
const util::SettingsValue value = ArgsManagerHelper::Get(*this, 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

View File

@ -217,7 +217,7 @@ static inline void ReadOrderPos(int64_t& nOrderPos, mapValue_t& mapValue)
nOrderPos = -1; // TODO: calculate elsewhere
return;
}
nOrderPos = atoi64(mapValue["n"]);
nOrderPos = LocaleIndependentAtoi<int64_t>(mapValue["n"]);
}
@ -440,7 +440,7 @@ public:
}
ReadOrderPos(nOrderPos, mapValue);
nTimeSmart = mapValue.count("timesmart") ? (unsigned int)atoi64(mapValue["timesmart"]) : 0;
nTimeSmart = mapValue.count("timesmart") ? (unsigned int)LocaleIndependentAtoi<int64_t>(mapValue["timesmart"]) : 0;
mapValue.erase("fromaccount");
mapValue.erase("spent");

View File

@ -5,31 +5,22 @@
export LC_ALL=C
# TODO: Reduce KNOWN_VIOLATIONS by replacing uses of locale dependent stoul/strtol with locale
# independent ToIntegral<T>(...).
# TODO: Reduce KNOWN_VIOLATIONS by replacing uses of locale dependent snprintf with strprintf.
KNOWN_VIOLATIONS=(
"src/bench/string_cast.cpp.*atoi"
"src/bitcoin-tx.cpp.*stoul"
"src/bitcoin-tx.cpp.*trim_right"
"src/dbwrapper.cpp.*stoul"
"src/dbwrapper.cpp:.*vsnprintf"
"src/httprpc.cpp.*trim"
"src/init.cpp:.*atoi"
"src/qt/rpcconsole.cpp:.*atoi"
"src/rest.cpp:.*strtol"
"src/rpc/blockchain.cpp.*atoi"
"src/rpc/governance.cpp.*atoi"
"src/rpc/masternode.cpp.*atoi"
"src/statsd_client.cpp:.*snprintf"
"src/test/dbwrapper_tests.cpp:.*snprintf"
"src/test/fuzz/locale.cpp"
"src/test/fuzz/parse_numbers.cpp:.*atoi"
"src/test/fuzz/string.cpp"
"src/torcontrol.cpp:.*atoi"
"src/torcontrol.cpp:.*strtol"
"src/util/strencodings.cpp:.*atoi"
"src/util/strencodings.cpp:.*strtoll"
"src/util/strencodings.h:.*atoi"
"src/wallet/wallet.cpp:.*atoi"
"src/util/system.cpp:.*atoi"
"src/util/system.cpp:.*fprintf"
)