refactor: split tr_parseNum inout var into two vars (#3748)

build: re-enable cert-err34-c warning

libtransmission/.clang-tidy

@@ -7,7 +7,6 @@ Checks: >
   -bugprone-implicit-widening-of-multiplication-result,
   -bugprone-narrowing-conversions,
   cert-*,
-  -cert-err34-c,
   -cert-err58-cpp,
   clang-analyzer-core.*,
   clang-analyzer-cplusplus.*,

libtransmission/clients.cc

@@ -180,7 +180,7 @@ constexpr std::string_view charint(char ch)
 int strint(char const* pch, int span, int base = 10)
 {
     auto sv = std::string_view{ pch, static_cast<size_t>(span) };
-    return tr_parseNum<int>(sv, base).value_or(0);
+    return tr_parseNum<int>(sv, nullptr, base).value_or(0);
 }
 
 constexpr std::string_view getMnemonicEnd(uint8_t ch)

libtransmission/rpc-server.cc

@@ -1039,7 +1039,7 @@ tr_rpc_server::tr_rpc_server(tr_session* session_in, tr_variant* settings)
     {
         /* Read the socket permission as a string representing an octal number. */
         is_missing_rpc_socket_mode_key = false;
-        i = tr_parseNum<int>(sv, 8).value_or(tr_rpc_server::DefaultRpcSocketMode);
+        i = tr_parseNum<int>(sv, nullptr, 8).value_or(tr_rpc_server::DefaultRpcSocketMode);
     }
     else if (tr_variantDictFindInt(settings, key, &i))
     {

libtransmission/session.cc

@@ -386,9 +386,9 @@ void tr_sessionGetDefaultSettings(tr_variant* setme_dictionary)
     tr_variantDictAddInt(d, TR_KEY_message_level, TR_LOG_INFO);
     tr_variantDictAddInt(d, TR_KEY_download_queue_size, 5);
     tr_variantDictAddBool(d, TR_KEY_download_queue_enabled, true);
-    tr_variantDictAddInt(d, TR_KEY_peer_limit_global, atoi(TR_DEFAULT_PEER_LIMIT_GLOBAL_STR));
-    tr_variantDictAddInt(d, TR_KEY_peer_limit_per_torrent, atoi(TR_DEFAULT_PEER_LIMIT_TORRENT_STR));
-    tr_variantDictAddInt(d, TR_KEY_peer_port, atoi(TR_DEFAULT_PEER_PORT_STR));
+    tr_variantDictAddInt(d, TR_KEY_peer_limit_global, *tr_parseNum<int64_t>(TR_DEFAULT_PEER_LIMIT_GLOBAL_STR));
+    tr_variantDictAddInt(d, TR_KEY_peer_limit_per_torrent, *tr_parseNum<int64_t>(TR_DEFAULT_PEER_LIMIT_TORRENT_STR));
+    tr_variantDictAddInt(d, TR_KEY_peer_port, *tr_parseNum<int64_t>(TR_DEFAULT_PEER_PORT_STR));
     tr_variantDictAddBool(d, TR_KEY_peer_port_random_on_start, false);
     tr_variantDictAddInt(d, TR_KEY_peer_port_random_low, 49152);
     tr_variantDictAddInt(d, TR_KEY_peer_port_random_high, 65535);
@@ -792,7 +792,7 @@ void tr_session::setImpl(init_data& data)
     if (tr_variantDictFindStrView(settings, TR_KEY_umask, &sv))
     {
         /* Read a umask as a string representing an octal number. */
-        this->umask_ = static_cast<mode_t>(tr_parseNum<uint32_t>(sv, 8).value_or(DefaultUmask));
+        this->umask_ = static_cast<mode_t>(tr_parseNum<uint32_t>(sv, nullptr, 8).value_or(DefaultUmask));
         ::umask(this->umask_);
     }
     else if (tr_variantDictFindInt(settings, TR_KEY_umask, &i))

libtransmission/tr-lpd.cc

@@ -124,7 +124,7 @@ std::optional<ParsedAnnounce> parseAnnounceMsg(std::string_view announce)
     {
         // parse `${major}.${minor}`
         auto walk = announce.substr(pos + std::size(key));
-        if (auto const major = tr_parseNum<int>(walk); major && tr_strvStartsWith(walk, '.'))
+        if (auto const major = tr_parseNum<int>(walk, &walk); major && tr_strvStartsWith(walk, '.'))
         {
             ret.major = *major;
         }
@@ -134,7 +134,7 @@ std::optional<ParsedAnnounce> parseAnnounceMsg(std::string_view announce)
         }
 
         walk.remove_prefix(1); // the '.' between major and minor
-        if (auto const minor = tr_parseNum<int>(walk); minor && tr_strvStartsWith(walk, CrLf))
+        if (auto const minor = tr_parseNum<int>(walk, &walk); minor && tr_strvStartsWith(walk, CrLf))
         {
             ret.minor = *minor;
         }
@@ -148,7 +148,7 @@ std::optional<ParsedAnnounce> parseAnnounceMsg(std::string_view announce)
     if (auto const pos = announce.find(key); pos != std::string_view::npos)
     {
         auto walk = announce.substr(pos + std::size(key));
-        if (auto const port = tr_parseNum<uint16_t>(walk); port && tr_strvStartsWith(walk, CrLf))
+        if (auto const port = tr_parseNum<uint16_t>(walk, &walk); port && tr_strvStartsWith(walk, CrLf))
         {
             ret.port = tr_port::fromHost(*port);
         }

libtransmission/utils.cc

@@ -545,7 +545,7 @@ bool parseNumberSection(std::string_view str, number_range& range)
 {
     auto constexpr Delimiter = "-"sv;
 
-    auto const first = tr_parseNum<size_t>(str);
+    auto const first = tr_parseNum<size_t>(str, &str);
     if (!first)
     {
         return false;
@@ -615,7 +615,7 @@ double tr_truncd(double x, int decimal_places)
         pt[decimal_places != 0 ? decimal_places + 1 : 0] = '\0';
     }
 
-    return atof(std::data(buf));
+    return *tr_parseNum<double>(std::data(buf));
 }
 
 std::string tr_strpercent(double x)
@@ -962,24 +962,14 @@ int tr_env_get_int(char const* key, int default_value)
 {
     TR_ASSERT(key != nullptr);
 
-#ifdef _WIN32
-
-    auto value = std::array<char, 16>{};
-
-    if (GetEnvironmentVariableA(key, std::data(value), std::size(value)) > 1)
+    if (auto const valstr = tr_env_get_string(key); !std::empty(valstr))
     {
-        return atoi(std::data(value));
+        if (auto const valint = tr_parseNum<int>(valstr); valint)
+        {
+            return *valint;
+        }
     }
 
-#else
-
-    if (char const* const value = getenv(key); !tr_str_is_empty(value))
-    {
-        return atoi(value);
-    }
-
-#endif
-
     return default_value;
 }
 
@@ -1069,11 +1059,11 @@ std::string_view tr_get_mime_type_for_filename(std::string_view filename)
 #include <sstream>
 
 template<typename T, std::enable_if_t<std::is_integral<T>::value, bool> = true>
-[[nodiscard]] std::optional<T> tr_parseNum(std::string_view& sv, int base)
+[[nodiscard]] std::optional<T> tr_parseNum(std::string_view str, std::string_view* remainder, int base)
 {
     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 tmpstr = std::string(std::data(str), std::min(std::size(str), size_t{ 64 }));
+    auto sstream = std::stringstream{ tmpstr };
     auto const oldpos = sstream.tellg();
     /* The base parameter only works for bases 8, 10 and 16.
        All other bases will be converted to 0 which activates the
@@ -1085,7 +1075,11 @@ template<typename T, std::enable_if_t<std::is_integral<T>::value, bool> = true>
     {
         return std::nullopt;
     }
-    sv.remove_prefix(sstream.eof() ? std::size(sv) : newpos - oldpos);
+    if (remainder != nullptr)
+    {
+        *remainder = str;
+        remainder->remove_prefix(sstream.eof() ? std::size(str) : newpos - oldpos);
+    }
     return val;
 }
 
@@ -1094,11 +1088,11 @@ template<typename T, std::enable_if_t<std::is_integral<T>::value, bool> = true>
 #include <charconv> // std::from_chars()
 
 template<typename T, std::enable_if_t<std::is_integral<T>::value, bool>>
-[[nodiscard]] std::optional<T> tr_parseNum(std::string_view& sv, int base)
+[[nodiscard]] std::optional<T> tr_parseNum(std::string_view str, std::string_view* remainder, int base)
 {
     auto val = T{};
-    auto const* const begin_ch = std::data(sv);
-    auto const* const end_ch = begin_ch + std::size(sv);
+    auto const* const begin_ch = std::data(str);
+    auto const* const end_ch = begin_ch + std::size(str);
     /* The base parameter works for any base from 2 to 36 (inclusive).
        This is different from the behaviour of the stringstream
        based solution above. */
@@ -1107,36 +1101,44 @@ template<typename T, std::enable_if_t<std::is_integral<T>::value, bool>>
     {
         return std::nullopt;
     }
-    sv.remove_prefix(result.ptr - std::data(sv));
+    if (remainder != nullptr)
+    {
+        *remainder = str;
+        remainder->remove_prefix(result.ptr - std::data(str));
+    }
     return val;
 }
 
 #endif // #if defined(__GNUC__) && !__has_include(<charconv>)
 
-template std::optional<long long> tr_parseNum(std::string_view& sv, int base);
-template std::optional<long> tr_parseNum(std::string_view& sv, int base);
-template std::optional<int> tr_parseNum(std::string_view& sv, int base);
-template std::optional<char> tr_parseNum(std::string_view& sv, int base);
+template std::optional<long long> tr_parseNum(std::string_view str, std::string_view* remainder, int base);
+template std::optional<long> tr_parseNum(std::string_view str, std::string_view* remainder, int base);
+template std::optional<int> tr_parseNum(std::string_view str, std::string_view* remainder, int base);
+template std::optional<char> tr_parseNum(std::string_view str, std::string_view* remainder, int base);
 
-template std::optional<unsigned long long> tr_parseNum(std::string_view& sv, int base);
-template std::optional<unsigned long> tr_parseNum(std::string_view& sv, int base);
-template std::optional<unsigned int> tr_parseNum(std::string_view& sv, int base);
-template std::optional<unsigned short> tr_parseNum(std::string_view& sv, int base);
-template std::optional<unsigned char> tr_parseNum(std::string_view& sv, int base);
+template std::optional<unsigned long long> tr_parseNum(std::string_view str, std::string_view* remainder, int base);
+template std::optional<unsigned long> tr_parseNum(std::string_view str, std::string_view* remainder, int base);
+template std::optional<unsigned int> tr_parseNum(std::string_view str, std::string_view* remainder, int base);
+template std::optional<unsigned short> tr_parseNum(std::string_view str, std::string_view* remainder, int base);
+template std::optional<unsigned char> tr_parseNum(std::string_view str, std::string_view* remainder, int base);
 
 template<typename T, std::enable_if_t<std::is_floating_point<T>::value, bool>>
-[[nodiscard]] std::optional<T> tr_parseNum(std::string_view& sv)
+[[nodiscard]] std::optional<T> tr_parseNum(std::string_view str, std::string_view* remainder)
 {
-    auto const* const begin_ch = std::data(sv);
-    auto const* const end_ch = begin_ch + std::size(sv);
+    auto const* const begin_ch = std::data(str);
+    auto const* const end_ch = begin_ch + std::size(str);
     auto val = T{};
     auto const result = fast_float::from_chars(begin_ch, end_ch, val);
     if (result.ec != std::errc{})
     {
         return std::nullopt;
     }
-    sv.remove_prefix(result.ptr - std::data(sv));
+    if (remainder != nullptr)
+    {
+        *remainder = str;
+        remainder->remove_prefix(result.ptr - std::data(str));
+    }
     return val;
 }
 
-template std::optional<double> tr_parseNum(std::string_view& sv);
+template std::optional<double> tr_parseNum(std::string_view sv, std::string_view* remainder);

libtransmission/utils.h

@@ -87,10 +87,10 @@ constexpr auto tr_saveFile(std::string_view filename, ContiguousRange const& x,
 void tr_wait_msec(long int delay_milliseconds);
 
 template<typename T, std::enable_if_t<std::is_integral<T>::value, bool> = true>
-[[nodiscard]] std::optional<T> tr_parseNum(std::string_view& sv, int base = 10);
+[[nodiscard]] std::optional<T> tr_parseNum(std::string_view str, std::string_view* setme_remainder = nullptr, int base = 10);
 
 template<typename T, std::enable_if_t<std::is_floating_point<T>::value, bool> = true>
-[[nodiscard]] std::optional<T> tr_parseNum(std::string_view& sv);
+[[nodiscard]] std::optional<T> tr_parseNum(std::string_view str, std::string_view* setme_remainder = nullptr);
 
 #ifdef _WIN32
 

libtransmission/variant-benc.cc

@@ -74,7 +74,7 @@ std::optional<int64_t> ParseInt(std::string_view* benc)
     }
 
     // parse the string and make sure the next char is `Suffix`
-    auto const value = tr_parseNum<int64_t>(walk);
+    auto const value = tr_parseNum<int64_t>(walk, &walk);
     if (!value || !tr_strvStartsWith(walk, Suffix))
     {
         return {};
@@ -107,7 +107,7 @@ std::optional<std::string_view> ParseString(std::string_view* benc)
         return {};
     }
 
-    auto const len = tr_parseNum<size_t>(svtmp);
+    auto const len = tr_parseNum<size_t>(svtmp, &svtmp);
     if (!len || *len >= MaxBencStrLength)
     {
         return {};