From 46f9582e366f5adf57a7711174ee5b3ff36ee14e Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 29 Dec 2021 02:28:12 -0600 Subject: [PATCH] refactor: prefer std::from_chars over strtoul (#2359) --- libtransmission/utils.cc | 67 ++++++++++++--------------------- libtransmission/utils.h | 42 +++++++++++++++++++++ libtransmission/variant-benc.cc | 41 ++++++++++---------- libtransmission/web-utils.cc | 20 ++-------- 4 files changed, 90 insertions(+), 80 deletions(-) diff --git a/libtransmission/utils.cc b/libtransmission/utils.cc index 04c21ab3c..e388614bf 100644 --- a/libtransmission/utils.cc +++ b/libtransmission/utils.cc @@ -27,16 +27,10 @@ #include #include // std::back_inserter #include +#include #include #include -#if defined(__GNUC__) && !__has_include() -#undef HAVE_CHARCONV -#else -#define HAVE_CHARCONV 1 -#include // std::from_chars() -#endif - #ifdef _WIN32 #include /* WSAStartup() */ #include /* Sleep(), GetSystemTimeAsFileTime(), GetEnvironmentVariable() */ @@ -1110,45 +1104,34 @@ struct number_range */ static bool parseNumberSection(std::string_view str, number_range& range) { - auto const error = errno; - auto success = bool{}; + auto constexpr Delimiter = "-"sv; -#if defined(HAVE_CHARCONV) - // wants char*, so string_view::iterator don't work. make our own begin/end - auto const* const begin_ch = std::data(str); - auto const* const end_ch = begin_ch + std::size(str); - auto result = std::from_chars(begin_ch, end_ch, range.low); - success = result.ec == std::errc{}; - if (success) + auto const first = tr_parseNum(str); + if (!first) { - range.high = range.low; - if (result.ptr < end_ch && *result.ptr == '-') - { - result = std::from_chars(result.ptr + 1, end_ch, range.high); - success = result.ec == std::errc{}; - } + return false; } -#else - try - { - auto tmp = std::string(str); - auto pos = size_t{}; - range.low = range.high = std::stoi(tmp, &pos); - if (pos != std::size(tmp) && tmp[pos] == '-') - { - tmp.erase(0, pos + 1); - range.high = std::stoi(tmp, &pos); - } - success = true; - } - catch (std::exception&) - { - success = false; - } -#endif - errno = error; - return success; + range.low = range.high = *first; + if (std::empty(str)) + { + return true; + } + + if (!tr_strvStartsWith(str, Delimiter)) + { + return false; + } + + str.remove_prefix(std::size(Delimiter)); + auto const second = tr_parseNum(str); + if (!second) + { + return false; + } + + range.high = *second; + return true; } /** diff --git a/libtransmission/utils.h b/libtransmission/utils.h index 5d897decc..ed07fdd92 100644 --- a/libtransmission/utils.h +++ b/libtransmission/utils.h @@ -126,6 +126,48 @@ uint64_t tr_time_msec(void); /** @brief sleep the specified number of milliseconds */ void tr_wait_msec(long int delay_milliseconds); +#if defined(__GNUC__) && !__has_include() + +#include + +template +std::optional tr_parseNum(std::string_view& sv) +{ + auto val = T{}; + auto const str = std::string(std::data(sv), std::min(std::size(sv), size_t{ 64 })); + auto sstream = std::stringstream{ str }; + auto const oldpos = sstream.tellg(); + sstream >> val; + auto const newpos = sstream.tellg(); + if ((newpos == oldpos) || (sstream.fail() && !sstream.eof())) + { + return std::nullopt; + } + sv.remove_prefix(sstream.eof() ? std::size(sv) : newpos - oldpos); + return val; +} + +#else // #if defined(__GNUC__) && !__has_include() + +#include // std::from_chars() + +template +std::optional tr_parseNum(std::string_view& sv) +{ + auto val = T{}; + auto const* const begin_ch = std::data(sv); + auto const* const end_ch = begin_ch + std::size(sv); + auto const result = std::from_chars(begin_ch, end_ch, val); + if (result.ec != std::errc{}) + { + return std::nullopt; + } + sv.remove_prefix(result.ptr - std::data(sv)); + return val; +} + +#endif // #if defined(__GNUC__) && !__has_include() + /** * @brief make a copy of 'str' whose non-utf8 content has been corrected or stripped * @return a newly-allocated string that must be freed with tr_free() diff --git a/libtransmission/variant-benc.cc b/libtransmission/variant-benc.cc index 15a82bf05..cb607ef81 100644 --- a/libtransmission/variant-benc.cc +++ b/libtransmission/variant-benc.cc @@ -11,8 +11,6 @@ #include /* isdigit() */ #include #include -#include /* strtoul() */ -#include /* strlen(), memchr() */ #include #include @@ -29,7 +27,7 @@ using namespace std::literals; -#define MAX_BENC_STR_LENGTH (128 * 1024 * 1024) /* arbitrary */ +auto constexpr MaxBencStrLength = size_t{ 128 * 1024 * 1024 }; // arbitrary /*** **** tr_variantParse() @@ -48,16 +46,19 @@ using namespace std::literals; */ std::optional tr_bencParseInt(std::string_view* benc) { + auto constexpr Prefix = "i"sv; + auto constexpr Suffix = "e"sv; + // find the beginning delimiter auto walk = *benc; - if (std::size(walk) < 3 || walk.front() != 'i') + if (std::size(walk) < 3 || !tr_strvStartsWith(walk, Prefix)) { return {}; } // find the ending delimiter - walk.remove_prefix(1); - auto const pos = walk.find('e'); + walk.remove_prefix(std::size(Prefix)); + auto const pos = walk.find(Suffix); if (pos == std::string_view::npos) { return {}; @@ -69,17 +70,16 @@ std::optional tr_bencParseInt(std::string_view* benc) return {}; } - errno = 0; - char* endptr = nullptr; - auto const value = std::strtoll(std::data(walk), &endptr, 10); - if (errno != 0 || endptr != std::data(walk) + pos) + // parse the string and make sure the next char is `Suffix` + auto const value = tr_parseNum(walk); + if (!value || !tr_strvStartsWith(walk, Suffix)) { return {}; } - walk.remove_prefix(pos + 1); + walk.remove_prefix(std::size(Suffix)); *benc = walk; - return value; + return *value; } /** @@ -98,25 +98,22 @@ std::optional tr_bencParseStr(std::string_view* benc) } // get the string length - errno = 0; - char* ulend = nullptr; - auto const len = strtoul(std::data(*benc), &ulend, 10); - if (errno != 0 || ulend != std::data(*benc) + colon_pos || len >= MAX_BENC_STR_LENGTH) + auto svtmp = benc->substr(0, colon_pos); + auto const len = tr_parseNum(svtmp); + if (!len || *len >= MaxBencStrLength) { return {}; } // do we have `len` bytes of string data? - auto walk = *benc; - walk.remove_prefix(colon_pos + 1); - if (std::size(walk) < len) + svtmp = benc->substr(colon_pos + 1); + if (std::size(svtmp) < len) { return {}; } - auto const string = walk.substr(0, len); - walk.remove_prefix(len); - *benc = walk; + auto const string = svtmp.substr(0, *len); + *benc = svtmp.substr(*len); return string; } diff --git a/libtransmission/web-utils.cc b/libtransmission/web-utils.cc index 8cf027f57..0e6b16ba5 100644 --- a/libtransmission/web-utils.cc +++ b/libtransmission/web-utils.cc @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -241,24 +242,11 @@ void tr_http_escape_sha1(char* out, uint8_t const* sha1_digest) namespace { -int parsePort(std::string_view port) +auto parsePort(std::string_view port_sv) { - auto tmp = std::array{}; + auto const port = tr_parseNum(port_sv); - if (std::size(port) >= std::size(tmp)) - { - return -1; - } - - std::copy(std::begin(port), std::end(port), std::begin(tmp)); - char* end = nullptr; - long port_num = strtol(std::data(tmp), &end, 10); - if (*end != '\0' || port_num <= 0 || port_num >= 65536) - { - port_num = -1; - } - - return int(port_num); + return port && *port >= std::numeric_limits::min() && *port <= std::numeric_limits::max() ? *port : -1; } constexpr std::string_view getPortForScheme(std::string_view scheme)