From ff3b07a086e837816ea7fc58ca4e55ed6f610963 Mon Sep 17 00:00:00 2001 From: Alexander Shaduri Date: Wed, 3 Mar 2021 16:53:22 +0400 Subject: [PATCH] Fixed a few clang-tidy warnings in applib. --- src/applib/cmdex.cpp | 44 ++++++++++++----------- src/applib/cmdex.h | 4 +-- src/applib/cmdex_sync.cpp | 18 ++++------ src/applib/cmdex_sync.h | 2 +- src/applib/cmdex_sync_gui.cpp | 10 +++--- src/applib/cmdex_sync_gui.h | 22 +++--------- src/applib/gui_utils.h | 4 +-- src/applib/selftest.cpp | 10 ++++-- src/applib/smartctl_parser.cpp | 51 +++++++++++++++------------ src/applib/smartctl_parser.h | 8 ++--- src/applib/storage_detector.h | 8 ++--- src/applib/storage_detector_linux.cpp | 13 ++++--- src/applib/storage_device.cpp | 14 +++++--- src/applib/storage_property.h | 38 ++++++++++---------- src/applib/storage_property_descr.cpp | 45 ++++++++++------------- 15 files changed, 142 insertions(+), 149 deletions(-) diff --git a/src/applib/cmdex.cpp b/src/applib/cmdex.cpp index 6183556..82aa0c2 100644 --- a/src/applib/cmdex.cpp +++ b/src/applib/cmdex.cpp @@ -15,6 +15,7 @@ Copyright: #include #include #include // errno (not std::errno, it may be a macro) +#include #ifdef _WIN32 // #include // close() @@ -69,7 +70,7 @@ extern "C" { DBG_FUNCTION_ENTER_MSG; auto* self = static_cast(data); self->try_stop(hz::Signal::Terminate); - return false; // one-time call + return FALSE; // one-time call } @@ -79,7 +80,7 @@ extern "C" { DBG_FUNCTION_ENTER_MSG; auto* self = static_cast(data); self->try_stop(hz::Signal::Kill); - return false; // one-time call + return FALSE; // one-time call } @@ -173,14 +174,14 @@ bool Cmdex::execute() // Since we invoke shutdown() manually before unref(), this would cause // a double-shutdown. // g_io_channel_set_close_on_unref(channel_stdout_, true); // close() on fd - g_io_channel_set_encoding(channel_stdout_, nullptr, 0); // binary IO - g_io_channel_set_flags(channel_stdout_, GIOFlags(g_io_channel_get_flags(channel_stdout_) & channel_flags), 0); + g_io_channel_set_encoding(channel_stdout_, nullptr, nullptr); // binary IO + g_io_channel_set_flags(channel_stdout_, GIOFlags(g_io_channel_get_flags(channel_stdout_) & channel_flags), nullptr); g_io_channel_set_buffer_size(channel_stdout_, channel_stdout_buffer_size_); } if (channel_stderr_) { // g_io_channel_set_close_on_unref(channel_stderr_, true); // close() on fd - g_io_channel_set_encoding(channel_stderr_, nullptr, 0); // binary IO - g_io_channel_set_flags(channel_stderr_, GIOFlags(g_io_channel_get_flags(channel_stderr_) & channel_flags), 0); + g_io_channel_set_encoding(channel_stderr_, nullptr, nullptr); // binary IO + g_io_channel_set_flags(channel_stderr_, GIOFlags(g_io_channel_get_flags(channel_stderr_) & channel_flags), nullptr); g_io_channel_set_buffer_size(channel_stderr_, channel_stderr_buffer_size_); } @@ -271,14 +272,14 @@ void Cmdex::set_stop_timeouts(std::chrono::milliseconds term_timeout_msec, std:: void Cmdex::unset_stop_timeouts() { DBG_FUNCTION_ENTER_MSG; - if (event_source_id_term) { + if (event_source_id_term != 0) { GSource* source_term = g_main_context_find_source_by_id(nullptr, event_source_id_term); if (source_term) g_source_destroy(source_term); event_source_id_term = 0; } - if (event_source_id_kill) { + if (event_source_id_kill != 0) { GSource* source_kill = g_main_context_find_source_by_id(nullptr, event_source_id_kill); if (source_kill) g_source_destroy(source_kill); @@ -356,13 +357,13 @@ void Cmdex::on_child_watch_handler([[maybe_unused]] GPid arg_pid, int waitpid_st on_channel_io(self->channel_stderr_, GIOCondition(0), self, Channel::standard_error); if (self->channel_stdout_) { - g_io_channel_shutdown(self->channel_stdout_, false, nullptr); + g_io_channel_shutdown(self->channel_stdout_, FALSE, nullptr); g_io_channel_unref(self->channel_stdout_); self->channel_stdout_ = nullptr; } if (self->channel_stderr_) { - g_io_channel_shutdown(self->channel_stderr_, false, nullptr); + g_io_channel_shutdown(self->channel_stderr_, FALSE, nullptr); g_io_channel_unref(self->channel_stderr_); self->channel_stderr_ = nullptr; } @@ -370,13 +371,13 @@ void Cmdex::on_child_watch_handler([[maybe_unused]] GPid arg_pid, int waitpid_st // Remove fd IO callbacks. They may actually be removed already (note sure about this). // This will force calling the iochannel callback (they may not be called // otherwise at all if there was no output). - if (self->event_source_id_stdout_) { + if (self->event_source_id_stdout_ != 0) { GSource* source_stdout = g_main_context_find_source_by_id(nullptr, self->event_source_id_stdout_); if (source_stdout) g_source_destroy(source_stdout); } - if (self->event_source_id_stderr_) { + if (self->event_source_id_stderr_ != 0) { GSource* source_stderr = g_main_context_find_source_by_id(nullptr, self->event_source_id_stderr_); if (source_stderr) g_source_destroy(source_stderr); @@ -405,17 +406,17 @@ gboolean Cmdex::on_channel_io(GIOChannel* channel, // << (type == Channel::standard_output ? "STDOUT" : "STDERR") << ") " << int(cond) << "\n"); bool continue_events = true; - if ((cond & G_IO_ERR) || (cond & G_IO_HUP) || (cond & G_IO_NVAL)) { + if (bool(cond & G_IO_ERR) || bool(cond & G_IO_HUP) || bool(cond & G_IO_NVAL)) { continue_events = false; // there'll be no more data } - DBG_ASSERT(channel_type == Channel::standard_output || channel_type == Channel::standard_error); + DBG_ASSERT_RETURN(channel_type == Channel::standard_output || channel_type == Channel::standard_error, false); // const gsize count = 4 * 1024; // read the bytes one by one. without this, a buffered iochannel hangs while waiting for data. // we don't use unbuffered iochannels - they may lose data on program exit. - const gsize count = 1; - gchar buf[count] = {0}; + constexpr gsize count = 1; + std::array buf = {0}; std::string* output_str = nullptr; if (channel_type == Channel::standard_output) { @@ -423,15 +424,16 @@ gboolean Cmdex::on_channel_io(GIOChannel* channel, } else if (channel_type == Channel::standard_error) { output_str = &self->str_stderr_; } + DBG_ASSERT_RETURN(output_str, false); // while there's anything to read, read it do { GError* channel_error = nullptr; gsize bytes_read = 0; - GIOStatus status = g_io_channel_read_chars(channel, buf, count, &bytes_read, &channel_error); - if (bytes_read) - output_str->append(buf, bytes_read); + GIOStatus status = g_io_channel_read_chars(channel, buf.data(), count, &bytes_read, &channel_error); + if (bytes_read != 0) + output_str->append(buf.data(), bytes_read); if (channel_error) { self->push_error(Error("giochannel", ErrorLevel::error, channel_error->message)); @@ -444,12 +446,12 @@ gboolean Cmdex::on_channel_io(GIOChannel* channel, continue_events = false; break; } - } while (g_io_channel_get_buffer_condition(channel) & G_IO_IN); + } while (bool(g_io_channel_get_buffer_condition(channel) & G_IO_IN)); // DBG_FUNCTION_EXIT_MSG; // false if the source should be removed, true otherwise. - return continue_events; + return gboolean(continue_events); } diff --git a/src/applib/cmdex.h b/src/applib/cmdex.h index b925616..a04a582 100644 --- a/src/applib/cmdex.h +++ b/src/applib/cmdex.h @@ -42,7 +42,7 @@ class Cmdex : public hz::ErrorHolder { /// Destructor. Don't destroy this object unless the child has exited. It will leak stuff /// and possibly crash, etc... . - ~Cmdex() + ~Cmdex() override { // This will help if object is destroyed after the command has exited, but before // stopped_cleanup() has been called. @@ -110,7 +110,7 @@ class Cmdex : public hz::ErrorHolder { /// Check if the process is running. Note that if this returns false, it doesn't mean that /// the io channels have been closed or that the data may be read safely. Poll /// stopped_cleanup_needed() instead. - bool is_running() const + [[nodiscard]] bool is_running() const { return running_; } diff --git a/src/applib/cmdex_sync.cpp b/src/applib/cmdex_sync.cpp index cd85f34..217281f 100644 --- a/src/applib/cmdex_sync.cpp +++ b/src/applib/cmdex_sync.cpp @@ -16,17 +16,10 @@ Copyright: -namespace { - - /// "Execution finished" signal - sigc::signal s_cmdex_sync_signal_execute_finish; - -} - - - cmdex_signal_execute_finish_type& cmdex_sync_signal_execute_finish() { + /// "Execution finished" signal + static sigc::signal s_cmdex_sync_signal_execute_finish; return s_cmdex_sync_signal_execute_finish; } @@ -134,11 +127,12 @@ bool CmdexSync::execute() // hang waiting for the child to exit (the watch handler won't be called). // Note: If you have an idle callback, g_main_context_pending() will // always return true (until the idle callback returns false and unregisters itself). - while(g_main_context_pending(nullptr)) { - g_main_context_iteration(nullptr, false); + while(g_main_context_pending(nullptr) != FALSE) { + g_main_context_iteration(nullptr, FALSE); } - g_usleep(50*1000); // 50 msec. avoids 100% CPU usage. + const gulong sleep_us = 50*1000; // 50 msec. avoids 100% CPU usage. + g_usleep(sleep_us); } // command exited, do a cleanup. diff --git a/src/applib/cmdex_sync.h b/src/applib/cmdex_sync.h index 04807e4..cba16ab 100644 --- a/src/applib/cmdex_sync.h +++ b/src/applib/cmdex_sync.h @@ -151,7 +151,7 @@ class CmdexSync : public sigc::trackable { /// See Cmdex::set_exit_status_translator() for details. void set_exit_status_translator(Cmdex::exit_status_translator_func_t func) { - cmdex_.set_exit_status_translator(func); + cmdex_.set_exit_status_translator(std::move(func)); } diff --git a/src/applib/cmdex_sync_gui.cpp b/src/applib/cmdex_sync_gui.cpp index 0040adf..333e59a 100644 --- a/src/applib/cmdex_sync_gui.cpp +++ b/src/applib/cmdex_sync_gui.cpp @@ -13,6 +13,8 @@ Copyright: #include // Gtk::Main #include +#include + #include "hz/string_algo.h" #include "hz/fs_ns.h" #include "cmdex_sync_gui.h" @@ -36,17 +38,17 @@ bool CmdexSyncGui::execute() Gtk::MessageDialog* CmdexSyncGui::create_running_dialog(Gtk::Window* parent, const Glib::ustring& msg) { if (running_dialog_) - return running_dialog_; + return running_dialog_.get(); if (!msg.empty()) set_running_msg(msg); // Construct the dialog so we can manipulate it before execution if (parent) { - running_dialog_ = new Gtk::MessageDialog(*parent, "", false, + running_dialog_ = std::make_unique(*parent, "", false, CMDEX_DIALOG_MESSAGE_TYPE, Gtk::BUTTONS_CANCEL); } else { - running_dialog_ = new Gtk::MessageDialog("", false, + running_dialog_ = std::make_unique("", false, CMDEX_DIALOG_MESSAGE_TYPE, Gtk::BUTTONS_CANCEL); } @@ -63,7 +65,7 @@ Gtk::MessageDialog* CmdexSyncGui::create_running_dialog(Gtk::Window* parent, con // this won't harm the tests - they don't involve long-running commands. running_dialog_->set_modal(true); - return running_dialog_; + return running_dialog_.get(); } diff --git a/src/applib/cmdex_sync_gui.h b/src/applib/cmdex_sync_gui.h index 898f323..d0dc9dd 100644 --- a/src/applib/cmdex_sync_gui.h +++ b/src/applib/cmdex_sync_gui.h @@ -12,10 +12,9 @@ Copyright: #ifndef APP_CMDEX_SYNC_GUI_H #define APP_CMDEX_SYNC_GUI_H -// TODO Remove this in gtkmm4. #include "local_glibmm.h" - #include +#include #include "cmdex_sync.h" @@ -41,19 +40,6 @@ class CmdexSyncGui : public CmdexSync { } - /// Non-construction-copyable - CmdexSyncGui(const CmdexSyncGui& other) = delete; - - /// Non-copyable - CmdexSyncGui& operator=(const CmdexSyncGui&) = delete; - - - /// Destructor - ~CmdexSyncGui() - { - delete running_dialog_; - } - // Reimplemented from CmdexSync bool execute() override; @@ -73,9 +59,9 @@ class CmdexSyncGui : public CmdexSync { /// Return the "running" dialog - Gtk::MessageDialog* get_running_dialog() + [[nodiscard]] Gtk::MessageDialog* get_running_dialog() { - return running_dialog_; + return running_dialog_.get(); } @@ -112,7 +98,7 @@ class CmdexSyncGui : public CmdexSync { bool execution_running_ = false; ///< If true, the execution is still in progress bool should_abort_ = false; ///< GUI callbacks may set this to abort the execution - Gtk::MessageDialog* running_dialog_ = nullptr; ///< "Running" dialog + std::unique_ptr running_dialog_; ///< "Running" dialog bool running_dialog_shown_ = false; ///< If true, the "running" dialog is visible bool running_dialog_abort_mode_ = false; ///< If true, the "running" dialog is in "aborting..." mode. Glib::Timer running_dialog_timer_; ///< "Running" dialog show timer. diff --git a/src/applib/gui_utils.h b/src/applib/gui_utils.h index 33be9ab..0d7acd7 100644 --- a/src/applib/gui_utils.h +++ b/src/applib/gui_utils.h @@ -12,12 +12,10 @@ Copyright: #ifndef GUI_UTILS_H #define GUI_UTILS_H -// TODO Remove this in gtkmm4. #include "local_glibmm.h" - +#include #include -#include // These functions won't return until the dialogs are closed. diff --git a/src/applib/selftest.cpp b/src/applib/selftest.cpp index 94aa92b..671dc46 100644 --- a/src/applib/selftest.cpp +++ b/src/applib/selftest.cpp @@ -94,9 +94,13 @@ bool SelfTest::is_supported() const std::string prop_name; switch(type_) { - case TestType::immediate_offline: prop_name = "iodc_support"; break; - case TestType::short_test: prop_name = "selftest_support"; break; - case TestType::long_test: prop_name = "selftest_support"; break; // same for short and long + case TestType::immediate_offline: + prop_name = "iodc_support"; + break; + case TestType::short_test: + case TestType::long_test: // same for short and long + prop_name = "selftest_support"; + break; case TestType::conveyance: prop_name = "conveyance_support"; break; } diff --git a/src/applib/smartctl_parser.cpp b/src/applib/smartctl_parser.cpp index 9067c2f..eabe528 100644 --- a/src/applib/smartctl_parser.cpp +++ b/src/applib/smartctl_parser.cpp @@ -211,24 +211,23 @@ bool SmartctlParser::parse_full(const std::string& full, StorageAttribute::DiskT set_error_msg("Cannot extract smartctl version information."); debug_out_warn("app", DBG_FUNC_MSG << "Cannot extract version information. Returning.\n"); return false; + } - } else { - { - StorageProperty p; - p.set_name("Smartctl version", "smartctl_version", "Smartctl Version"); - p.reported_value = version; - p.value = p.reported_value; // string-type value - p.section = StorageProperty::Section::info; // add to info section - add_property(p); - } - { - StorageProperty p; - p.set_name("Smartctl version", "smartctl_version_full", "Smartctl Version"); - p.reported_value = version_full; - p.value = p.reported_value; // string-type value - p.section = StorageProperty::Section::info; // add to info section - add_property(p); - } + { + StorageProperty p; + p.set_name("Smartctl version", "smartctl_version", "Smartctl Version"); + p.reported_value = version; + p.value = p.reported_value; // string-type value + p.section = StorageProperty::Section::info; // add to info section + add_property(p); + } + { + StorageProperty p; + p.set_name("Smartctl version", "smartctl_version_full", "Smartctl Version"); + p.reported_value = version_full; + p.value = p.reported_value; // string-type value + p.section = StorageProperty::Section::info; // add to info section + add_property(p); } if (!check_parsed_version(version, version_full)) { @@ -368,23 +367,27 @@ bool SmartctlParser::parse_section(const std::string& header, const std::string& { if (app_pcre_match("/START OF INFORMATION SECTION/mi", header)) { return parse_section_info(body); + } - } else if (app_pcre_match("/START OF READ SMART DATA SECTION/mi", header)) { + if (app_pcre_match("/START OF READ SMART DATA SECTION/mi", header)) { return parse_section_data(body); + } // These sections provide information about actions performed. // You may encounter this if e.g. executing "smartctl -a -s on". // example contents: "SMART Enabled.". - } else if (app_pcre_match("/START OF READ SMART DATA SECTION/mi", header)) { + if (app_pcre_match("/START OF READ SMART DATA SECTION/mi", header)) { return true; + } // We don't parse this - it's parsed by the respective command issuer. - } else if (app_pcre_match("/START OF ENABLE/DISABLE COMMANDS SECTION/mi", header)) { + if (app_pcre_match("/START OF ENABLE/DISABLE COMMANDS SECTION/mi", header)) { return true; + } // This is printed when executing "-t long", etc... . Parsed by respective command issuer. - } else if (app_pcre_match("/START OF OFFLINE IMMEDIATE AND SELF-TEST SECTION/mi", header)) { + if (app_pcre_match("/START OF OFFLINE IMMEDIATE AND SELF-TEST SECTION/mi", header)) { return true; } @@ -1024,7 +1027,8 @@ SCT capabilities: (0x003d) SCT Status supported. // add as a time property StorageProperty p(pt); p.set_name(name); - p.reported_value = numvalue_orig + " | " + strvalue_orig; // well, not really as reported, but still... + // well, not really as reported, but still... + p.reported_value.append(numvalue_orig).append(" | ").append(strvalue_orig); p.value = std::chrono::seconds(numvalue); // always in seconds // Set some generic names on the recognized ones @@ -1039,7 +1043,8 @@ SCT capabilities: (0x003d) SCT Status supported. StorageProperty p(pt); p.set_name(name); - p.reported_value = numvalue_orig + " | " + strvalue_orig; // well, not really as reported, but still... + // well, not really as reported, but still... + p.reported_value.append(numvalue_orig).append(" | ").append(strvalue_orig); StorageCapability cap; cap.reported_flag_value = numvalue_orig; diff --git a/src/applib/smartctl_parser.h b/src/applib/smartctl_parser.h index fbc7611..234807f 100644 --- a/src/applib/smartctl_parser.h +++ b/src/applib/smartctl_parser.h @@ -26,7 +26,7 @@ class SmartctlParser { public: /// Parse full "smartctl -x" output - bool parse_full(const std::string& s, StorageAttribute::DiskType disk_type); + bool parse_full(const std::string& full, StorageAttribute::DiskType disk_type); /// Supply any output of smartctl here, the smartctl version will be retrieved. @@ -89,7 +89,7 @@ class SmartctlParser { /// Get "full" data, as passed to parse_full(). - std::string get_data_full() const; + [[nodiscard]] std::string get_data_full() const; /* std::string get_data_section_info() const @@ -105,11 +105,11 @@ class SmartctlParser { /// Get parse error message. Call this only if parsing doesn't succeed, /// to get a friendly error message. - std::string get_error_msg() const; + [[nodiscard]] std::string get_error_msg() const; /// Get parse result properties - const std::vector& get_properties() const; + [[nodiscard]] const std::vector& get_properties() const; diff --git a/src/applib/storage_detector.h b/src/applib/storage_detector.h index 24875c4..73ddb0e 100644 --- a/src/applib/storage_detector.h +++ b/src/applib/storage_detector.h @@ -26,7 +26,7 @@ class StorageDetector { public: /// Detects a list of drives. Returns detection error message if error occurs. - std::string detect(std::vector& put_drives_here, + std::string detect(std::vector& drives, const ExecutorFactoryPtr& ex_factory); @@ -50,21 +50,21 @@ class StorageDetector { /// Add device patterns to drive detection blacklist - void add_blacklist_patterns(std::vector& patterns) + void add_blacklist_patterns(const std::vector& patterns) { blacklist_patterns_.insert(blacklist_patterns_.end(), patterns.begin(), patterns.end()); } /// Get all errors produced by fetch_basic_data(). - const std::vector& get_fetch_data_errors() const + [[nodiscard]] const std::vector& get_fetch_data_errors() const { return fetch_data_errors_; } /// Get command output for each error in get_fetch_data_errors(). - const std::vector& get_fetch_data_error_outputs() const + [[nodiscard]] const std::vector& get_fetch_data_error_outputs() const { return fetch_data_error_outputs_; } diff --git a/src/applib/storage_detector_linux.cpp b/src/applib/storage_detector_linux.cpp index 3feb7dc..094558c 100644 --- a/src/applib/storage_detector_linux.cpp +++ b/src/applib/storage_detector_linux.cpp @@ -121,14 +121,18 @@ inline std::string detect_drives_linux_udev_byid(std::vector& devic /// Cache of file->contents, caches read files. -std::map s_read_file_cache; +inline std::map& get_read_file_cache_ref() +{ + static std::map cache; + return cache; +} /// Clear the read file cache. inline void clear_read_file_cache() { - s_read_file_cache.clear(); + get_read_file_cache_ref().clear(); } @@ -136,7 +140,8 @@ inline void clear_read_file_cache() /// Read procfs file without using seeking. inline std::error_code read_proc_file(const hz::fs::path& file, std::string& contents) { - if (auto iter = s_read_file_cache.find(file); iter != s_read_file_cache.end()) { + auto& cache = get_read_file_cache_ref(); + if (auto iter = cache.find(file); iter != cache.end()) { contents = iter->second; return std::error_code(); } @@ -146,7 +151,7 @@ inline std::error_code read_proc_file(const hz::fs::path& file, std::string& con return ec; } - s_read_file_cache[file] = contents; + cache[file] = contents; debug_begin(); // avoiding printing prefix on every line debug_out_dump("app", DBG_FUNC_MSG << "File contents (\"" << file.string() << "\"):\n" << contents << "\n"); diff --git a/src/applib/storage_device.cpp b/src/applib/storage_device.cpp index 1928b5b..04e4b0b 100644 --- a/src/applib/storage_device.cpp +++ b/src/applib/storage_device.cpp @@ -368,14 +368,16 @@ A mandatory SMART command failed: exiting. To continue, add one or more '-T perm std::string output; std::string error_msg = execute_device_smartctl((b ? "--smart=on --saveauto=on" : "--smart=off"), smartctl_ex, output); - if (!error_msg.empty()) + if (!error_msg.empty()) { return error_msg; + } // search at line start, because they are sometimes present in other sentences too. if (app_pcre_match("/^SMART Enabled/mi", output) || app_pcre_match("/^SMART Disabled/mi", output)) { return std::string(); // success + } - } else if (app_pcre_match("/^A mandatory SMART command failed/mi", output)) { + if (app_pcre_match("/^A mandatory SMART command failed/mi", output)) { return _("Mandatory SMART command failed."); } @@ -386,8 +388,9 @@ A mandatory SMART command failed: exiting. To continue, add one or more '-T perm std::string StorageDevice::set_aodc_enabled(bool b, const std::shared_ptr& smartctl_ex) { - if (this->test_is_active_) + if (this->test_is_active_) { return _("A test is currently being performed on this drive."); + } // execute smartctl --offlineauto=on|off /dev/... // Output: @@ -407,8 +410,9 @@ A mandatory SMART command failed: exiting. To continue, add one or more '-T perm if (app_pcre_match("/Testing Enabled/mi", output) || app_pcre_match("/Testing Disabled/mi", output)) { return std::string(); // success + } - } else if (app_pcre_match("/^A mandatory SMART command failed/mi", output)) { + if (app_pcre_match("/^A mandatory SMART command failed/mi", output)) { return _("Mandatory SMART command failed."); } @@ -692,7 +696,7 @@ std::string StorageDevice::get_serial_number() const bool StorageDevice::get_is_hdd() const { - return hdd_.has_value() ? hdd_.value() : false; + return hdd_.has_value() && hdd_.value(); } diff --git a/src/applib/storage_property.h b/src/applib/storage_property.h index a27955f..b6eb61b 100644 --- a/src/applib/storage_property.h +++ b/src/applib/storage_property.h @@ -62,7 +62,7 @@ class StorageAttribute { }; /// Get readable attribute type name - static std::string get_attr_type_name(AttributeType type) + [[nodiscard]] static std::string get_attr_type_name(AttributeType type) { static const std::unordered_map m { {AttributeType::unknown, "[unknown]"}, @@ -84,7 +84,7 @@ class StorageAttribute { }; /// Get readable when-updated type name - static std::string get_update_type_name(UpdateType type) + [[nodiscard]] static std::string get_update_type_name(UpdateType type) { static const std::unordered_map m { {UpdateType::unknown, "[unknown]"}, @@ -107,7 +107,7 @@ class StorageAttribute { }; /// Get a readable when-failed type name - static std::string get_fail_time_name(FailTime type) + [[nodiscard]] static std::string get_fail_time_name(FailTime type) { static const std::unordered_map m { {FailTime::unknown, "[unknown]"}, @@ -123,7 +123,7 @@ class StorageAttribute { /// Format raw value with commas (if it's a number) - std::string format_raw_value() const; + [[nodiscard]] std::string format_raw_value() const; int32_t id = -1; ///< Attribute ID (most vendors agree on this) @@ -151,13 +151,13 @@ class StorageStatistic { public: /// Whether the normalization flag is present - bool is_normalized() const + [[nodiscard]] bool is_normalized() const { - return flags.find('N') != flags.npos; + return flags.find('N') != std::string::npos; } /// Format value with commas (if it's a number) - std::string format_value() const; + [[nodiscard]] std::string format_value() const; bool is_header = false; ///< If the line is a header std::string flags; ///< Flags in "NDC" / "---" format @@ -184,7 +184,7 @@ class StorageErrorBlock { static WarningLevel get_warning_level_for_error_type(const std::string& type); /// Format lifetime hours with comma - std::string format_lifetime_hours() const; + [[nodiscard]] std::string format_lifetime_hours() const; uint32_t error_num = 0; ///< Error number uint32_t lifetime_hours = 0; ///< When the error occurred (in lifetime hours) @@ -229,7 +229,7 @@ class StorageSelftestEntry { }; /// Get log entry status displayable name - static std::string get_status_displayable_name(Status s) + [[nodiscard]] static std::string get_status_displayable_name(Status s) { static const std::unordered_map m { {Status::unknown, "[unknown]"}, @@ -252,7 +252,7 @@ class StorageSelftestEntry { } /// Get severity of error status - static StatusSeverity get_status_severity(Status s) + [[nodiscard]] static StatusSeverity get_status_severity(Status s) { static const std::unordered_map m { {Status::unknown, StatusSeverity::none}, @@ -276,14 +276,14 @@ class StorageSelftestEntry { /// Get error status as a string - std::string get_status_str() const + [[nodiscard]] std::string get_status_str() const { return (status == Status::unknown ? status_str : get_status_displayable_name(status)); } /// Format lifetime hours with comma - std::string format_lifetime_hours() const; + [[nodiscard]] std::string format_lifetime_hours() const; uint32_t test_num = 0; ///< Test number. always starts from 1. larger means older or newer, depending on model. 0 for capability. @@ -315,7 +315,7 @@ class StorageProperty { }; /// Get displayable section type name - static std::string get_section_name(Section s) + [[nodiscard]] static std::string get_section_name(Section s) { static const std::unordered_map m { {Section::unknown, "unknown"}, @@ -347,7 +347,7 @@ class StorageProperty { }; /// Get displayable subsection type name - static std::string get_subsection_name(SubSection s) + [[nodiscard]] static std::string get_subsection_name(SubSection s) { static const std::unordered_map m { {SubSection::unknown, "unknown"}, @@ -371,7 +371,7 @@ class StorageProperty { /// Get displayable value type name - std::string get_value_type_name() const + [[nodiscard]] std::string get_value_type_name() const { if (std::holds_alternative(value)) return "empty"; @@ -398,7 +398,7 @@ class StorageProperty { /// Check if this is an empty object with no value set. - bool empty() const + [[nodiscard]] bool empty() const { return std::holds_alternative(value); } @@ -409,7 +409,7 @@ class StorageProperty { /// Format this property for debugging purposes - std::string format_value(bool add_reported_too = false) const; + [[nodiscard]] std::string format_value(bool add_reported_too = false) const; /// Get value of type T @@ -422,14 +422,14 @@ class StorageProperty { /// Check if value is of type T template - bool is_value_type() const + [[nodiscard]] bool is_value_type() const { return std::holds_alternative(value); } /// Get property description (used in tooltips) - std::string get_description(bool clean = false) const + [[nodiscard]] std::string get_description(bool clean = false) const { if (clean) return this->description; diff --git a/src/applib/storage_property_descr.cpp b/src/applib/storage_property_descr.cpp index 377a581..1bd869d 100644 --- a/src/applib/storage_property_descr.cpp +++ b/src/applib/storage_property_descr.cpp @@ -1101,7 +1101,7 @@ namespace { /// Find the description by smartctl name or id, merging them if they're partial. - AttributeDescription find(const std::string& reported_name, int32_t id, StorageAttribute::DiskType type) const + [[nodiscard]] AttributeDescription find(const std::string& reported_name, int32_t id, StorageAttribute::DiskType type) const { // search by ID first auto id_iter = id_db.find(id); @@ -1143,8 +1143,14 @@ namespace { }; - /// Program-wide attribute description database - const AttributeDatabase s_attribute_db; + + + /// Get program-wide attribute description database + inline const AttributeDatabase& get_attribute_db() + { + static const AttributeDatabase attribute_db; + return attribute_db; + } @@ -1356,7 +1362,7 @@ namespace { /// Find the description by smartctl name or id, merging them if they're partial. - StatisticDescription find(const std::string& reported_name) const + [[nodiscard]] StatisticDescription find(const std::string& reported_name) const { // search by ID first auto iter = devstat_db.find(reported_name); @@ -1374,8 +1380,13 @@ namespace { }; - /// Program-wide devstat description database - const StatisticsDatabase s_devstat_db; + + /// Get program-wide devstat description database + inline const StatisticsDatabase& get_devstat_db() + { + static const StatisticsDatabase devstat_db; + return devstat_db; + } @@ -1414,7 +1425,7 @@ namespace { /// with all the readable information we can gather. inline void auto_set_attr(StorageProperty& p, StorageAttribute::DiskType disk_type) { - AttributeDescription attr = s_attribute_db.find(p.reported_name, p.get_value().id, disk_type); + AttributeDescription attr = get_attribute_db().find(p.reported_name, p.get_value().id, disk_type); std::string humanized_reported_name; std::string ssd_hdd_str; @@ -1511,7 +1522,7 @@ namespace { /// with all the readable information we can gather. inline bool auto_set_statistic(StorageProperty& p) { - StatisticDescription sd = s_devstat_db.find(p.reported_name); + StatisticDescription sd = get_devstat_db().find(p.reported_name); std::string displayable_name = (sd.displayable_name.empty() ? sd.reported_name : sd.displayable_name); @@ -1641,17 +1652,8 @@ bool storage_property_autoset_description(StorageProperty& p, StorageAttribute:: break; case StorageProperty::SubSection::erc_log: - // nothing here - break; - case StorageProperty::SubSection::phy_log: - // nothing here - break; - case StorageProperty::SubSection::directory_log: - // nothing here - break; - case StorageProperty::SubSection::unknown: // nothing break; @@ -1952,17 +1954,8 @@ WarningLevel storage_property_autoset_warning(StorageProperty& p) break; case StorageProperty::SubSection::erc_log: - // nothing here - break; - case StorageProperty::SubSection::phy_log: - // nothing here - break; - case StorageProperty::SubSection::directory_log: - // nothing here - break; - case StorageProperty::SubSection::unknown: // nothing here break;