From c61f3d6a08ccfc24341bbcf96d66195b96c2f916 Mon Sep 17 00:00:00 2001 From: bad code <> Date: Sun, 22 Sep 2024 15:54:11 +0200 Subject: [PATCH 01/17] Avoid comma operator Simply split the expressions. --- src/btop.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/btop.cpp b/src/btop.cpp index cef761336..6f6b63c52 100644 --- a/src/btop.cpp +++ b/src/btop.cpp @@ -301,7 +301,8 @@ void term_resize(bool force) { } } min_size = Term::get_min_size(boxes); - minWidth = min_size.at(0), minHeight = min_size.at(1); + minWidth = min_size.at(0); + minHeight = min_size.at(1); } else if (not Term::refresh()) break; } From d9f44434f5d16c01efab560f014f2d508010de0e Mon Sep 17 00:00:00 2001 From: bad code <> Date: Sun, 22 Sep 2024 15:54:34 +0200 Subject: [PATCH 02/17] Add missing override annotations Help the compiler in finding mismatches. --- src/btop.cpp | 4 ++-- src/btop_tools.hpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/btop.cpp b/src/btop.cpp index 6f6b63c52..80783f02e 100644 --- a/src/btop.cpp +++ b/src/btop.cpp @@ -515,8 +515,8 @@ namespace Runner { class MyNumPunct : public std::numpunct { protected: - virtual char do_thousands_sep() const { return '\''; } - virtual std::string do_grouping() const { return "\03"; } + virtual char do_thousands_sep() const override { return '\''; } + virtual std::string do_grouping() const override { return "\03"; } }; diff --git a/src/btop_tools.hpp b/src/btop_tools.hpp index 07f7de334..a61bd76e0 100644 --- a/src/btop_tools.hpp +++ b/src/btop_tools.hpp @@ -188,8 +188,8 @@ namespace Tools { class MyNumPunct : public std::numpunct { protected: - virtual char do_thousands_sep() const { return '\''; } - virtual std::string do_grouping() const { return "\03"; } + virtual char do_thousands_sep() const override { return '\''; } + virtual std::string do_grouping() const override { return "\03"; } }; size_t wide_ulen(const string& str); From cfb96a35f1ca9314cd0c3f3ee96d3f38ec84e9a4 Mon Sep 17 00:00:00 2001 From: bad code <> Date: Sun, 22 Sep 2024 15:54:58 +0200 Subject: [PATCH 03/17] Use nullptr for pointer argument Avoid implicit integer to pointer conversion. --- src/linux/btop_collect.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/linux/btop_collect.cpp b/src/linux/btop_collect.cpp index 4b3ab9541..9980094dd 100644 --- a/src/linux/btop_collect.cpp +++ b/src/linux/btop_collect.cpp @@ -1582,7 +1582,7 @@ namespace Gpu { //? PCIe link speeds if (gpus_slice[i].supported_functions.pcie_txrx and Config::getB("rsmi_measure_pcie_speeds")) { uint64_t tx, rx; - result = rsmi_dev_pci_throughput_get(i, &tx, &rx, 0); + result = rsmi_dev_pci_throughput_get(i, &tx, &rx, nullptr); if (result != RSMI_STATUS_SUCCESS) { Logger::warning("ROCm SMI: Failed to get PCIe throughput"); if constexpr(is_init) gpus_slice[i].supported_functions.pcie_txrx = false; From 52b82b70eab42002fa1582dbb78ac66bf125cd7d Mon Sep 17 00:00:00 2001 From: bad code <> Date: Sun, 22 Sep 2024 15:55:18 +0200 Subject: [PATCH 04/17] Use explicit initialization Avoid uninitialized member variable. --- src/btop_draw.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/btop_draw.hpp b/src/btop_draw.hpp index 205fa8521..f987121f3 100644 --- a/src/btop_draw.hpp +++ b/src/btop_draw.hpp @@ -67,7 +67,7 @@ namespace Draw { class TextEdit { size_t pos{}; size_t upos{}; - bool numeric; + bool numeric = false; public: string text; TextEdit(); From 6c2d468f5dfdbb7a068b800939e6528f1ecd764c Mon Sep 17 00:00:00 2001 From: bad code <> Date: Sun, 22 Sep 2024 15:55:38 +0200 Subject: [PATCH 05/17] Simplify ternary expression --- src/btop_config.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/btop_config.cpp b/src/btop_config.cpp index b1ccfd11d..ef1fedfda 100644 --- a/src/btop_config.cpp +++ b/src/btop_config.cpp @@ -455,9 +455,9 @@ namespace Config { for (const auto& box : ssplit(preset, ',')) { const auto& vals = ssplit(box, ':'); - if (vals.at(0) == "cpu") set("cpu_bottom", (vals.at(1) == "0" ? false : true)); - else if (vals.at(0) == "mem") set("mem_below_net", (vals.at(1) == "0" ? false : true)); - else if (vals.at(0) == "proc") set("proc_left", (vals.at(1) == "0" ? false : true)); + if (vals.at(0) == "cpu") set("cpu_bottom", (vals.at(1) != "0")); + else if (vals.at(0) == "mem") set("mem_below_net", (vals.at(1) != "0")); + else if (vals.at(0) == "proc") set("proc_left", (vals.at(1) != "0")); set("graph_symbol_" + vals.at(0), vals.at(2)); } From 4651c31365fbffe2d363a32ded94046c0f471442 Mon Sep 17 00:00:00 2001 From: bad code <> Date: Sun, 22 Sep 2024 15:55:43 +0200 Subject: [PATCH 06/17] Drop duplicate include --- src/linux/btop_collect.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/linux/btop_collect.cpp b/src/linux/btop_collect.cpp index 9980094dd..67f1d285b 100644 --- a/src/linux/btop_collect.cpp +++ b/src/linux/btop_collect.cpp @@ -32,7 +32,6 @@ tab-size = 4 #include #include #include -#include #include #if defined(RSMI_STATIC) From 51728e89366f60763d9d2fa0b0f2b28e7b4e9e5d Mon Sep 17 00:00:00 2001 From: bad code <> Date: Sun, 22 Sep 2024 15:55:47 +0200 Subject: [PATCH 07/17] Declare local function static Clarifies the intended scope of the functions. --- src/btop.cpp | 16 ++++++++-------- src/btop_menu.cpp | 14 +++++++------- src/linux/btop_collect.cpp | 4 ++-- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/btop.cpp b/src/btop.cpp index 80783f02e..481aaaff3 100644 --- a/src/btop.cpp +++ b/src/btop.cpp @@ -158,7 +158,7 @@ static void print_help_hint() { } //* A simple argument parser -void argumentParser(const int argc, char **argv) { +static void argumentParser(const int argc, char **argv) { for(int i = 1; i < argc; i++) { const string argument = argv[i]; if (is_in(argument, "-h", "--help")) { @@ -360,23 +360,23 @@ void clean_quit(int sig) { } //* Handler for SIGTSTP; stops threads, restores terminal and sends SIGSTOP -void _sleep() { +static void _sleep() { Runner::stop(); Term::restore(); std::raise(SIGSTOP); } //* Handler for SIGCONT; re-initialize terminal and force a resize event -void _resume() { +static void _resume() { Term::init(); term_resize(true); } -void _exit_handler() { +static void _exit_handler() { clean_quit(-1); } -void _signal_handler(const int sig) { +static void _signal_handler(const int sig) { switch (sig) { case SIGINT: if (Runner::active) { @@ -415,7 +415,7 @@ void _signal_handler(const int sig) { } //* Config init -void init_config(){ +static void init_config(){ atomic_lock lck(Global::init_conf); vector load_warnings; Config::load(Config::conf_file, load_warnings); @@ -531,7 +531,7 @@ namespace Runner { struct runner_conf current_conf; - void debug_timer(const char* name, const int action) { + static void debug_timer(const char* name, const int action) { switch (action) { case collect_begin: debug_times[name].at(collect) = time_micros(); @@ -556,7 +556,7 @@ namespace Runner { } //? ------------------------------- Secondary thread: async launcher and drawing ---------------------------------- - void * _runner(void *) { + static void * _runner(void *) { //? Block some signals in this thread to avoid deadlock from any signal handlers trying to stop this thread sigemptyset(&mask); // sigaddset(&mask, SIGINT); diff --git a/src/btop_menu.cpp b/src/btop_menu.cpp index 488886a5f..2987b3463 100644 --- a/src/btop_menu.cpp +++ b/src/btop_menu.cpp @@ -878,7 +878,7 @@ namespace Menu { Switch }; - int signalChoose(const string& key) { + static int signalChoose(const string& key) { auto s_pid = (Config::getB("show_detailed") and Config::getI("selected_pid") == 0 ? Config::getI("detailed_pid") : Config::getI("selected_pid")); static int x{}; static int y{}; @@ -986,7 +986,7 @@ namespace Menu { return (redraw ? Changed : retval); } - int sizeError(const string& key) { + static int sizeError(const string& key) { if (redraw) { vector cont_vec; cont_vec.push_back(Fx::b + Theme::g("used")[100] + "Error:" + Theme::c("main_fg") + Fx::ub); @@ -1008,7 +1008,7 @@ namespace Menu { return NoChange; } - int signalSend(const string& key) { + static int signalSend(const string& key) { auto s_pid = (Config::getB("show_detailed") and Config::getI("selected_pid") == 0 ? Config::getI("detailed_pid") : Config::getI("selected_pid")); if (s_pid == 0) return Closed; if (redraw) { @@ -1048,7 +1048,7 @@ namespace Menu { return NoChange; } - int signalReturn(const string& key) { + static int signalReturn(const string& key) { if (redraw) { vector cont_vec; cont_vec.push_back(Fx::b + Theme::g("used")[100] + "Failure:" + Theme::c("main_fg") + Fx::ub); @@ -1080,7 +1080,7 @@ namespace Menu { return NoChange; } - int mainMenu(const string& key) { + static int mainMenu(const string& key) { enum MenuItems { Options, Help, Quit }; static int y{}; static int selected{}; @@ -1159,7 +1159,7 @@ namespace Menu { return (redraw ? Changed : retval); } - int optionsMenu(const string& key) { + static int optionsMenu(const string& key) { enum Predispositions { isBool, isInt, isString, is2D, isBrowseable, isEditable}; static int y{}; static int x{}; @@ -1511,7 +1511,7 @@ namespace Menu { return (redraw ? Changed : retval); } - int helpMenu(const string& key) { + static int helpMenu(const string& key) { static int y{}; static int x{}; static int height{}; diff --git a/src/linux/btop_collect.cpp b/src/linux/btop_collect.cpp index 67f1d285b..b64f12757 100644 --- a/src/linux/btop_collect.cpp +++ b/src/linux/btop_collect.cpp @@ -547,7 +547,7 @@ namespace Cpu { return not found_sensors.empty(); } - void update_sensors() { + static void update_sensors() { if (cpu_sensor.empty()) return; const auto& cpu_sensor = (not Config::getS("cpu_sensor").empty() and found_sensors.contains(Config::getS("cpu_sensor")) ? Config::getS("cpu_sensor") : Cpu::cpu_sensor); @@ -2562,7 +2562,7 @@ namespace Proc { static std::unordered_set kernels_procs = {KTHREADD}; //* Get detailed info for selected process - void _collect_details(const size_t pid, const uint64_t uptime, vector& procs) { + static void _collect_details(const size_t pid, const uint64_t uptime, vector& procs) { fs::path pid_path = Shared::procPath / std::to_string(pid); if (pid != detailed.last_pid) { From 43c8853c3695cff6f97d65414a0a5173c85ef59a Mon Sep 17 00:00:00 2001 From: bad code <> Date: Sun, 22 Sep 2024 15:56:12 +0200 Subject: [PATCH 08/17] Drop trailing newlines --- src/btop_draw.cpp | 1 - src/btop_tools.hpp | 3 --- 2 files changed, 4 deletions(-) diff --git a/src/btop_draw.cpp b/src/btop_draw.cpp index 00de3dede..24db3d6db 100644 --- a/src/btop_draw.cpp +++ b/src/btop_draw.cpp @@ -2239,4 +2239,3 @@ namespace Draw { } } } - diff --git a/src/btop_tools.hpp b/src/btop_tools.hpp index a61bd76e0..654ab63f7 100644 --- a/src/btop_tools.hpp +++ b/src/btop_tools.hpp @@ -453,6 +453,3 @@ namespace Tools { }; } - - - From 408f267588de478e665e0618b0f4a9d39c89b419 Mon Sep 17 00:00:00 2001 From: bad code <> Date: Sun, 22 Sep 2024 15:56:15 +0200 Subject: [PATCH 09/17] Avoid copy in constructor Enable compilers to move temporary variables into the constructor. --- src/btop_draw.cpp | 4 ++-- src/btop_draw.hpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/btop_draw.cpp b/src/btop_draw.cpp index 24db3d6db..7c86d1437 100644 --- a/src/btop_draw.cpp +++ b/src/btop_draw.cpp @@ -361,8 +361,8 @@ namespace Draw { //* Meter class ------------------------------------------------------------------------------------------------------------> Meter::Meter() {} - Meter::Meter(const int width, const string& color_gradient, bool invert) - : width(width), color_gradient(color_gradient), invert(invert) {} + Meter::Meter(const int width, string color_gradient, bool invert) + : width(width), color_gradient(std::move(color_gradient)), invert(invert) {} string Meter::operator()(int value) { if (width < 1) return ""; diff --git a/src/btop_draw.hpp b/src/btop_draw.hpp index f987121f3..af4ba833e 100644 --- a/src/btop_draw.hpp +++ b/src/btop_draw.hpp @@ -92,7 +92,7 @@ namespace Draw { array cache; public: Meter(); - Meter(const int width, const string& color_gradient, bool invert = false); + Meter(const int width, string color_gradient, bool invert = false); //* Return a string representation of the meter with given value string operator()(int value); From b5f94bf6edac32cc6a7c23da592ad6f280ac8044 Mon Sep 17 00:00:00 2001 From: bad code <> Date: Sun, 22 Sep 2024 15:56:40 +0200 Subject: [PATCH 10/17] Use member function instead of cast Avoid C-style cast. --- src/btop_menu.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/btop_menu.cpp b/src/btop_menu.cpp index 2987b3463..90239ecbc 100644 --- a/src/btop_menu.cpp +++ b/src/btop_menu.cpp @@ -1450,7 +1450,7 @@ namespace Menu { auto cy = y+9; for (int c = 0, i = max(0, item_height * page); c++ < item_height and i < (int)categories[selected_cat].size(); i++) { const auto& option = categories[selected_cat][i][0]; - const auto& value = (option == "color_theme" ? (string) fs::path(Config::getS("color_theme")).stem() : Config::getAsString(option)); + const auto& value = (option == "color_theme" ? fs::path(Config::getS("color_theme")).stem().string() : Config::getAsString(option)); out += Mv::to(cy++, x + 1) + (c-1 == selected ? Theme::c("selected_bg") + Theme::c("selected_fg") : Theme::c("title")) + Fx::b + cjust(capitalize(s_replace(option, "_", " ")) From a451814668106e3ae27c2781163c62cc39ca0179 Mon Sep 17 00:00:00 2001 From: bad code <> Date: Sun, 22 Sep 2024 15:56:49 +0200 Subject: [PATCH 11/17] Use O_CLOEXEC Avoid leaking file descriptor via potential sibling threads. --- src/btop_tools.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/btop_tools.cpp b/src/btop_tools.cpp index 263009d75..abe9cf2a8 100644 --- a/src/btop_tools.cpp +++ b/src/btop_tools.cpp @@ -95,7 +95,7 @@ namespace Term { struct winsize wsize {}; if (uses_dev_tty || ioctl(STDOUT_FILENO, TIOCGWINSZ, &wsize) < 0 || (wsize.ws_col == 0 && wsize.ws_row == 0)) { Logger::error(R"(Couldn't determine terminal size of "STDOUT_FILENO"!)"); - auto dev_tty = open("/dev/tty", O_RDONLY); + auto dev_tty = open("/dev/tty", O_RDONLY | O_CLOEXEC); if (dev_tty != -1) { ioctl(dev_tty, TIOCGWINSZ, &wsize); close(dev_tty); From 55f2b93cbe79fb2df1e2650c9d9c7e320f64495e Mon Sep 17 00:00:00 2001 From: bad code <> Date: Sun, 22 Sep 2024 15:57:37 +0200 Subject: [PATCH 12/17] Avoid repeated bounds check The bounds check is done explicitly in the line before. --- src/btop_tools.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/btop_tools.hpp b/src/btop_tools.hpp index 654ab63f7..568bb59a8 100644 --- a/src/btop_tools.hpp +++ b/src/btop_tools.hpp @@ -364,7 +364,7 @@ namespace Tools { #ifdef BTOP_DEBUG const T& safeVal(const std::vector& vec, const size_t& index, const T& fallback = T{}, std::source_location loc = std::source_location::current()) { if (index < vec.size()) { - return vec.at(index); + return vec[index]; } else { Logger::error(fmt::format("safeVal() called with invalid index: [{}] in file: {} on line: {}", index, loc.file_name(), loc.line())); return fallback; @@ -373,7 +373,7 @@ namespace Tools { #else const T& safeVal(const std::vector& vec, const size_t& index, const T& fallback = T{}) { if (index < vec.size()) { - return vec.at(index); + return vec[index]; } else { Logger::error(fmt::format("safeVal() called with invalid index: [{}] (Compile btop with DEBUG=true for more extensive logging!)", index)); return fallback; From 7713126db8230edf3aadaf8a88c95d8ad170da73 Mon Sep 17 00:00:00 2001 From: bad code <> Date: Sun, 22 Sep 2024 15:57:57 +0200 Subject: [PATCH 13/17] Ensure NUL-termination on truncation gethostname() might not NUL-terminate the passed buffer on truncation. --- src/btop_tools.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/btop_tools.cpp b/src/btop_tools.cpp index abe9cf2a8..c98ce39f3 100644 --- a/src/btop_tools.cpp +++ b/src/btop_tools.cpp @@ -556,6 +556,7 @@ namespace Tools { string hostname() { char host[HOST_NAME_MAX]; gethostname(host, HOST_NAME_MAX); + host[HOST_NAME_MAX - 1] = '\0'; return string{host}; } From b703a6ff606c9eeb9c945a2709a8a2a88f6fb75e Mon Sep 17 00:00:00 2001 From: bad code <> Date: Sun, 22 Sep 2024 15:58:32 +0200 Subject: [PATCH 14/17] Use explicit single parameter constructor Avoid implicit conversions; these might be unwanted or truncating. --- src/btop.cpp | 2 +- src/btop_draw.hpp | 2 +- src/btop_input.cpp | 2 +- src/btop_tools.hpp | 4 ++-- src/linux/btop_collect.cpp | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/btop.cpp b/src/btop.cpp index 481aaaff3..3cd3c2260 100644 --- a/src/btop.cpp +++ b/src/btop.cpp @@ -465,7 +465,7 @@ namespace Runner { pthread_mutex_t& pt_mutex; public: int status; - thread_lock(pthread_mutex_t& mtx) : pt_mutex(mtx) { + explicit thread_lock(pthread_mutex_t& mtx) : pt_mutex(mtx) { pthread_mutex_init(&pt_mutex, nullptr); status = pthread_mutex_lock(&pt_mutex); } diff --git a/src/btop_draw.hpp b/src/btop_draw.hpp index af4ba833e..907b5ca8b 100644 --- a/src/btop_draw.hpp +++ b/src/btop_draw.hpp @@ -71,7 +71,7 @@ namespace Draw { public: string text; TextEdit(); - TextEdit(string text, bool numeric=false); + explicit TextEdit(string text, bool numeric=false); bool command(const string& key); string operator()(const size_t limit=0); void clear(); diff --git a/src/btop_input.cpp b/src/btop_input.cpp index da77313bb..585d1a477 100644 --- a/src/btop_input.cpp +++ b/src/btop_input.cpp @@ -317,7 +317,7 @@ namespace Input { } else if (is_in(key, "f", "/")) { Config::flip("proc_filtering"); - Proc::filter = { Config::getS("proc_filter") }; + Proc::filter = Draw::TextEdit{Config::getS("proc_filter")}; old_filter = Proc::filter.text; } else if (key == "e") { diff --git a/src/btop_tools.hpp b/src/btop_tools.hpp index 568bb59a8..eed96d598 100644 --- a/src/btop_tools.hpp +++ b/src/btop_tools.hpp @@ -410,7 +410,7 @@ namespace Tools { atomic& atom; bool not_true{}; public: - atomic_lock(atomic& atom, bool wait = false); + explicit atomic_lock(atomic& atom, bool wait = false); ~atomic_lock(); }; @@ -438,7 +438,7 @@ namespace Tools { bool delayed_report{}; Logger::Level log_level = Logger::DEBUG; DebugTimer() = default; - DebugTimer(const string name, bool start = true, bool delayed_report = true); + explicit DebugTimer(const string name, bool start = true, bool delayed_report = true); ~DebugTimer(); void start(); diff --git a/src/linux/btop_collect.cpp b/src/linux/btop_collect.cpp index b64f12757..ff6555b86 100644 --- a/src/linux/btop_collect.cpp +++ b/src/linux/btop_collect.cpp @@ -481,7 +481,7 @@ namespace Cpu { const int64_t high = stol(readfile(fs::path(basepath + "max"), "80000")) / 1000; const int64_t crit = stol(readfile(fs::path(basepath + "crit"), "95000")) / 1000; - found_sensors[sensor_name] = {fs::path(basepath + "input"), label, temp, high, crit}; + found_sensors[sensor_name] = Sensor{fs::path(basepath + "input"), label, temp, high, crit}; if (not got_cpu and (label.starts_with("Package id") or label.starts_with("Tdie"))) { got_cpu = true; @@ -514,7 +514,7 @@ namespace Cpu { if (high < 1) high = 80; if (crit < 1) crit = 95; - found_sensors[sensor_name] = {basepath / "temp", label, temp, high, crit}; + found_sensors[sensor_name] = Sensor{basepath / "temp", label, temp, high, crit}; } } From 89cb932002e272c6cfc6bdbb3a6c9cf947667e85 Mon Sep 17 00:00:00 2001 From: bad code <> Date: Sun, 22 Sep 2024 15:58:53 +0200 Subject: [PATCH 15/17] Mark member variables private Hide class internals. --- src/btop_tools.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/btop_tools.hpp b/src/btop_tools.hpp index eed96d598..fce93c239 100644 --- a/src/btop_tools.hpp +++ b/src/btop_tools.hpp @@ -433,10 +433,10 @@ namespace Tools { bool running{}; std::locale custom_locale = std::locale(std::locale::classic(), new Tools::MyNumPunct); vector report_buffer{}; - public: string name{}; bool delayed_report{}; Logger::Level log_level = Logger::DEBUG; + public: DebugTimer() = default; explicit DebugTimer(const string name, bool start = true, bool delayed_report = true); ~DebugTimer(); From 36b438b2b93352f3a1c195f6ee1efcb5fef01520 Mon Sep 17 00:00:00 2001 From: bad code <> Date: Sun, 22 Sep 2024 15:59:06 +0200 Subject: [PATCH 16/17] Initialize all struct fields Avoid uninitialized members. --- src/btop.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/btop.cpp b/src/btop.cpp index 3cd3c2260..3ceb3411e 100644 --- a/src/btop.cpp +++ b/src/btop.cpp @@ -322,8 +322,7 @@ void clean_quit(int sig) { pthread_cancel(Runner::runner_id); } #else - struct timespec ts; - ts.tv_sec = 5; + constexpr struct timespec ts { .tv_sec = 5, .tv_nsec = 0 }; if (pthread_timedjoin_np(Runner::runner_id, nullptr, &ts) != 0) { Logger::warning("Failed to join _runner thread on exit!"); pthread_cancel(Runner::runner_id); From 4b3410b32c13ed3d50b6ae4806c57fb65173b13c Mon Sep 17 00:00:00 2001 From: bad code <> Date: Sun, 22 Sep 2024 15:59:19 +0200 Subject: [PATCH 17/17] Mark destructor noexcept and satisfy rule-of-five Follow the RAII paradigm and define all 5 special class function if any of them has a non-default implementation. Avoids duplicate destruction or other unwanted effects. --- src/btop.cpp | 12 ++++++++++-- src/btop_shared.hpp | 6 +++++- src/btop_tools.cpp | 8 ++++++-- src/btop_tools.hpp | 10 +++++++++- 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/btop.cpp b/src/btop.cpp index 3ceb3411e..cdfdb3d09 100644 --- a/src/btop.cpp +++ b/src/btop.cpp @@ -468,10 +468,14 @@ namespace Runner { pthread_mutex_init(&pt_mutex, nullptr); status = pthread_mutex_lock(&pt_mutex); } - ~thread_lock() { + ~thread_lock() noexcept { if (status == 0) pthread_mutex_unlock(&pt_mutex); } + thread_lock(const thread_lock& other) = delete; + thread_lock& operator=(const thread_lock& other) = delete; + thread_lock(thread_lock&& other) = delete; + thread_lock& operator=(thread_lock&& other) = delete; }; //* Wrapper for raising privileges when using SUID bit @@ -482,10 +486,14 @@ namespace Runner { if (Global::real_uid != Global::set_uid) this->status = seteuid(Global::set_uid); } - ~gain_priv() { + ~gain_priv() noexcept { if (status == 0) status = seteuid(Global::real_uid); } + gain_priv(const gain_priv& other) = delete; + gain_priv& operator=(const gain_priv& other) = delete; + gain_priv(gain_priv&& other) = delete; + gain_priv& operator=(gain_priv&& other) = delete; }; string output; diff --git a/src/btop_shared.hpp b/src/btop_shared.hpp index 7e24078a2..3a4e3fcaf 100644 --- a/src/btop_shared.hpp +++ b/src/btop_shared.hpp @@ -313,7 +313,11 @@ namespace Net { int status; public: IfAddrsPtr() { status = getifaddrs(&ifaddr); } - ~IfAddrsPtr() { freeifaddrs(ifaddr); } + ~IfAddrsPtr() noexcept { freeifaddrs(ifaddr); } + IfAddrsPtr(const IfAddrsPtr &) = delete; + IfAddrsPtr& operator=(IfAddrsPtr& other) = delete; + IfAddrsPtr(IfAddrsPtr &&) = delete; + IfAddrsPtr& operator=(IfAddrsPtr&& other) = delete; [[nodiscard]] constexpr auto operator()() -> struct ifaddrs* { return ifaddr; } [[nodiscard]] constexpr auto get() -> struct ifaddrs* { return ifaddr; } [[nodiscard]] constexpr auto get_status() const noexcept -> int { return status; }; diff --git a/src/btop_tools.cpp b/src/btop_tools.cpp index c98ce39f3..3fe1c9f83 100644 --- a/src/btop_tools.cpp +++ b/src/btop_tools.cpp @@ -523,7 +523,7 @@ namespace Tools { else this->atom.store(true); } - atomic_lock::~atomic_lock() { + atomic_lock::~atomic_lock() noexcept { this->atom.store(false); } @@ -654,11 +654,15 @@ namespace Logger { this->status = seteuid(Global::real_uid); } } - ~lose_priv() { + ~lose_priv() noexcept { if (status == 0) { status = seteuid(Global::set_uid); } } + lose_priv(const lose_priv& other) = delete; + lose_priv& operator=(const lose_priv& other) = delete; + lose_priv(lose_priv&& other) = delete; + lose_priv& operator=(lose_priv&& other) = delete; }; void set(const string& level) { diff --git a/src/btop_tools.hpp b/src/btop_tools.hpp index fce93c239..fa8f7275d 100644 --- a/src/btop_tools.hpp +++ b/src/btop_tools.hpp @@ -411,7 +411,11 @@ namespace Tools { bool not_true{}; public: explicit atomic_lock(atomic& atom, bool wait = false); - ~atomic_lock(); + ~atomic_lock() noexcept; + atomic_lock(const atomic_lock& other) = delete; + atomic_lock& operator=(const atomic_lock& other) = delete; + atomic_lock(atomic_lock&& other) = delete; + atomic_lock& operator=(atomic_lock&& other) = delete; }; //* Read a complete file and return as a string @@ -440,6 +444,10 @@ namespace Tools { DebugTimer() = default; explicit DebugTimer(const string name, bool start = true, bool delayed_report = true); ~DebugTimer(); + DebugTimer(const DebugTimer& other) = delete; + DebugTimer& operator=(const DebugTimer& other) = delete; + DebugTimer(DebugTimer&& other) = delete; + DebugTimer& operator=(DebugTimer&& other) = delete; void start(); void stop(bool report = true);