fix: sonarcloud warnings (#6615)

* fix: cpp:S3358 conditional-operators-should-not-be-nested warning in gtk client

* fix: cpp:S1659 define-each-identifier-in-a-dedicated-statement warning in peer-mgr-wishlist

* Revert "fix; cpp:S3642 replace-enum-with-enum-class warning in gtk client"

* fix: cpp:S3576 remove-virtual-specifier-or-replace-it-by-override warning in rpc-server::Settings

* fix: cpp:S3576 remove-virtual-specifier-or-replace-it-by-override warning in tr_session_alt_speeds::Settings

* fix: remove unnecessary Settings subclass declarations

* fix: cpp:S1117 shadow warning in qt client

* fix: cpp:S6004 use init-statement to limit scope of local

* fix: cpp:S5997 replace-std-lock-guard-with-std-sccoped-lock in favicon-cache.h

* fix: cpp:S1659 define-each-identifier-in-a-dedicated-statement warning in announcer

* fix: cpp:S5817 function-should-be-declared-const warning in cache.h

* fix: cpp:S1186 explain-why-method-is-empty warning in favicon-cache

* fix: cpp:S5408 constexpr-variables-should-not-be-declared-inline warning in favicon-cache

* fix: cpp:S3624 explicitly delete copy assignment, ctor of InFlightData

* fix: cpp:S5997 use std-scoped-lock-instead-of-std-lock-guard

* fix: cpp:S5997 use std-scoped-lock-instead-of-std-lock-guard

* fix: cpp:S5817 function-should-be-declared-const warning in favicon-cache.h

* fix: cpp:S5817 function-should-be-declared-const warning in lru-cache

* fix: cpp:S1709 add-the-explicit-keyword-to-this-constructor

gtk/PrefsDialog.cc

@@ -974,12 +974,25 @@ void NetworkPage::portTestSetSensitive()
 
 void NetworkPage::onPortTested(std::optional<bool> const result, Session::PortTestIpProtocol const ip_protocol)
 {
+    auto constexpr ResultToStatus = [](std::optional<bool> const res)
+    {
+        if (!res)
+        {
+            return PORT_TEST_ERROR;
+        }
+        if (!*res)
+        {
+            return PORT_TEST_CLOSED;
+        }
+        return PORT_TEST_OPEN;
+    };
+
     // Only update the UI if the current status is "checking", so that
     // we won't show the port test results for the old peer port if it
     // changed while we have port-test RPC call(s) in-flight.
-    if (portTestStatus_[ip_protocol] == PORT_TEST_CHECKING)
+    if (auto& status = portTestStatus_[ip_protocol]; status == PORT_TEST_CHECKING)
     {
-        portTestStatus_[ip_protocol] = result ? (*result ? PORT_TEST_OPEN : PORT_TEST_CLOSED) : PORT_TEST_ERROR;
+        status = ResultToStatus(result);
         updatePortStatusText();
     }
     portTestSetSensitive();

libtransmission/announcer-http.cc

@@ -46,14 +46,13 @@ namespace
 {
 void verboseLog(std::string_view description, tr_direction direction, std::string_view message)
 {
-    auto& out = std::cerr;
+    if (static bool const verbose = tr_env_key_exists("TR_CURL_VERBOSE"); !verbose)
-    static bool const verbose = tr_env_key_exists("TR_CURL_VERBOSE");
-    if (!verbose)
     {
         return;
     }
 
     auto const direction_sv = direction == TR_DOWN ? "<< "sv : ">> "sv;
+    auto& out = std::cerr;
     out << description << '\n' << "[raw]"sv << direction_sv;
     for (unsigned char const ch : message)
     {

libtransmission/announcer.cc

@@ -1475,9 +1475,9 @@ int compareAnnounceTiers(tr_tier const* a, tr_tier const* b)
     }
 
     /* prefer swarms where we might download */
-    if (auto const is_done_a = a->tor->is_done(), is_done_b = b->tor->is_done(); is_done_a != is_done_b)
+    if (auto const val = tr_compare_3way(a->tor->is_done(), b->tor->is_done()); val != 0)
     {
-        return is_done_a ? 1 : -1;
+        return val;
     }
 
     /* prefer larger stats, to help ensure stats get recorded when stopping on shutdown */

libtransmission/cache.h

@@ -91,11 +91,11 @@ private:
 
     static constexpr struct
     {
-        [[nodiscard]] constexpr bool operator()(Key const& key, CacheBlock const& block)
+        [[nodiscard]] constexpr bool operator()(Key const& key, CacheBlock const& block) const
         {
             return key < block.key;
         }
-        [[nodiscard]] constexpr bool operator()(CacheBlock const& block, Key const& key)
+        [[nodiscard]] constexpr bool operator()(CacheBlock const& block, Key const& key) const
         {
             return block.key < key;
         }

libtransmission/crypto-utils-mbedtls.cc

@@ -229,7 +229,7 @@ bool tr_rand_buffer_crypto(void* buffer, size_t length)
     auto constexpr ChunkSize = size_t{ MBEDTLS_CTR_DRBG_MAX_REQUEST };
     static_assert(ChunkSize > 0U);
 
-    auto const lock = std::lock_guard(rng_mutex_);
+    auto const lock = std::scoped_lock{ rng_mutex_ };
 
     for (auto offset = size_t{ 0 }; offset < length; offset += ChunkSize)
     {

libtransmission/crypto-utils-wolfssl.cc

@@ -180,6 +180,6 @@ bool tr_rand_buffer_crypto(void* buffer, size_t length)
 
     TR_ASSERT(buffer != nullptr);
 
-    auto const lock = std::lock_guard(rng_mutex_);
+    auto const lock = std::lock_guard{ rng_mutex_ };
     return check_result(wc_RNG_GenerateBlock(get_rng(), static_cast<byte*>(buffer), length));
 }

libtransmission/favicon-cache.h

@@ -46,7 +46,7 @@ public:
 
     void load(
         std::string_view url_in,
-        IconFunc callback = [](Icon const&) {})
+        IconFunc callback = [](Icon const&) { /*default callback is a no-op */ })
     {
         std::call_once(scan_once_flag_, &FaviconCache::scan_file_cache, this);
 
@@ -99,8 +99,8 @@ public:
         }
     }
 
-    static inline constexpr auto Width = 16;
+    static constexpr auto Width = 16;
-    static inline constexpr auto Height = 16;
+    static constexpr auto Height = 16;
 
 private:
     class InFlightData
@@ -112,6 +112,9 @@ private:
         {
         }
 
+        InFlightData(InFlightData const&) = delete;
+        InFlightData& operator=(InFlightData const&) = delete;
+
         [[nodiscard]] constexpr auto const& sitename() const noexcept
         {
             return sitename_;
@@ -133,7 +136,7 @@ private:
 
         [[nodiscard]] auto get_responses()
         {
-            auto lock = std::lock_guard{ responses_mutex_ };
+            auto lock = std::scoped_lock{ responses_mutex_ };
 
             auto tmp = decltype(responses_){};
             std::swap(tmp, responses_);
@@ -142,7 +145,7 @@ private:
 
         void add_response(std::string contents, long code)
         {
-            auto lock = std::lock_guard{ responses_mutex_ };
+            auto lock = std::scoped_lock{ responses_mutex_ };
 
             responses_.emplace_back(std::move(contents), code);
         }
@@ -201,7 +204,7 @@ private:
         }
     }
 
-    void mark_site_as_scraped(std::string_view sitename)
+    void mark_site_as_scraped(std::string_view sitename) const
     {
         if (auto ofs = std::ofstream{ scraped_sitenames_filename_, std::ios_base::out | std::ios_base::app }; ofs.is_open())
         {

libtransmission/global-ip-cache.cc

@@ -207,7 +207,7 @@ bool tr_global_ip_cache::set_global_addr(tr_address_type type, tr_address const&
 {
     if (type == addr.type && addr.is_global_unicast_address())
     {
-        auto const lock = std::lock_guard{ global_addr_mutex_[addr.type] };
+        auto const lock = std::scoped_lock{ global_addr_mutex_[addr.type] };
         global_addr_[addr.type] = addr;
         tr_logAddTrace(fmt::format("Cached global address {}", addr.display_name()));
         return true;
@@ -339,21 +339,21 @@ void tr_global_ip_cache::on_response_ip_query(tr_address_type type, tr_web::Fetc
 
 void tr_global_ip_cache::unset_global_addr(tr_address_type type) noexcept
 {
-    auto const lock = std::lock_guard{ global_addr_mutex_[type] };
+    auto const lock = std::scoped_lock{ global_addr_mutex_[type] };
     global_addr_[type].reset();
     tr_logAddTrace(fmt::format("Unset {} global address cache", tr_ip_protocol_to_sv(type)));
 }
 
 void tr_global_ip_cache::set_source_addr(tr_address const& addr) noexcept
 {
-    auto const lock = std::lock_guard{ source_addr_mutex_[addr.type] };
+    auto const lock = std::scoped_lock{ source_addr_mutex_[addr.type] };
     source_addr_[addr.type] = addr;
     tr_logAddTrace(fmt::format("Cached source address {}", addr.display_name()));
 }
 
 void tr_global_ip_cache::unset_addr(tr_address_type type) noexcept
 {
-    auto const lock = std::lock_guard{ source_addr_mutex_[type] };
+    auto const lock = std::scoped_lock{ source_addr_mutex_[type] };
     source_addr_[type].reset();
     tr_logAddTrace(fmt::format("Unset {} source address cache", tr_ip_protocol_to_sv(type)));
 

libtransmission/lru-cache.h

@@ -77,7 +77,7 @@ private:
         uint64_t sequence_ = InvalidSeq;
     };
 
-    void erase(Entry& entry)
+    void erase(Entry& entry) const
     {
         entry.key_ = {};
         entry.val_ = {};

libtransmission/peer-common.h

@@ -183,7 +183,7 @@ class tr_peer
 public:
     using Speed = libtransmission::Values::Speed;
 
-    tr_peer(tr_torrent const* tor);
+    explicit tr_peer(tr_torrent const* tor);
     virtual ~tr_peer();
 
     [[nodiscard]] virtual Speed get_piece_speed(uint64_t now, tr_direction direction) const = 0;

libtransmission/peer-mgr-wishlist.cc

@@ -279,9 +279,10 @@ private:
             return;
         }
 
+        auto const pos_begin = std::begin(candidates_);
+
         // Candidate needs to be moved towards the front of the list
-        if (auto const pos_next = std::next(pos_old), pos_begin = std::begin(candidates_);
+        if (auto const pos_next = std::next(pos_old); pos_old > pos_begin && *pos_old < *std::prev(pos_old))
-            pos_old > pos_begin && *pos_old < *std::prev(pos_old))
         {
             auto const pos_new = std::lower_bound(pos_begin, pos_old, *pos_old);
             std::rotate(pos_new, pos_old, pos_next);

libtransmission/rpc-server.h

@@ -39,7 +39,6 @@ public:
     {
     public:
         Settings() = default;
-        virtual ~Settings() = default;
 
         explicit Settings(tr_variant const& src)
         {

libtransmission/session-alt-speeds.h

@@ -32,7 +32,6 @@ public:
     {
     public:
         Settings() = default;
-        virtual ~Settings() = default;
 
         explicit Settings(tr_variant const& src)
         {

libtransmission/session.h

@@ -355,7 +355,6 @@ public:
     {
     public:
         Settings() = default;
-        virtual ~Settings() = default;
 
         explicit Settings(tr_variant const& src)
         {

libtransmission/verify.cc

@@ -134,7 +134,7 @@ void tr_verify_worker::verify_thread_func()
     for (;;)
     {
         {
-            auto const lock = std::lock_guard{ verify_mutex_ };
+            auto const lock = std::scoped_lock{ verify_mutex_ };
 
             if (stop_current_)
             {
@@ -158,7 +158,7 @@ void tr_verify_worker::verify_thread_func()
 
 void tr_verify_worker::add(std::unique_ptr<Mediator> mediator, tr_priority_t priority)
 {
-    auto const lock = std::lock_guard{ verify_mutex_ };
+    auto const lock = std::scoped_lock{ verify_mutex_ };
 
     mediator->on_verify_queued();
     todo_.emplace(std::move(mediator), priority);
@@ -194,7 +194,7 @@ void tr_verify_worker::remove(tr_sha1_digest_t const& info_hash)
 tr_verify_worker::~tr_verify_worker()
 {
     {
-        auto const lock = std::lock_guard{ verify_mutex_ };
+        auto const lock = std::scoped_lock{ verify_mutex_ };
         stop_current_ = true;
         todo_.clear();
     }

qt/PrefsDialog.cc

@@ -470,13 +470,13 @@ void PrefsDialog::portTestSetEnabled()
 
 void PrefsDialog::onPortTested(std::optional<bool> result, Session::PortTestIpProtocol ip_protocol)
 {
-    constexpr auto StatusFromResult = [](std::optional<bool> const result)
+    constexpr auto StatusFromResult = [](std::optional<bool> const res)
     {
-        if (!result)
+        if (!res)
         {
             return PORT_TEST_ERROR;
         }
-        if (!*result)
+        if (!*res)
         {
             return PORT_TEST_CLOSED;
         }