From b94c6796c872d42f3cae2a245c04d7d20bddffc0 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Fri, 25 Mar 2022 00:24:04 -0500 Subject: [PATCH] fix: sonarcloud warnings (#2815) * fix: add default case to switch statement * fix: remove redundant static specifier * fix: use std::optional.value_or * fix: make type of variable pointer-to-const * refactor: move log state into a struct * refactor: make tr_peerMgr constructor explicit * fix: make type of variable pointer-to-const * fix: replace insert with try_emplace * fix: implicit conversion may lose precision * fix: use init-statement to reduce variable scope * chore: mark unused return value with (void) --- gtk/MessageLogWindow.cc | 3 +- gtk/PrefsDialog.cc | 2 +- gtk/TorrentCellRenderer.cc | 15 ++------- libtransmission/file-posix.cc | 6 ++-- libtransmission/log.cc | 62 +++++++++++++++++++++-------------- libtransmission/peer-mgr.cc | 8 ++--- libtransmission/rpcimpl.cc | 9 ++--- libtransmission/webseed.cc | 5 ++- 8 files changed, 55 insertions(+), 55 deletions(-) diff --git a/gtk/MessageLogWindow.cc b/gtk/MessageLogWindow.cc index 85cb45b23..7b6378d94 100644 --- a/gtk/MessageLogWindow.cc +++ b/gtk/MessageLogWindow.cc @@ -271,8 +271,7 @@ void setForegroundColor(Gtk::CellRendererText* renderer, tr_log_level level) renderer->property_foreground() = "forestgreen"; break; - case TR_LOG_INFO: - case TR_LOG_OFF: + default: renderer->property_foreground_set() = false; break; } diff --git a/gtk/PrefsDialog.cc b/gtk/PrefsDialog.cc index 7c1f973be..54af41d17 100644 --- a/gtk/PrefsDialog.cc +++ b/gtk/PrefsDialog.cc @@ -849,7 +849,7 @@ Gtk::ComboBox* new_time_combo(Glib::RefPtr const& core, tr_quark const return w; } -static auto get_weekday_string(Glib::Date::Weekday weekday) +auto get_weekday_string(Glib::Date::Weekday weekday) { auto date = Glib::Date{}; date.set_time_current(); diff --git a/gtk/TorrentCellRenderer.cc b/gtk/TorrentCellRenderer.cc index 8bbcdc2bd..c9cd4e036 100644 --- a/gtk/TorrentCellRenderer.cc +++ b/gtk/TorrentCellRenderer.cc @@ -180,7 +180,7 @@ std::string getShortStatusString( } } -static std::optional getErrorString(tr_stat const* st) +std::optional getErrorString(tr_stat const* st) { switch (st->error) { @@ -198,7 +198,7 @@ static std::optional getErrorString(tr_stat const* st) } } -static auto getActivityString( +auto getActivityString( tr_torrent const* const tor, tr_stat const* const st, double const uploadSpeed_KBps, @@ -275,16 +275,7 @@ std::string getStatusString( double const uploadSpeed_KBps, double const downloadSpeed_KBps) { - auto status_str = std::string{}; - - if (auto error_string = getErrorString(st); error_string) - { - status_str = *error_string; - } - else - { - status_str = getActivityString(tor, st, uploadSpeed_KBps, downloadSpeed_KBps); - } + auto status_str = getErrorString(st).value_or(getActivityString(tor, st, uploadSpeed_KBps, downloadSpeed_KBps)); if (st->activity != TR_STATUS_CHECK_WAIT && st->activity != TR_STATUS_CHECK && st->activity != TR_STATUS_DOWNLOAD_WAIT && st->activity != TR_STATUS_SEED_WAIT && st->activity != TR_STATUS_STOPPED) diff --git a/libtransmission/file-posix.cc b/libtransmission/file-posix.cc index f91da7834..2e5ae010f 100644 --- a/libtransmission/file-posix.cc +++ b/libtransmission/file-posix.cc @@ -387,9 +387,8 @@ char* tr_sys_path_resolve(char const* path, tr_error** error) std::string tr_sys_path_basename(std::string_view path, tr_error** error) { auto tmp = std::string{ path }; - char* ret = basename(std::data(tmp)); - if (ret != nullptr) + if (char const* ret = basename(std::data(tmp)); ret != nullptr) { return ret; } @@ -401,9 +400,8 @@ std::string tr_sys_path_basename(std::string_view path, tr_error** error) std::string tr_sys_path_dirname(std::string_view path, tr_error** error) { auto tmp = std::string{ path }; - char* ret = dirname(std::data(tmp)); - if (ret != nullptr) + if (char const* ret = dirname(std::data(tmp)); ret != nullptr) { return ret; } diff --git a/libtransmission/log.cc b/libtransmission/log.cc index 3aacbd532..741d7e030 100644 --- a/libtransmission/log.cc +++ b/libtransmission/log.cc @@ -33,17 +33,29 @@ using namespace std::literals; namespace { -tr_log_level tr_message_level = TR_LOG_ERROR; +class tr_log_state +{ +public: + [[nodiscard]] auto unique_lock() + { + return std::unique_lock(message_mutex_); + } -bool myQueueEnabled = false; + tr_log_level level = TR_LOG_ERROR; -tr_log_message* myQueue = nullptr; + bool queue_enabled_ = false; -tr_log_message** myQueueTail = &myQueue; + tr_log_message* queue_ = nullptr; -int myQueueLength = 0; + tr_log_message** queue_tail_ = &queue_; -std::recursive_mutex message_mutex_; + int queue_length_ = 0; + +private: + std::recursive_mutex message_mutex_; +}; + +auto log_state = tr_log_state{}; /// @@ -87,7 +99,7 @@ void logAddImpl( return; } - auto const lock = std::lock_guard(message_mutex_); + auto const lock = log_state.unique_lock(); #ifdef _WIN32 OutputDebugStringA(tr_strvJoin(msg, "\r\n").c_str()); @@ -135,18 +147,18 @@ void logAddImpl( newmsg->line = line; newmsg->name = tr_strvDup(name); - *myQueueTail = newmsg; - myQueueTail = &newmsg->next; - ++myQueueLength; + *log_state.queue_tail_ = newmsg; + log_state.queue_tail_ = &newmsg->next; + ++log_state.queue_length_; - if (myQueueLength > TR_LOG_MAX_QUEUE_LENGTH) + if (log_state.queue_length_ > TR_LOG_MAX_QUEUE_LENGTH) { - tr_log_message* old = myQueue; - myQueue = old->next; + tr_log_message* old = log_state.queue_; + log_state.queue_ = old->next; old->next = nullptr; tr_logFreeQueue(old); - --myQueueLength; - TR_ASSERT(myQueueLength == TR_LOG_MAX_QUEUE_LENGTH); + --log_state.queue_length_; + TR_ASSERT(log_state.queue_length_ == TR_LOG_MAX_QUEUE_LENGTH); } } else @@ -174,7 +186,7 @@ void logAddImpl( tr_log_level tr_logGetLevel() { - return tr_message_level; + return log_state.level; } bool tr_logLevelIsActive(tr_log_level level) @@ -184,27 +196,27 @@ bool tr_logLevelIsActive(tr_log_level level) void tr_logSetLevel(tr_log_level level) { - tr_message_level = level; + log_state.level = level; } void tr_logSetQueueEnabled(bool isEnabled) { - myQueueEnabled = isEnabled; + log_state.queue_enabled_ = isEnabled; } bool tr_logGetQueueEnabled() { - return myQueueEnabled; + return log_state.queue_enabled_; } tr_log_message* tr_logGetQueue() { - auto const lock = std::lock_guard(message_mutex_); + auto const lock = log_state.unique_lock(); - auto* const ret = myQueue; - myQueue = nullptr; - myQueueTail = &myQueue; - myQueueLength = 0; + auto* const ret = log_state.queue_; + log_state.queue_ = nullptr; + log_state.queue_tail_ = &log_state.queue_; + log_state.queue_length_ = 0; return ret; } @@ -264,7 +276,7 @@ void tr_logAddMessage(char const* file, int line, tr_log_level level, std::strin return; } - auto const lock = std::lock_guard(message_mutex_); + auto const lock = log_state.unique_lock(); // don't log the same warning ad infinitum. // it's not useful after some point. diff --git a/libtransmission/peer-mgr.cc b/libtransmission/peer-mgr.cc index f7d6ecfb7..98c8985d2 100644 --- a/libtransmission/peer-mgr.cc +++ b/libtransmission/peer-mgr.cc @@ -209,7 +209,7 @@ public: struct tr_peerMgr { - tr_peerMgr(tr_session* session_in) + explicit tr_peerMgr(tr_session* session_in) : session{ session_in } { } @@ -315,7 +315,7 @@ static struct peer_atom* getExistingAtom(tr_swarm const* cswarm, tr_address cons static bool peerIsInUse(tr_swarm const* cs, struct peer_atom const* atom) { - auto* s = const_cast(cs); + auto const* const s = const_cast(cs); auto const lock = s->manager->unique_lock(); return atom->peer != nullptr || s->outgoing_handshakes.count(atom->addr) != 0 || @@ -1105,7 +1105,7 @@ void tr_peerMgrAddIncoming(tr_peerMgr* manager, tr_address const* addr, tr_port tr_peerIoUnref(io); /* balanced by the implicit ref in tr_peerIoNewIncoming() */ - manager->incoming_handshakes.insert({ *addr, handshake }); + manager->incoming_handshakes.try_emplace(*addr, handshake); } } @@ -3048,7 +3048,7 @@ static void initiateConnection(tr_peerMgr* mgr, tr_swarm* s, struct peer_atom* a tr_peerIoUnref(io); /* balanced by the initial ref in tr_peerIoNewOutgoing() */ - s->outgoing_handshakes.insert({ atom->addr, handshake }); + s->outgoing_handshakes.try_emplace(atom->addr, handshake); } atom->lastConnectionAttemptAt = now; diff --git a/libtransmission/rpcimpl.cc b/libtransmission/rpcimpl.cc index 74fac37fb..9a968457e 100644 --- a/libtransmission/rpcimpl.cc +++ b/libtransmission/rpcimpl.cc @@ -1714,10 +1714,11 @@ static char const* groupGet(tr_session* s, tr_variant* args_in, tr_variant* args } else if (tr_variant* namesList = nullptr; tr_variantDictFindList(args_in, TR_KEY_name, &namesList)) { - int names_count = tr_variantListSize(namesList); - for (int i = 0; i < names_count; i++) + auto const names_count = tr_variantListSize(namesList); + + for (size_t i = 0; i < names_count; ++i) { - tr_variant* v = tr_variantListChild(namesList, i); + auto const* const v = tr_variantListChild(namesList, i); if (std::string_view l; tr_variantIsString(v) && tr_variantGetStrView(v, &l)) { names.insert(l); @@ -1751,7 +1752,7 @@ static char const* groupSet( struct tr_rpc_idle_data* /*idle_data*/) { auto name = std::string_view{}; - tr_variantDictFindStrView(args_in, TR_KEY_name, &name); + (void)tr_variantDictFindStrView(args_in, TR_KEY_name, &name); name = tr_strvStrip(name); if (std::empty(name)) { diff --git a/libtransmission/webseed.cc b/libtransmission/webseed.cc index 9166a6d6d..826a1266e 100644 --- a/libtransmission/webseed.cc +++ b/libtransmission/webseed.cc @@ -369,7 +369,7 @@ void onBufferGotData(evbuffer* /*buf*/, evbuffer_cb_info const* info, void* vtas return; } - auto* const session = task->session; + auto const* const session = task->session; auto const lock = session->unique_lock(); auto* const webseed = task->webseed; @@ -427,8 +427,7 @@ void onPartialDataFetched(tr_web::FetchResponse const& web_response) return; } - auto* const tor = webseed->getTorrent(); - if (tor == nullptr) + if (auto const* const tor = webseed->getTorrent(); tor == nullptr) { return; }