refactor: prefer std::from_chars over strtoul (#2359)

libtransmission/utils.cc

@@ -27,16 +27,10 @@
 #include <exception>
 #include <iterator> // std::back_inserter
 #include <set>
+#include <sstream>
 #include <string>
 #include <vector>
 
-#if defined(__GNUC__) && !__has_include(<charconv>)
-#undef HAVE_CHARCONV
-#else
-#define HAVE_CHARCONV 1
-#include <charconv> // std::from_chars()
-#endif
-
 #ifdef _WIN32
 #include <ws2tcpip.h> /* WSAStartup() */
 #include <windows.h> /* 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<size_t>(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<size_t>(str);
+    if (!second)
+    {
+        return false;
+    }
+
+    range.high = *second;
+    return true;
 }
 
 /**

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(<charconv>)
+
+#include <sstream>
+
+template<typename T>
+std::optional<T> 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(<charconv>)
+
+#include <charconv> // std::from_chars()
+
+template<typename T>
+std::optional<T> 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(<charconv>)
+
 /**
  * @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()

libtransmission/variant-benc.cc

@@ -11,8 +11,6 @@
 #include <cctype> /* isdigit() */
 #include <deque>
 #include <cerrno>
-#include <cstdlib> /* strtoul() */
-#include <cstring> /* strlen(), memchr() */
 #include <string_view>
 #include <optional>
 
@@ -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<int64_t> 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<int64_t> 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<int64_t>(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<std::string_view> 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<size_t>(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;
 }
 

libtransmission/web-utils.cc

@@ -10,6 +10,7 @@
 #include <array>
 #include <cctype>
 #include <cstddef>
+#include <limits>
 #include <optional>
 #include <string_view>
 
@@ -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<char, 16>{};
+    auto const port = tr_parseNum<int>(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<tr_port>::min() && *port <= std::numeric_limits<tr_port>::max() ? *port : -1;
 }
 
 constexpr std::string_view getPortForScheme(std::string_view scheme)