Fix issues reported by clang-tidy misc checks (GTK client) (#4167)

* Fix `misc-const-correctness` clang-tidy issues

* Fix `misc-no-recursion` clang-tidy issues

* Extend clang-tidy configuration

gtk/.clang-tidy

@@ -5,6 +5,7 @@ Checks: >
   -cppcoreguidelines-avoid-magic-numbers,
   -cppcoreguidelines-avoid-non-const-global-variables,
   -cppcoreguidelines-narrowing-conversions,
+  misc-*,
   modernize-*,
   -modernize-use-trailing-return-type,
   performance-*,
@@ -15,4 +16,5 @@ Checks: >
   -readability-redundant-access-specifiers
 
 CheckOptions:
+  misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic: true
   modernize-pass-by-value.ValuesOnly: true

gtk/FileList.cc

@@ -7,6 +7,7 @@
 #include <cstddef>
 #include <list>
 #include <memory>
+#include <stack>
 #include <string>
 #include <string_view>
 #include <unordered_map>
@@ -91,6 +92,7 @@ public:
     TR_DISABLE_COPY_MOVE(Impl)
 
     void set_torrent(tr_torrent_id_t torrent_id);
+    void reset_torrent();
 
 private:
     void clearData();
@@ -269,26 +271,34 @@ bool refreshFilesForeach(
     return false; /* keep walking */
 }
 
-void gtr_tree_model_foreach_postorder_subtree(
-    Gtk::TreeModel::iterator const& parent,
-    Gtk::TreeModel::SlotForeachIter const& func)
-{
-    for (auto& child : parent->children())
-    {
-        gtr_tree_model_foreach_postorder_subtree(TR_GTK_TREE_MODEL_CHILD_ITER(child), func);
-    }
-
-    if (parent)
-    {
-        func(parent);
-    }
-}
-
 void gtr_tree_model_foreach_postorder(Glib::RefPtr<Gtk::TreeModel> const& model, Gtk::TreeModel::SlotForeachIter const& func)
 {
-    for (auto& iter : model->children())
+    auto items = std::stack<Gtk::TreeModel::iterator>();
+    if (auto const root_child_it = model->children().begin(); root_child_it)
     {
-        gtr_tree_model_foreach_postorder_subtree(TR_GTK_TREE_MODEL_CHILD_ITER(iter), func);
+        items.push(root_child_it);
+    }
+
+    while (!items.empty())
+    {
+        while (items.top())
+        {
+            if (auto const child_it = items.top()->children().begin(); child_it)
+            {
+                items.push(child_it);
+            }
+            else
+            {
+                func(items.top()++);
+            }
+        }
+
+        items.pop();
+
+        if (!items.empty())
+        {
+            func(items.top()++);
+        }
     }
 }
 
@@ -418,7 +428,7 @@ std::vector<tr_file_index_t> FileList::Impl::getActiveFilesForPath(Gtk::TreeMode
 
 void FileList::clear()
 {
-    impl_->set_torrent(-1);
+    impl_->reset_torrent();
 }
 
 namespace
@@ -566,6 +576,14 @@ void FileList::Impl::set_torrent(tr_torrent_id_t torrent_id)
     // view_->expand_all();
 }
 
+void FileList::Impl::reset_torrent()
+{
+    clearData();
+
+    store_ = Gtk::TreeStore::create(file_cols);
+    view_->set_model(store_);
+}
+
 /***
 ****
 ***/

gtk/Prefs.cc

@@ -85,7 +85,7 @@ static void tr_prefs_init_defaults(tr_variant* d)
 
 static void ensure_sound_cmd_is_a_list(tr_variant* dict)
 {
-    tr_quark key = TR_KEY_torrent_complete_sound_command;
+    tr_quark const key = TR_KEY_torrent_complete_sound_command;
     tr_variant* list = nullptr;
     if (tr_variantDictFindList(dict, key, &list))
     {

gtk/Session.cc

@@ -942,7 +942,7 @@ bool is_torrent_active(tr_stat const* st)
 
 void Session::add_torrent(tr_torrent* tor, bool do_notify)
 {
-    ScopedModelSortBlocker disable_sort(*impl_->get_model().get());
+    ScopedModelSortBlocker const disable_sort(*impl_->get_model().get());
     impl_->add_torrent(tor, do_notify);
 }
 
@@ -1032,7 +1032,7 @@ int Session::Impl::add_ctor(tr_ctor* ctor, bool do_prompt, bool do_notify)
 
     if (!do_prompt)
     {
-        ScopedModelSortBlocker disable_sort(*sorted_model_.get());
+        ScopedModelSortBlocker const disable_sort(*sorted_model_.get());
         add_torrent(create_new_torrent(ctor), do_notify);
         tr_ctorFree(ctor);
         return 0;
@@ -1260,7 +1260,7 @@ void Session::load(bool force_paused)
     auto const n_torrents = tr_sessionLoadTorrents(session, ctor);
     tr_ctorFree(ctor);
 
-    ScopedModelSortBlocker disable_sort(*impl_->get_model().get());
+    ScopedModelSortBlocker const disable_sort(*impl_->get_model().get());
 
     auto torrents = std::vector<tr_torrent*>{};
     torrents.resize(n_torrents);

gtk/Utils.cc

@@ -6,6 +6,7 @@
 #include <array>
 #include <functional>
 #include <memory>
+#include <stack>
 #include <stdexcept>
 #include <utility>
 
@@ -585,45 +586,54 @@ auto const ChildHiddenKey = Glib::Quark("gtr-child-hidden");
 
 } // namespace
 
-void gtr_widget_set_visible(Gtk::Widget& w, bool b)
+void gtr_widget_set_visible(Gtk::Widget& widget, bool is_visible)
 {
-    /* toggle the transient children, too */
+    auto* const widget_as_window = dynamic_cast<Gtk::Window*>(&widget);
-    if (auto const* const window = dynamic_cast<Gtk::Window*>(&w); window != nullptr)
+    if (widget_as_window == nullptr)
     {
-        auto top_levels = Gtk::Window::list_toplevels();
+        widget.set_visible(is_visible);
-
+        return;
-        for (auto top_level_it = top_levels.begin(); top_level_it != top_levels.end();)
-        {
-            auto* const l = *top_level_it++;
-
-            if (l->get_transient_for() != window)
-            {
-                continue;
-            }
-
-            if (l->get_visible() == b)
-            {
-                continue;
-            }
-
-            if (b && l->get_data(ChildHiddenKey) != nullptr)
-            {
-                l->steal_data(ChildHiddenKey);
-                gtr_widget_set_visible(*l, true);
-            }
-            else if (!b)
-            {
-                l->set_data(ChildHiddenKey, GINT_TO_POINTER(1));
-                gtr_widget_set_visible(*l, false);
-
-                // Retrieve updated top-levels list in case hiding the window resulted in its destruction
-                top_levels = Gtk::Window::list_toplevels();
-                top_level_it = top_levels.begin();
-            }
-        }
     }
 
-    w.set_visible(b);
+    /* toggle the transient children, too */
+    auto windows = std::stack<Gtk::Window*>();
+    windows.push(widget_as_window);
+
+    while (!windows.empty())
+    {
+        auto* const window = windows.top();
+        bool transient_child_found = false;
+
+        for (auto* const top_level_window : Gtk::Window::list_toplevels())
+        {
+            if (top_level_window->get_transient_for() != window || top_level_window->get_visible() == is_visible)
+            {
+                continue;
+            }
+
+            windows.push(top_level_window);
+            transient_child_found = true;
+            break;
+        }
+
+        if (transient_child_found)
+        {
+            continue;
+        }
+
+        if (is_visible && window->get_data(ChildHiddenKey) != nullptr)
+        {
+            window->steal_data(ChildHiddenKey);
+            window->set_visible(true);
+        }
+        else if (!is_visible)
+        {
+            window->set_data(ChildHiddenKey, GINT_TO_POINTER(1));
+            window->set_visible(false);
+        }
+
+        windows.pop();
+    }
 }
 
 Gtk::Window& gtr_widget_get_window(Gtk::Widget& widget)

gtk/Utils.h

@@ -177,7 +177,7 @@ Glib::ustring gtr_get_help_uri();
 ***/
 
 /* backwards-compatible wrapper around gtk_widget_set_visible() */
-void gtr_widget_set_visible(Gtk::Widget&, bool);
+void gtr_widget_set_visible(Gtk::Widget& widget, bool is_visible);
 
 Gtk::Window& gtr_widget_get_window(Gtk::Widget& widget);