From d11cddc39a526ce4edcfb2a6909f3641d49383b2 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Sat, 17 Feb 2024 22:43:24 -0600 Subject: [PATCH] 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 | 17 +++++++++++++++-- libtransmission/announcer-http.cc | 5 ++--- libtransmission/announcer.cc | 4 ++-- libtransmission/cache.h | 4 ++-- libtransmission/crypto-utils-mbedtls.cc | 2 +- libtransmission/crypto-utils-wolfssl.cc | 2 +- libtransmission/favicon-cache.h | 15 +++++++++------ libtransmission/global-ip-cache.cc | 8 ++++---- libtransmission/lru-cache.h | 2 +- libtransmission/peer-common.h | 2 +- libtransmission/peer-mgr-wishlist.cc | 5 +++-- libtransmission/rpc-server.h | 1 - libtransmission/session-alt-speeds.h | 1 - libtransmission/session.h | 1 - libtransmission/verify.cc | 6 +++--- qt/PrefsDialog.cc | 6 +++--- 16 files changed, 47 insertions(+), 34 deletions(-) diff --git a/gtk/PrefsDialog.cc b/gtk/PrefsDialog.cc index 246c14718..c04990ad2 100644 --- a/gtk/PrefsDialog.cc +++ b/gtk/PrefsDialog.cc @@ -974,12 +974,25 @@ void NetworkPage::portTestSetSensitive() void NetworkPage::onPortTested(std::optional const result, Session::PortTestIpProtocol const ip_protocol) { + auto constexpr ResultToStatus = [](std::optional 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(); diff --git a/libtransmission/announcer-http.cc b/libtransmission/announcer-http.cc index b980f1c46..8c0ce7c40 100644 --- a/libtransmission/announcer-http.cc +++ b/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; - static bool const verbose = tr_env_key_exists("TR_CURL_VERBOSE"); - if (!verbose) + if (static bool const verbose = tr_env_key_exists("TR_CURL_VERBOSE"); !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) { diff --git a/libtransmission/announcer.cc b/libtransmission/announcer.cc index 086a0acb1..c536f11bf 100644 --- a/libtransmission/announcer.cc +++ b/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 */ diff --git a/libtransmission/cache.h b/libtransmission/cache.h index 1161afc74..48f47b402 100644 --- a/libtransmission/cache.h +++ b/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; } diff --git a/libtransmission/crypto-utils-mbedtls.cc b/libtransmission/crypto-utils-mbedtls.cc index 80ea9af82..9a4d4f4a8 100644 --- a/libtransmission/crypto-utils-mbedtls.cc +++ b/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) { diff --git a/libtransmission/crypto-utils-wolfssl.cc b/libtransmission/crypto-utils-wolfssl.cc index a5554bbbb..5f1b40108 100644 --- a/libtransmission/crypto-utils-wolfssl.cc +++ b/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(buffer), length)); } diff --git a/libtransmission/favicon-cache.h b/libtransmission/favicon-cache.h index b0bf0a243..9e65f9fcd 100644 --- a/libtransmission/favicon-cache.h +++ b/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 inline constexpr auto Height = 16; + static constexpr auto Width = 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()) { diff --git a/libtransmission/global-ip-cache.cc b/libtransmission/global-ip-cache.cc index 0f196e35c..195eda3ba 100644 --- a/libtransmission/global-ip-cache.cc +++ b/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))); diff --git a/libtransmission/lru-cache.h b/libtransmission/lru-cache.h index faf9ce61b..124e27280 100644 --- a/libtransmission/lru-cache.h +++ b/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_ = {}; diff --git a/libtransmission/peer-common.h b/libtransmission/peer-common.h index e7bf9cfe9..55ba2a754 100644 --- a/libtransmission/peer-common.h +++ b/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; diff --git a/libtransmission/peer-mgr-wishlist.cc b/libtransmission/peer-mgr-wishlist.cc index 5f06c08c8..9f2f747fa 100644 --- a/libtransmission/peer-mgr-wishlist.cc +++ b/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_); - pos_old > pos_begin && *pos_old < *std::prev(pos_old)) + if (auto const pos_next = std::next(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); diff --git a/libtransmission/rpc-server.h b/libtransmission/rpc-server.h index 1267f2abd..18650a36e 100644 --- a/libtransmission/rpc-server.h +++ b/libtransmission/rpc-server.h @@ -39,7 +39,6 @@ public: { public: Settings() = default; - virtual ~Settings() = default; explicit Settings(tr_variant const& src) { diff --git a/libtransmission/session-alt-speeds.h b/libtransmission/session-alt-speeds.h index b0f821717..1671b8e74 100644 --- a/libtransmission/session-alt-speeds.h +++ b/libtransmission/session-alt-speeds.h @@ -32,7 +32,6 @@ public: { public: Settings() = default; - virtual ~Settings() = default; explicit Settings(tr_variant const& src) { diff --git a/libtransmission/session.h b/libtransmission/session.h index ee0a270f6..7f0c7a00c 100644 --- a/libtransmission/session.h +++ b/libtransmission/session.h @@ -355,7 +355,6 @@ public: { public: Settings() = default; - virtual ~Settings() = default; explicit Settings(tr_variant const& src) { diff --git a/libtransmission/verify.cc b/libtransmission/verify.cc index ac20b1ed6..78a134658 100644 --- a/libtransmission/verify.cc +++ b/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, 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(); } diff --git a/qt/PrefsDialog.cc b/qt/PrefsDialog.cc index 29616554a..36af243ee 100644 --- a/qt/PrefsDialog.cc +++ b/qt/PrefsDialog.cc @@ -470,13 +470,13 @@ void PrefsDialog::portTestSetEnabled() void PrefsDialog::onPortTested(std::optional result, Session::PortTestIpProtocol ip_protocol) { - constexpr auto StatusFromResult = [](std::optional const result) + constexpr auto StatusFromResult = [](std::optional const res) { - if (!result) + if (!res) { return PORT_TEST_ERROR; } - if (!*result) + if (!*res) { return PORT_TEST_CLOSED; }