From 92b74fee742822841a56b1b7ab835653c91c997c Mon Sep 17 00:00:00 2001 From: Mike Gelfand Date: Thu, 10 Nov 2022 20:35:31 +0100 Subject: [PATCH] Fix issues reported by clang-tidy `modernize` checks (GTK client) (#4137) * Fix `modernize-avoid-c-arrays` clang-tidy issues * Fix `modernize-raw-string-literal` clang-tidy issues * Fix `modernize-use-nodiscard` clang-tidy issues * Fix `modernize-deprecated-headers` clang-tidy issues * Fix `modernize-pass-by-value` clang-tidy issues * Extend clang-tidy configuration Enable all `modernize` checks except for one (use-trailing-return-type) which is an extensive stylistic change for later. --- gtk/.clang-tidy | 5 +++++ gtk/Application.cc | 10 ++++------ gtk/DetailsDialog.cc | 41 +++++++++++++++++++++----------------- gtk/FileList.cc | 7 +++---- gtk/FilterBar.cc | 11 ++++++---- gtk/MainWindow.cc | 22 +++++++++++--------- gtk/MessageLogWindow.cc | 12 +++++------ gtk/Prefs.cc | 1 - gtk/PrefsDialog.cc | 18 ++++++++--------- gtk/TorrentCellRenderer.cc | 1 - gtk/Utils.cc | 3 --- 11 files changed, 70 insertions(+), 61 deletions(-) diff --git a/gtk/.clang-tidy b/gtk/.clang-tidy index a43de824a..9cd1c64db 100644 --- a/gtk/.clang-tidy +++ b/gtk/.clang-tidy @@ -1,8 +1,13 @@ --- Checks: > -*, + modernize-*, + -modernize-use-trailing-return-type, readability-*, -readability-function-cognitive-complexity, -readability-identifier-length, -readability-magic-numbers, -readability-redundant-access-specifiers + +CheckOptions: + modernize-pass-by-value.ValuesOnly: true diff --git a/gtk/Application.cc b/gtk/Application.cc index 0fe1a2660..264119390 100644 --- a/gtk/Application.cc +++ b/gtk/Application.cc @@ -3,6 +3,7 @@ // License text can be found in the licenses/ folder. #include +#include #include // exit() #include #include // std::back_inserter @@ -15,9 +16,6 @@ #include #include -#include -#include - #include #include @@ -157,9 +155,9 @@ private: void on_add_torrent(tr_ctor* ctor); void on_prefs_changed(tr_quark key); - std::vector get_selected_torrent_ids() const; - tr_torrent* get_first_selected_torrent() const; - counts_data get_selected_torrent_counts() const; + [[nodiscard]] std::vector get_selected_torrent_ids() const; + [[nodiscard]] tr_torrent* get_first_selected_torrent() const; + [[nodiscard]] counts_data get_selected_torrent_counts() const; void start_all_torrents(); void pause_all_torrents(); diff --git a/gtk/DetailsDialog.cc b/gtk/DetailsDialog.cc index 25b64567d..2fb3688b4 100644 --- a/gtk/DetailsDialog.cc +++ b/gtk/DetailsDialog.cc @@ -12,13 +12,12 @@ #include #include -#include // INT_MAX +#include +#include // abort() +#include #include #include #include -#include -#include // sscanf() -#include // abort() #include #include #include @@ -470,7 +469,7 @@ void DetailsDialog::Impl::options_page_init(Glib::RefPtr const& /* down_limited_check_tag_ = down_limited_check_->signal_toggled().connect( [this]() { torrent_set_bool(TR_KEY_downloadLimited, down_limited_check_->get_active()); }); - down_limit_spin_->set_adjustment(Gtk::Adjustment::create(0, 0, INT_MAX, 5)); + down_limit_spin_->set_adjustment(Gtk::Adjustment::create(0, 0, std::numeric_limits::max(), 5)); down_limit_spin_tag_ = down_limit_spin_->signal_value_changed().connect( [this]() { torrent_set_int(TR_KEY_downloadLimit, down_limit_spin_->get_value_as_int()); }); @@ -478,7 +477,7 @@ void DetailsDialog::Impl::options_page_init(Glib::RefPtr const& /* up_limited_check_tag_ = up_limited_check_->signal_toggled().connect( [this]() { torrent_set_bool(TR_KEY_uploadLimited, up_limited_check_->get_active()); }); - up_limit_sping_->set_adjustment(Gtk::Adjustment::create(0, 0, INT_MAX, 5)); + up_limit_sping_->set_adjustment(Gtk::Adjustment::create(0, 0, std::numeric_limits::max(), 5)); up_limit_spin_tag_ = up_limit_sping_->signal_value_changed().connect( [this]() { torrent_set_int(TR_KEY_uploadLimit, up_limit_sping_->get_value_as_int()); }); @@ -1680,7 +1679,7 @@ void setPeerViewColumns(Gtk::TreeView* peer_view) } else { - abort(); + std::abort(); } c->set_resizable(false); @@ -1767,11 +1766,11 @@ void DetailsDialog::Impl::peer_page_init(Glib::RefPtr const& build namespace { -auto constexpr ErrMarkupBegin = ""sv; +auto constexpr ErrMarkupBegin = ""sv; auto constexpr ErrMarkupEnd = ""sv; -auto constexpr TimeoutMarkupBegin = ""sv; +auto constexpr TimeoutMarkupBegin = ""sv; auto constexpr TimeoutMarkupEnd = ""sv; -auto constexpr SuccessMarkupBegin = ""sv; +auto constexpr SuccessMarkupBegin = ""sv; auto constexpr SuccessMarkupEnd = ""sv; std::array const text_dir_mark = { ""sv, "\u200E"sv, "\u200F"sv }; @@ -2166,12 +2165,15 @@ public: BaseObjectType* cast_item, Glib::RefPtr const& builder, DetailsDialog& parent, - Glib::RefPtr core, + Glib::RefPtr const& core, tr_torrent const* torrent); TR_DISABLE_COPY_MOVE(EditTrackersDialog) - static std::unique_ptr create(DetailsDialog& parent, Glib::RefPtr core, tr_torrent const* tor); + static std::unique_ptr create( + DetailsDialog& parent, + Glib::RefPtr const& core, + tr_torrent const* tor); private: void on_response(int response) override; @@ -2187,7 +2189,7 @@ EditTrackersDialog::EditTrackersDialog( BaseObjectType* cast_item, Glib::RefPtr const& builder, DetailsDialog& parent, - Glib::RefPtr core, + Glib::RefPtr const& core, tr_torrent const* torrent) : Gtk::Dialog(cast_item) , parent_(parent) @@ -2203,7 +2205,7 @@ EditTrackersDialog::EditTrackersDialog( std::unique_ptr EditTrackersDialog::create( DetailsDialog& parent, - Glib::RefPtr core, + Glib::RefPtr const& core, tr_torrent const* torrent) { auto const builder = Gtk::Builder::create_from_resource(gtr_get_full_resource_path("EditTrackersDialog.ui")); @@ -2281,12 +2283,15 @@ public: BaseObjectType* cast_item, Glib::RefPtr const& builder, DetailsDialog& parent, - Glib::RefPtr core, + Glib::RefPtr const& core, tr_torrent const* torrent); TR_DISABLE_COPY_MOVE(AddTrackerDialog) - static std::unique_ptr create(DetailsDialog& parent, Glib::RefPtr core, tr_torrent const* tor); + static std::unique_ptr create( + DetailsDialog& parent, + Glib::RefPtr const& core, + tr_torrent const* tor); private: void on_response(int response) override; @@ -2302,7 +2307,7 @@ AddTrackerDialog::AddTrackerDialog( BaseObjectType* cast_item, Glib::RefPtr const& builder, DetailsDialog& parent, - Glib::RefPtr core, + Glib::RefPtr const& core, tr_torrent const* torrent) : Gtk::Dialog(cast_item) , parent_(parent) @@ -2318,7 +2323,7 @@ AddTrackerDialog::AddTrackerDialog( std::unique_ptr AddTrackerDialog::create( DetailsDialog& parent, - Glib::RefPtr core, + Glib::RefPtr const& core, tr_torrent const* torrent) { auto const builder = Gtk::Builder::create_from_resource(gtr_get_full_resource_path("AddTrackerDialog.ui")); diff --git a/gtk/FileList.cc b/gtk/FileList.cc index 330d5f6aa..71493615a 100644 --- a/gtk/FileList.cc +++ b/gtk/FileList.cc @@ -4,7 +4,6 @@ // License text can be found in the licenses/ folder. #include -#include // INT_MAX #include #include #include @@ -99,9 +98,9 @@ private: bool getAndSelectEventPath(double view_x, double view_y, Gtk::TreeViewColumn*& col, Gtk::TreeModel::Path& path); - std::vector getActiveFilesForPath(Gtk::TreeModel::Path const& path) const; - std::vector getSelectedFilesAndDescendants() const; - std::vector getSubtree(Gtk::TreeModel::Path const& path) const; + [[nodiscard]] std::vector getActiveFilesForPath(Gtk::TreeModel::Path const& path) const; + [[nodiscard]] std::vector getSelectedFilesAndDescendants() const; + [[nodiscard]] std::vector getSubtree(Gtk::TreeModel::Path const& path) const; bool onViewButtonPressed(guint button, TrGdkModifierType state, double view_x, double view_y); bool onViewPathToggled(Gtk::TreeViewColumn* col, Gtk::TreeModel::Path const& path); diff --git a/gtk/FilterBar.cc b/gtk/FilterBar.cc index 6616d3e78..c17696352 100644 --- a/gtk/FilterBar.cc +++ b/gtk/FilterBar.cc @@ -4,6 +4,7 @@ // License text can be found in the licenses/ folder. #include // std::transform() +#include #include #include #include @@ -37,7 +38,7 @@ public: TR_DISABLE_COPY_MOVE(Impl) - Glib::RefPtr get_filter_model() const; + [[nodiscard]] Glib::RefPtr get_filter_model() const; private: template @@ -510,13 +511,15 @@ bool activity_filter_model_update(Glib::RefPtr const& activity_m Glib::RefPtr activity_filter_model_new(Glib::RefPtr const& tmodel) { - static struct + struct FilterTypeInfo { int type; char const* context; char const* name; Glib::ustring icon_name; - } const types[] = { + }; + + static auto const types = std::array({ { { ACTIVITY_FILTER_ALL, nullptr, N_("All"), {} }, { ACTIVITY_FILTER_SEPARATOR, nullptr, nullptr, {} }, { ACTIVITY_FILTER_ACTIVE, nullptr, N_("Active"), "system-run" }, @@ -526,7 +529,7 @@ Glib::RefPtr activity_filter_model_new(Glib::RefPtr get_selection() const; + [[nodiscard]] Glib::RefPtr get_selection() const; void refresh(); @@ -65,8 +65,8 @@ private: void init_view(Gtk::TreeView* view, Glib::RefPtr const& model); Glib::RefPtr createOptionsMenu(); - Glib::RefPtr createSpeedMenu(Glib::RefPtr actions, tr_direction dir); - Glib::RefPtr createRatioMenu(Glib::RefPtr actions); + Glib::RefPtr createSpeedMenu(Glib::RefPtr const& actions, tr_direction dir); + Glib::RefPtr createRatioMenu(Glib::RefPtr const& actions); Glib::RefPtr createStatsMenu(); @@ -292,7 +292,9 @@ void MainWindow::Impl::onSpeedSet(tr_direction dir, int KBps) core_->set_pref(dir == TR_UP ? TR_KEY_speed_limit_up_enabled : TR_KEY_speed_limit_down_enabled, true); } -Glib::RefPtr MainWindow::Impl::createSpeedMenu(Glib::RefPtr actions, tr_direction dir) +Glib::RefPtr MainWindow::Impl::createSpeedMenu( + Glib::RefPtr const& actions, + tr_direction dir) { auto& info = speed_menu_info_[dir]; @@ -353,9 +355,9 @@ void MainWindow::Impl::onRatioSet(double ratio) core_->set_pref(TR_KEY_ratio_limit_enabled, true); } -Glib::RefPtr MainWindow::Impl::createRatioMenu(Glib::RefPtr actions) +Glib::RefPtr MainWindow::Impl::createRatioMenu(Glib::RefPtr const& actions) { - static double const stockRatios[] = { 0.25, 0.5, 0.75, 1, 1.5, 2, 3 }; + static auto const stockRatios = std::array({ 0.25, 0.5, 0.75, 1, 1.5, 2, 3 }); auto& info = ratio_menu_info_; @@ -459,16 +461,18 @@ void MainWindow::Impl::onOptionsClicked() Glib::RefPtr MainWindow::Impl::createStatsMenu() { - static struct + struct StatsModeInfo { char const* val; char const* i18n; - } const stats_modes[] = { + }; + + static auto const stats_modes = std::array({ { { "total-ratio", N_("Total Ratio") }, { "session-ratio", N_("Session Ratio") }, { "total-transfer", N_("Total Transfer") }, { "session-transfer", N_("Session Transfer") }, - }; + } }); auto top = Gio::Menu::create(); auto actions = Gio::SimpleActionGroup::create(); diff --git a/gtk/MessageLogWindow.cc b/gtk/MessageLogWindow.cc index 71ef5919d..9f743b2f5 100644 --- a/gtk/MessageLogWindow.cc +++ b/gtk/MessageLogWindow.cc @@ -3,8 +3,8 @@ // or any future license endorsed by Mnemosyne LLC. // License text can be found in the licenses/ folder. -#include -#include +#include +#include #include #include @@ -64,8 +64,8 @@ private: void level_combo_changed_cb(Gtk::ComboBox* combo_box); void level_combo_init(Gtk::ComboBox* level_combo) const; - bool is_pinned_to_new() const; - bool isRowVisible(Gtk::TreeModel::const_iterator const& iter) const; + [[nodiscard]] bool is_pinned_to_new() const; + [[nodiscard]] bool isRowVisible(Gtk::TreeModel::const_iterator const& iter) const; private: MessageLogWindow& window_; @@ -179,7 +179,7 @@ Glib::ustring gtr_asctime(time_t t) void MessageLogWindow::Impl::doSave(Gtk::Window& parent, Glib::ustring const& filename) { - auto* fp = fopen(filename.c_str(), "w+"); + auto* fp = std::fopen(filename.c_str(), "w+"); if (fp == nullptr) { @@ -211,7 +211,7 @@ void MessageLogWindow::Impl::doSave(Gtk::Window& parent, Glib::ustring const& fi fmt::print(fp, "{}\t{}\t{}\t{}\n", date, level_str, node->name, node->message); } - fclose(fp); + std::fclose(fp); } } diff --git a/gtk/Prefs.cc b/gtk/Prefs.cc index a580e8b85..b76ff5339 100644 --- a/gtk/Prefs.cc +++ b/gtk/Prefs.cc @@ -2,7 +2,6 @@ // It may be used under the MIT (SPDX: MIT) license. // License text can be found in the licenses/ folder. -#include #include #include diff --git a/gtk/PrefsDialog.cc b/gtk/PrefsDialog.cc index eef78e762..cf13498c4 100644 --- a/gtk/PrefsDialog.cc +++ b/gtk/PrefsDialog.cc @@ -3,7 +3,7 @@ // or any future license endorsed by Mnemosyne LLC. // License text can be found in the licenses/ folder. -#include /* USHRT_MAX, INT_MAX */ +#include #include #include #include @@ -274,7 +274,7 @@ DownloadingPage::DownloadingPage( TR_KEY_download_queue_size, core_, 0, - INT_MAX, + std::numeric_limits::max(), 1); init_spin_button( @@ -282,7 +282,7 @@ DownloadingPage::DownloadingPage( TR_KEY_queue_stalled_minutes, core_, 1, - INT_MAX, + std::numeric_limits::max(), 15); init_check_button( @@ -727,7 +727,7 @@ RemotePage::RemotePage(BaseObjectType* cast_item, Glib::RefPtr con /* port */ auto* port_spin = gtr_get_widget(builder, "rpc_port_spin"); - init_spin_button(*port_spin, TR_KEY_rpc_port, core_, 0, USHRT_MAX, 1); + init_spin_button(*port_spin, TR_KEY_rpc_port, core_, 0, std::numeric_limits::max(), 1); /* require authentication */ init_check_button(*auth_tb_, TR_KEY_rpc_authentication_required, core_); @@ -889,7 +889,7 @@ SpeedPage::SpeedPage(BaseObjectType* cast_item, Glib::RefPtr const w->set_label(fmt::format(w->get_label().raw(), fmt::arg("speed_units", speed_K_str))); auto* const w2 = gtr_get_widget(builder, "upload_limit_spin"); - init_spin_button(*w2, TR_KEY_speed_limit_up, core_, 0, INT_MAX, 5); + init_spin_button(*w2, TR_KEY_speed_limit_up, core_, 0, std::numeric_limits::max(), 5); } { @@ -898,7 +898,7 @@ SpeedPage::SpeedPage(BaseObjectType* cast_item, Glib::RefPtr const w->set_label(fmt::format(w->get_label().raw(), fmt::arg("speed_units", speed_K_str))); auto* const w2 = gtr_get_widget(builder, "download_limit_spin"); - init_spin_button(*w2, TR_KEY_speed_limit_down, core_, 0, INT_MAX, 5); + init_spin_button(*w2, TR_KEY_speed_limit_down, core_, 0, std::numeric_limits::max(), 5); } { @@ -906,7 +906,7 @@ SpeedPage::SpeedPage(BaseObjectType* cast_item, Glib::RefPtr const w->set_label(fmt::format(w->get_label().raw(), fmt::arg("speed_units", speed_K_str))); auto* const w2 = gtr_get_widget(builder, "alt_upload_limit_spin"); - init_spin_button(*w2, TR_KEY_alt_speed_up, core_, 0, INT_MAX, 5); + init_spin_button(*w2, TR_KEY_alt_speed_up, core_, 0, std::numeric_limits::max(), 5); } { @@ -914,7 +914,7 @@ SpeedPage::SpeedPage(BaseObjectType* cast_item, Glib::RefPtr const w->set_label(fmt::format(w->get_label().raw(), fmt::arg("speed_units", speed_K_str))); auto* const w2 = gtr_get_widget(builder, "alt_download_limit_spin"); - init_spin_button(*w2, TR_KEY_alt_speed_down, core_, 0, INT_MAX, 5); + init_spin_button(*w2, TR_KEY_alt_speed_down, core_, 0, std::numeric_limits::max(), 5); } { @@ -1022,7 +1022,7 @@ NetworkPage::NetworkPage( , portButton_(gtr_get_widget(builder, "test_listening_port_button")) , portSpin_(gtr_get_widget(builder, "listening_port_spin")) { - init_spin_button(*portSpin_, TR_KEY_peer_port, core_, 1, USHRT_MAX, 1); + init_spin_button(*portSpin_, TR_KEY_peer_port, core_, 1, std::numeric_limits::max(), 1); portButton_->signal_clicked().connect([this]() { onPortTest(); }); diff --git a/gtk/TorrentCellRenderer.cc b/gtk/TorrentCellRenderer.cc index e986aefe1..447ce2ee2 100644 --- a/gtk/TorrentCellRenderer.cc +++ b/gtk/TorrentCellRenderer.cc @@ -4,7 +4,6 @@ // License text can be found in the licenses/ folder. #include // std::max() -#include /* INT_MAX */ #include // strchr() #include #include diff --git a/gtk/Utils.cc b/gtk/Utils.cc index a94360176..92fc5a8fe 100644 --- a/gtk/Utils.cc +++ b/gtk/Utils.cc @@ -4,10 +4,7 @@ // License text can be found in the licenses/ folder. #include -#include /* isxdigit() */ -#include #include -#include /* INT_MAX */ #include #include