Skip to content

Commit

Permalink
Switched most of message-based error handling to std::expected.
Browse files Browse the repository at this point in the history 
Refactored enum class naming to use ThisCase.
Decorated most getters with [[nodiscard]].
  • Loading branch information
@ashaduri
ashaduri committed Mar 22, 2024
1 parent 6edcf19 commit 351f5ed
Show file tree
Hide file tree
Showing 53 changed files with 1,039 additions and 917 deletions.
10 changes: 5 additions & 5 deletions src/applib/app_builder_widget.h
View file
Expand Up @@ -31,7 +31,7 @@ License: GNU General Public License v3.0 only
#define APP_BUILDER_CONNECT(ui_element, signal_name, callback) \
if (true) { \
if (!(ui_element)) \
this->lookup_widget(#ui_element, ui_element); \
[[maybe_unused]] bool found = this->lookup_widget(#ui_element, ui_element); \
if (ui_element) { \
(ui_element)->signal_ ## signal_name ().connect(sigc::mem_fun(*this, &std::remove_reference_t<decltype(*this)>::callback)); \
} \
Expand Down Expand Up @@ -85,24 +85,24 @@ class AppBuilderWidget : public WidgetType, public WindowInstanceManager<Child,


/// Get UI resource
Glib::RefPtr<Gtk::Builder> get_ui();
[[nodiscard]] Glib::RefPtr<Gtk::Builder> get_ui();


/// Find a widget in UI and return it.
/// \return nullptr if widget was not found.
Gtk::Widget* lookup_widget(const Glib::ustring& name);
[[nodiscard]] Gtk::Widget* lookup_widget(const Glib::ustring& name);


/// Find a widget in UI and return it.
/// \return nullptr if widget was not found.
template<typename WidgetPtr>
WidgetPtr lookup_widget(const Glib::ustring& name);
[[nodiscard]] WidgetPtr lookup_widget(const Glib::ustring& name);


/// Find a widget in UI and return it in \ref w.
/// \return false if widget was not found.
template<typename Widget>
bool lookup_widget(const Glib::ustring& name, Widget*& w);
[[nodiscard]] bool lookup_widget(const Glib::ustring& name, Widget*& w);


protected:
Expand Down
28 changes: 14 additions & 14 deletions src/applib/async_command_executor.cpp
View file
Expand Up @@ -53,14 +53,14 @@ extern "C" {
/// Child process stdout handler callback
inline gboolean cmdex_on_channel_io_stdout(GIOChannel* source, GIOCondition cond, gpointer data)
{
return AsyncCommandExecutor::on_channel_io(source, cond, static_cast<AsyncCommandExecutor*>(data), AsyncCommandExecutor::Channel::standard_output);
return AsyncCommandExecutor::on_channel_io(source, cond, static_cast<AsyncCommandExecutor*>(data), AsyncCommandExecutor::Channel::StandardOutput);
}


/// Child process stderr handler callback
inline gboolean cmdex_on_channel_io_stderr(GIOChannel* source, GIOCondition cond, gpointer data)
{
return AsyncCommandExecutor::on_channel_io(source, cond, static_cast<AsyncCommandExecutor*>(data), AsyncCommandExecutor::Channel::standard_error);
return AsyncCommandExecutor::on_channel_io(source, cond, static_cast<AsyncCommandExecutor*>(data), AsyncCommandExecutor::Channel::StandardError);
}


Expand Down Expand Up @@ -142,7 +142,7 @@ bool AsyncCommandExecutor::execute()
}
catch(Glib::ShellError& e)
{
push_error(Error<void>("gshell", ErrorLevel::error, e.what()));
push_error(Error<void>("gshell", ErrorLevel::Error, e.what()));
return false;
}

Expand Down Expand Up @@ -184,7 +184,7 @@ bool AsyncCommandExecutor::execute()
}
catch(Glib::SpawnError& e) {
// no data is returned to &-parameters on error.
push_error(Error<void>("gspawn", ErrorLevel::error, e.what()));
push_error(Error<void>("gspawn", ErrorLevel::Error, e.what()));
// Restore CWD
if (path_changed) {
std::error_code dummy_ec;
Expand Down Expand Up @@ -280,7 +280,7 @@ bool AsyncCommandExecutor::try_stop(hz::Signal sig)
}

// Possible: EPERM (no permissions), ESRCH (no such process, or zombie)
push_error(Error<int>("errno", ErrorLevel::error, errno));
push_error(Error<int>("errno", ErrorLevel::Error, errno));

DBG_FUNCTION_EXIT_MSG;
return false;
Expand Down Expand Up @@ -355,7 +355,7 @@ void AsyncCommandExecutor::stopped_cleanup()
// translate the exit_code into a message
const std::string msg = (translator_func_ ? translator_func_(exit_status)
: "[no translator function, exit code: " + std::to_string(exit_status));
push_error(Error<int>("exit", ErrorLevel::warn, exit_status, msg));
push_error(Error<int>("exit", ErrorLevel::Warn, exit_status, msg));
}

} else {
Expand All @@ -365,9 +365,9 @@ void AsyncCommandExecutor::stopped_cleanup()
// If it's not our signal, treat as error.
// Note: they will never match under win32
if (sig_num != this->kill_signal_sent_) {
push_error(Error<int>("signal", ErrorLevel::error, sig_num));
push_error(Error<int>("signal", ErrorLevel::Error, sig_num));
} else { // it's our signal, treat as warning
push_error(Error<int>("signal", ErrorLevel::warn, sig_num));
push_error(Error<int>("signal", ErrorLevel::Warn, sig_num));
}
}
}
Expand Down Expand Up @@ -395,10 +395,10 @@ void AsyncCommandExecutor::on_child_watch_handler([[maybe_unused]] GPid arg_pid,

// These are needed because Windows doesn't read the remaining data otherwise.
g_io_channel_flush(self->channel_stdout_, nullptr);
on_channel_io(self->channel_stdout_, GIOCondition(0), self, Channel::standard_output);
on_channel_io(self->channel_stdout_, GIOCondition(0), self, Channel::StandardOutput);

g_io_channel_flush(self->channel_stderr_, nullptr);
on_channel_io(self->channel_stderr_, GIOCondition(0), self, Channel::standard_error);
on_channel_io(self->channel_stderr_, GIOCondition(0), self, Channel::StandardError);

if (self->channel_stdout_) {
g_io_channel_shutdown(self->channel_stdout_, FALSE, nullptr);
Expand Down Expand Up @@ -452,7 +452,7 @@ gboolean AsyncCommandExecutor::on_channel_io(GIOChannel* channel,
continue_events = false; // there'll be no more data
}

DBG_ASSERT_RETURN(channel_type == Channel::standard_output || channel_type == Channel::standard_error, false);
DBG_ASSERT_RETURN(channel_type == Channel::StandardOutput || channel_type == Channel::StandardError, false);

// const gsize count = 4 * 1024;
// read the bytes one by one. without this, a buffered iochannel hangs while waiting for data.
Expand All @@ -461,9 +461,9 @@ gboolean AsyncCommandExecutor::on_channel_io(GIOChannel* channel,
std::array<gchar, count> buf = {0};

std::string* output_str = nullptr;
if (channel_type == Channel::standard_output) {
if (channel_type == Channel::StandardOutput) {
output_str = &self->str_stdout_;
} else if (channel_type == Channel::standard_error) {
} else if (channel_type == Channel::StandardError) {
output_str = &self->str_stderr_;
}
DBG_ASSERT_RETURN(output_str, false);
Expand All @@ -478,7 +478,7 @@ gboolean AsyncCommandExecutor::on_channel_io(GIOChannel* channel,
output_str->append(buf.data(), bytes_read);

if (channel_error) {
self->push_error(Error<void>("giochannel", ErrorLevel::error, channel_error->message));
self->push_error(Error<void>("giochannel", ErrorLevel::Error, channel_error->message));
g_error_free(channel_error);
break; // stop on next invocation (is this correct?)
}
Expand Down
8 changes: 4 additions & 4 deletions src/applib/async_command_executor.h
View file
Expand Up @@ -123,11 +123,11 @@ class AsyncCommandExecutor : public hz::ErrorHolder {
/// If stdout_make_str_as_available_ is false, call this after stopped_cleanup(),
/// before next execute(). If it's true, you may call this before the command has
/// stopped, but it will decrease performance significantly.
std::string get_stdout_str(bool clear_existing = false);
[[nodiscard]] std::string get_stdout_str(bool clear_existing = false);


/// See notes for \ref get_stdout_str().
std::string get_stderr_str(bool clear_existing = false);
[[nodiscard]] std::string get_stderr_str(bool clear_existing = false);


/// Return execution time, in seconds. Call this after execute().
Expand All @@ -149,8 +149,8 @@ class AsyncCommandExecutor : public hz::ErrorHolder {

/// Channel type, for passing to callbacks
enum class Channel {
standard_output,
standard_error
StandardOutput,
StandardError
};


Expand Down
136 changes: 68 additions & 68 deletions src/applib/ata_storage_property.cpp
View file
Expand Up @@ -42,9 +42,9 @@ std::ostream& operator<< (std::ostream& os, const AtaStorageCapability& p)
std::string AtaStorageAttribute::get_attr_type_name(AtaStorageAttribute::AttributeType type)
{
static const std::unordered_map<AttributeType, std::string> m {
{AttributeType::unknown, "[unknown]"},
{AttributeType::prefail, "pre-failure"},
{AttributeType::old_age, "old age"},
{AttributeType::Unknown, "[unknown]"},
{AttributeType::Prefail, "pre-failure"},
{AttributeType::OldAge, "old age"},
};
if (auto iter = m.find(type); iter != m.end()) {
return iter->second;
Expand All @@ -57,9 +57,9 @@ std::string AtaStorageAttribute::get_attr_type_name(AtaStorageAttribute::Attribu
std::string AtaStorageAttribute::get_update_type_name(AtaStorageAttribute::UpdateType type)
{
static const std::unordered_map<UpdateType, std::string> m {
{UpdateType::unknown, "[unknown]"},
{UpdateType::always, "continuously"},
{UpdateType::offline, "on offline data collect."},
{UpdateType::Unknown, "[unknown]"},
{UpdateType::Always, "continuously"},
{UpdateType::Offline, "on offline data collect."},
};
if (auto iter = m.find(type); iter != m.end()) {
return iter->second;
Expand All @@ -72,10 +72,10 @@ std::string AtaStorageAttribute::get_update_type_name(AtaStorageAttribute::Updat
std::string AtaStorageAttribute::get_fail_time_name(AtaStorageAttribute::FailTime type)
{
static const std::unordered_map<FailTime, std::string> m {
{FailTime::unknown, "[unknown]"},
{FailTime::none, "never"},
{FailTime::past, "in the past"},
{FailTime::now, "now"},
{FailTime::Unknown, "[unknown]"},
{FailTime::None, "never"},
{FailTime::Past, "in the past"},
{FailTime::Now, "now"},
};
if (auto iter = m.find(type); iter != m.end()) {
return iter->second;
Expand Down Expand Up @@ -192,26 +192,26 @@ std::string AtaStorageErrorBlock::get_displayable_error_types(const std::vector<
WarningLevel AtaStorageErrorBlock::get_warning_level_for_error_type(const std::string& type)
{
static const std::map<std::string, WarningLevel> m = {
{"ABRT", WarningLevel::none},
{"AMNF", WarningLevel::alert},
{"CCTO", WarningLevel::warning},
{"EOM", WarningLevel::warning},
{"ICRC", WarningLevel::warning},
{"IDNF", WarningLevel::alert},
{"ILI", WarningLevel::notice},
{"MC", WarningLevel::none},
{"MCR", WarningLevel::none},
{"NM", WarningLevel::none},
{"obs", WarningLevel::none},
{"TK0NF", WarningLevel::alert},
{"UNC", WarningLevel::alert},
{"WP", WarningLevel::none},
{"ABRT", WarningLevel::None},
{"AMNF", WarningLevel::Alert},
{"CCTO", WarningLevel::Warning},
{"EOM", WarningLevel::Warning},
{"ICRC", WarningLevel::Warning},
{"IDNF", WarningLevel::Alert},
{"ILI", WarningLevel::Notice},
{"MC", WarningLevel::None},
{"MCR", WarningLevel::None},
{"NM", WarningLevel::None},
{"obs", WarningLevel::None},
{"TK0NF", WarningLevel::Alert},
{"UNC", WarningLevel::Alert},
{"WP", WarningLevel::None},
};

if (m.find(type) != m.end()) {
return m.at(type);
}
return WarningLevel::none; // unknown error
return WarningLevel::None; // unknown error
}


Expand Down Expand Up @@ -244,18 +244,18 @@ std::ostream& operator<< (std::ostream& os, const AtaStorageErrorBlock& b)
std::string AtaStorageSelftestEntry::get_status_displayable_name(AtaStorageSelftestEntry::Status s)
{
static const std::unordered_map<Status, std::string> m {
{Status::unknown, "[unknown]"},
{Status::completed_no_error, "Completed without error"},
{Status::aborted_by_host, "Manually aborted"},
{Status::interrupted, "Interrupted (host reset)"},
{Status::fatal_or_unknown, "Fatal or unknown error"},
{Status::compl_unknown_failure, "Completed with unknown failure"},
{Status::compl_electrical_failure, "Completed with electrical failure"},
{Status::compl_servo_failure, "Completed with servo/seek failure"},
{Status::compl_read_failure, "Completed with read failure"},
{Status::compl_handling_damage, "Completed: handling damage"},
{Status::in_progress, "In progress"},
{Status::reserved, "Unknown / reserved state"},
{Status::Unknown, "[unknown]"},
{Status::CompletedNoError, "Completed without error"},
{Status::AbortedByHost, "Manually aborted"},
{Status::Interrupted, "Interrupted (host reset)"},
{Status::FatalOrUnknown, "Fatal or unknown error"},
{Status::ComplUnknownFailure, "Completed with unknown failure"},
{Status::ComplElectricalFailure, "Completed with electrical failure"},
{Status::ComplServoFailure, "Completed with servo/seek failure"},
{Status::ComplReadFailure, "Completed with read failure"},
{Status::ComplHandlingDamage, "Completed: handling damage"},
{Status::InProgress, "In progress"},
{Status::Reserved, "Unknown / reserved state"},
};
if (auto iter = m.find(s); iter != m.end()) {
return iter->second;
Expand All @@ -268,30 +268,30 @@ std::string AtaStorageSelftestEntry::get_status_displayable_name(AtaStorageSelft
AtaStorageSelftestEntry::StatusSeverity AtaStorageSelftestEntry::get_status_severity(AtaStorageSelftestEntry::Status s)
{
static const std::unordered_map<Status, StatusSeverity> m {
{Status::unknown, StatusSeverity::none},
{Status::completed_no_error, StatusSeverity::none},
{Status::aborted_by_host, StatusSeverity::warning},
{Status::interrupted, StatusSeverity::warning},
{Status::fatal_or_unknown, StatusSeverity::error},
{Status::compl_unknown_failure, StatusSeverity::error},
{Status::compl_electrical_failure, StatusSeverity::error},
{Status::compl_servo_failure, StatusSeverity::error},
{Status::compl_read_failure, StatusSeverity::error},
{Status::compl_handling_damage, StatusSeverity::error},
{Status::in_progress, StatusSeverity::none},
{Status::reserved, StatusSeverity::none},
{Status::Unknown, StatusSeverity::None},
{Status::CompletedNoError, StatusSeverity::None},
{Status::AbortedByHost, StatusSeverity::Warning},
{Status::Interrupted, StatusSeverity::Warning},
{Status::FatalOrUnknown, StatusSeverity::Error},
{Status::ComplUnknownFailure, StatusSeverity::Error},
{Status::ComplElectricalFailure, StatusSeverity::Error},
{Status::ComplServoFailure, StatusSeverity::Error},
{Status::ComplReadFailure, StatusSeverity::Error},
{Status::ComplHandlingDamage, StatusSeverity::Error},
{Status::InProgress, StatusSeverity::None},
{Status::Reserved, StatusSeverity::None},
};
if (auto iter = m.find(s); iter != m.end()) {
return iter->second;
}
return StatusSeverity::none;
return StatusSeverity::None;
}



std::string AtaStorageSelftestEntry::get_status_str() const
{
return (status == Status::unknown ? status_str : get_status_displayable_name(status));
return (status == Status::Unknown ? status_str : get_status_displayable_name(status));
}


Expand Down Expand Up @@ -323,10 +323,10 @@ std::ostream& operator<< (std::ostream& os, const AtaStorageSelftestEntry& b)
std::string AtaStorageProperty::get_section_name(AtaStorageProperty::Section s)
{
static const std::unordered_map<Section, std::string> m {
{Section::unknown, "unknown"},
{Section::info, "info"},
{Section::data, "data"},
{Section::internal, "internal"},
{Section::Unknown, "unknown"},
{Section::Info, "info"},
{Section::Data, "data"},
{Section::Internal, "internal"},
};
if (auto iter = m.find(s); iter != m.end()) {
return iter->second;
Expand All @@ -339,18 +339,18 @@ std::string AtaStorageProperty::get_section_name(AtaStorageProperty::Section s)
std::string AtaStorageProperty::get_subsection_name(AtaStorageProperty::SubSection s)
{
static const std::unordered_map<SubSection, std::string> m {
{SubSection::unknown, "unknown"},
{SubSection::health, "health"},
{SubSection::capabilities, "capabilities"},
{SubSection::attributes, "attributes"},
{SubSection::devstat, "devstat"},
{SubSection::error_log, "error_log"},
{SubSection::selftest_log, "selftest_log"},
{SubSection::selective_selftest_log, "selective_selftest_log"},
{SubSection::temperature_log, "temperature_log"},
{SubSection::erc_log, "erc_log"},
{SubSection::phy_log, "phy_log"},
{SubSection::directory_log, "directory_log"},
{SubSection::Unknown, "unknown"},
{SubSection::Health, "health"},
{SubSection::Capabilities, "capabilities"},
{SubSection::Attributes, "attributes"},
{SubSection::Devstat, "devstat"},
{SubSection::ErrorLog, "error_log"},
{SubSection::SelftestLog, "selftest_log"},
{SubSection::SelectiveSelftestLog, "selective_selftest_log"},
{SubSection::TemperatureLog, "temperature_log"},
{SubSection::ErcLog, "erc_log"},
{SubSection::PhyLog, "phy_log"},
{SubSection::DirectoryLog, "directory_log"},
};
if (auto iter = m.find(s); iter != m.end()) {
return iter->second;
Expand Down Expand Up @@ -399,7 +399,7 @@ void AtaStorageProperty::dump(std::ostream& os, std::size_t internal_offset) con
const std::string offset(internal_offset, ' ');

os << offset << "[" << get_section_name(section)
<< (section == Section::data ? (", " + get_subsection_name(subsection)) : "") << "]"
<< (section == Section::Data ? (", " + get_subsection_name(subsection)) : "") << "]"
<< " " << generic_name
// << (generic_name == reported_name ? "" : (" (" + reported_name + ")"))
<< ": [" << get_value_type_name() << "] ";
Expand Down

0 comments on commit 351f5ed

Please sign in to comment.