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

@@ -271,8 +271,7 @@ void setForegroundColor(Gtk::CellRendererText* renderer, tr_log_level level)
         renderer->property_foreground() = "forestgreen";
         break;
 
-    case TR_LOG_INFO:
+    default:
-    case TR_LOG_OFF:
         renderer->property_foreground_set() = false;
         break;
     }

gtk/PrefsDialog.cc

@@ -849,7 +849,7 @@ Gtk::ComboBox* new_time_combo(Glib::RefPtr<Session> 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();

gtk/TorrentCellRenderer.cc

@@ -180,7 +180,7 @@ std::string getShortStatusString(
     }
 }
 
-static std::optional<std::string> getErrorString(tr_stat const* st)
+std::optional<std::string> getErrorString(tr_stat const* st)
 {
     switch (st->error)
     {
@@ -198,7 +198,7 @@ static std::optional<std::string> 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{};
+    auto status_str = getErrorString(st).value_or(getActivityString(tor, st, uploadSpeed_KBps, downloadSpeed_KBps));
-
-    if (auto error_string = getErrorString(st); error_string)
-    {
-        status_str = *error_string;
-    }
-    else
-    {
-        status_str = 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)

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;
     }

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;
+        *log_state.queue_tail_ = newmsg;
-        myQueueTail = &newmsg->next;
+        log_state.queue_tail_ = &newmsg->next;
-        ++myQueueLength;
+        ++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;
+            tr_log_message* old = log_state.queue_;
-            myQueue = old->next;
+            log_state.queue_ = old->next;
             old->next = nullptr;
             tr_logFreeQueue(old);
-            --myQueueLength;
+            --log_state.queue_length_;
-            TR_ASSERT(myQueueLength == TR_LOG_MAX_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;
+    auto* const ret = log_state.queue_;
-    myQueue = nullptr;
+    log_state.queue_ = nullptr;
-    myQueueTail = &myQueue;
+    log_state.queue_tail_ = &log_state.queue_;
-    myQueueLength = 0;
+    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.

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<tr_swarm*>(cs);
+    auto const* const s = const_cast<tr_swarm*>(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;

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);
+        auto const names_count = tr_variantListSize(namesList);
-        for (int i = 0; i < names_count; i++)
+
+        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))
     {

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 (auto const* const tor = webseed->getTorrent(); tor == nullptr)
-    if (tor == nullptr)
     {
         return;
     }