Fixed a few clang-tidy warnings in applib.

src/applib/cmdex.cpp

@@ -15,6 +15,7 @@ Copyright:
 #include <string>
 #include <sys/types.h>
 #include <cerrno>  // errno (not std::errno, it may be a macro)
+#include <array>
 
 #ifdef _WIN32
 // 	#include <io.h>  // close()
@@ -69,7 +70,7 @@ extern "C" {
 		DBG_FUNCTION_ENTER_MSG;
 		auto* self = static_cast<Cmdex*>(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<Cmdex*>(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<gchar, count> 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<void>("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);
 }
 
 

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

src/applib/cmdex_sync.cpp

@@ -16,17 +16,10 @@ Copyright:
 
 
 
-namespace {
-
-	/// "Execution finished" signal
-	sigc::signal<void, const CmdexSyncCommandInfo&> s_cmdex_sync_signal_execute_finish;
-
-}
-
-
-
 cmdex_signal_execute_finish_type& cmdex_sync_signal_execute_finish()
 {
+	/// "Execution finished" signal
+	static sigc::signal<void, const CmdexSyncCommandInfo&> 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.

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

src/applib/cmdex_sync_gui.cpp

@@ -13,6 +13,8 @@ Copyright:
 #include <gtkmm.h>  // Gtk::Main
 #include <gdkmm.h>
 
+#include <memory>
+
 #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<Gtk::MessageDialog>(*parent, "", false,
 				CMDEX_DIALOG_MESSAGE_TYPE, Gtk::BUTTONS_CANCEL);
 	} else {
-		running_dialog_ = new Gtk::MessageDialog("", false,
+		running_dialog_ = std::make_unique<Gtk::MessageDialog>("", 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();
 }
 
 

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 <gtkmm.h>
+#include <memory>
 
 #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<Gtk::MessageDialog> 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.

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 <gtkmm.h>
 #include <string>
 
-#include <gtkmm.h>
 
 
 // These functions won't return until the dialogs are closed.

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

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;

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<StorageProperty>& get_properties() const;
+		[[nodiscard]] const std::vector<StorageProperty>& get_properties() const;
 
 
 

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<StorageDevicePtr>& put_drives_here,
+		std::string detect(std::vector<StorageDevicePtr>& drives,
 				const ExecutorFactoryPtr& ex_factory);
 
 
@@ -50,21 +50,21 @@ class StorageDetector {
 
 
 		/// Add device patterns to drive detection blacklist
-		void add_blacklist_patterns(std::vector<std::string>& patterns)
+		void add_blacklist_patterns(const std::vector<std::string>& patterns)
 		{
 			blacklist_patterns_.insert(blacklist_patterns_.end(), patterns.begin(), patterns.end());
 		}
 
 
 		/// Get all errors produced by fetch_basic_data().
-		const std::vector<std::string>& get_fetch_data_errors() const
+		[[nodiscard]] const std::vector<std::string>& get_fetch_data_errors() const
 		{
 			return fetch_data_errors_;
 		}
 
 
 		/// Get command output for each error in get_fetch_data_errors().
-		const std::vector<std::string>& get_fetch_data_error_outputs() const
+		[[nodiscard]] const std::vector<std::string>& get_fetch_data_error_outputs() const
 		{
 			return fetch_data_error_outputs_;
 		}

src/applib/storage_detector_linux.cpp

@@ -121,14 +121,18 @@ inline std::string detect_drives_linux_udev_byid(std::vector<std::string>& devic
 
 
 /// Cache of file->contents, caches read files.
-std::map<hz::fs::path, std::string> s_read_file_cache;
+inline std::map<hz::fs::path, std::string>& get_read_file_cache_ref()
+{
+	static std::map<hz::fs::path, std::string> 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");

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<CmdexSync>& 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();
 }
 
 

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<AttributeType, std::string> 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<UpdateType, std::string> 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<FailTime, std::string> 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<Status, std::string> 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<Status, StatusSeverity> 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<Section, std::string> 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<SubSection, std::string> 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<std::monostate>(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<std::monostate>(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<typename T>
-		bool is_value_type() const
+		[[nodiscard]] bool is_value_type() const
 		{
 			return std::holds_alternative<T>(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;

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<StorageAttribute>().id, disk_type);
+		AttributeDescription attr = get_attribute_db().find(p.reported_name, p.get_value<StorageAttribute>().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;