diff --git a/init/action.cpp b/init/action.cpp index 94ccef2aa..a4f69367c 100644 --- a/init/action.cpp +++ b/init/action.cpp @@ -127,7 +127,7 @@ void Action::ExecuteCommand(const Command& command) const { // report such failures unless we're running at the DEBUG log level. bool report_failure = !result.has_value(); if (report_failure && android::base::GetMinimumLogSeverity() > android::base::DEBUG && - result.error_errno() == ENOENT) { + result.error().as_errno == ENOENT) { report_failure = false; } @@ -139,7 +139,7 @@ void Action::ExecuteCommand(const Command& command) const { LOG(INFO) << "Command '" << cmd_str << "' action=" << trigger_name << " (" << filename_ << ":" << command.line() << ") took " << duration.count() << "ms and " - << (result ? "succeeded" : "failed: " + result.error_string()); + << (result ? "succeeded" : "failed: " + result.error().as_string); } } diff --git a/init/keychords.cpp b/init/keychords.cpp index f5ac44f1f..d0ca3e709 100644 --- a/init/keychords.cpp +++ b/init/keychords.cpp @@ -41,7 +41,7 @@ Keychords::Keychords() : epoll_(nullptr), inotify_fd_(-1) {} Keychords::~Keychords() noexcept { if (inotify_fd_ >= 0) { - epoll_->UnregisterHandler(inotify_fd_).IgnoreError(); + epoll_->UnregisterHandler(inotify_fd_); ::close(inotify_fd_); } while (!registration_.empty()) GeteventCloseDevice(registration_.begin()->first); @@ -212,7 +212,7 @@ void Keychords::GeteventCloseDevice(const std::string& device) { auto it = registration_.find(device); if (it == registration_.end()) return; auto fd = (*it).second; - epoll_->UnregisterHandler(fd).IgnoreError(); + epoll_->UnregisterHandler(fd); registration_.erase(it); ::close(fd); } diff --git a/init/keychords_test.cpp b/init/keychords_test.cpp index e5a6fd35f..a3baeb116 100644 --- a/init/keychords_test.cpp +++ b/init/keychords_test.cpp @@ -213,7 +213,7 @@ TestFrame::TestFrame(const std::vector>& chords, EventHan } void TestFrame::RelaxForMs(std::chrono::milliseconds wait) { - epoll_.Wait(wait).IgnoreError(); + epoll_.Wait(wait); } void TestFrame::SetChord(int key, bool value) { diff --git a/init/mount_handler.cpp b/init/mount_handler.cpp index c8f0e7691..b0b63c585 100644 --- a/init/mount_handler.cpp +++ b/init/mount_handler.cpp @@ -121,7 +121,7 @@ MountHandler::MountHandler(Epoll* epoll) : epoll_(epoll), fp_(fopen("/proc/mount } MountHandler::~MountHandler() { - if (fp_) epoll_->UnregisterHandler(fileno(fp_.get())).IgnoreError(); + if (fp_) epoll_->UnregisterHandler(fileno(fp_.get())); } void MountHandler::MountHandlerFunction() { diff --git a/init/reboot.cpp b/init/reboot.cpp index 0966b6cd6..54f68bb8f 100644 --- a/init/reboot.cpp +++ b/init/reboot.cpp @@ -527,13 +527,13 @@ static void DoReboot(unsigned int cmd, const std::string& reason, const std::str // start all animation classes if stopped. if (do_shutdown_animation) { - service->Start().IgnoreError(); + service->Start(); } service->SetShutdownCritical(); // will not check animation class separately } if (do_shutdown_animation) { - bootAnim->Start().IgnoreError(); + bootAnim->Start(); surfaceFlinger->SetShutdownCritical(); bootAnim->SetShutdownCritical(); } diff --git a/init/result.h b/init/result.h index 0e3fd3d3f..baa680dfe 100644 --- a/init/result.h +++ b/init/result.h @@ -68,14 +68,14 @@ // if (!output) return Error() << "CalculateResult failed: " << output.error(); // UseOutput(*output); -#ifndef _INIT_RESULT_H -#define _INIT_RESULT_H +#pragma once #include #include #include -#include + +#include namespace android { namespace init { @@ -83,19 +83,24 @@ namespace init { struct ResultError { template ResultError(T&& error_string, int error_errno) - : error_string(std::forward(error_string)), error_errno(error_errno) {} + : as_string(std::forward(error_string)), as_errno(error_errno) {} - std::string error_string; - int error_errno; + template + operator android::base::expected() { + return android::base::unexpected(ResultError(as_string, as_errno)); + } + + std::string as_string; + int as_errno; }; inline std::ostream& operator<<(std::ostream& os, const ResultError& t) { - os << t.error_string; + os << t.as_string; return os; } inline std::ostream& operator<<(std::ostream& os, ResultError&& t) { - os << std::move(t.error_string); + os << std::move(t.as_string); return os; } @@ -104,6 +109,11 @@ class Error { Error() : errno_(0), append_errno_(false) {} Error(int errno_to_append) : errno_(errno_to_append), append_errno_(true) {} + template + operator android::base::expected() { + return android::base::unexpected(ResultError(str(), errno_)); + } + template Error&& operator<<(T&& t) { ss_ << std::forward(t); @@ -111,14 +121,14 @@ class Error { } Error&& operator<<(const ResultError& result_error) { - ss_ << result_error.error_string; - errno_ = result_error.error_errno; + ss_ << result_error.as_string; + errno_ = result_error.as_errno; return std::move(*this); } Error&& operator<<(ResultError&& result_error) { - ss_ << std::move(result_error.error_string); - errno_ = result_error.error_errno; + ss_ << std::move(result_error.as_string); + errno_ = result_error.as_errno; return std::move(*this); } @@ -151,63 +161,10 @@ inline Error ErrnoError() { } template -class [[nodiscard]] Result { - public: - Result() {} - - template , Result> && - sizeof...(V) == 0)>> - Result(U&& result, V&&... results) - : contents_(std::in_place_index_t<0>(), std::forward(result), - std::forward(results)...) {} - - Result(Error&& error) : contents_(std::in_place_index_t<1>(), error.str(), error.get_errno()) {} - Result(const ResultError& result_error) - : contents_(std::in_place_index_t<1>(), result_error.error_string, - result_error.error_errno) {} - Result(ResultError&& result_error) - : contents_(std::in_place_index_t<1>(), std::move(result_error.error_string), - result_error.error_errno) {} - - void IgnoreError() const {} - - bool has_value() const { return contents_.index() == 0; } - - T& value() & { return std::get<0>(contents_); } - const T& value() const & { return std::get<0>(contents_); } - T&& value() && { return std::get<0>(std::move(contents_)); } - const T&& value() const && { return std::get<0>(std::move(contents_)); } - - const ResultError& error() const & { return std::get<1>(contents_); } - ResultError&& error() && { return std::get<1>(std::move(contents_)); } - const ResultError&& error() const && { return std::get<1>(std::move(contents_)); } - - const std::string& error_string() const & { return std::get<1>(contents_).error_string; } - std::string&& error_string() && { return std::get<1>(std::move(contents_)).error_string; } - const std::string&& error_string() const && { - return std::get<1>(std::move(contents_)).error_string; - } - - int error_errno() const { return std::get<1>(contents_).error_errno; } - - explicit operator bool() const { return has_value(); } - - T& operator*() & { return value(); } - const T& operator*() const & { return value(); } - T&& operator*() && { return std::move(value()); } - const T&& operator*() const && { return std::move(value()); } - - T* operator->() { return &value(); } - const T* operator->() const { return &value(); } - - private: - std::variant contents_; -}; +using Result = android::base::expected; using Success = std::monostate; } // namespace init } // namespace android -#endif diff --git a/init/result_test.cpp b/init/result_test.cpp index 327b4444f..d3d04a015 100644 --- a/init/result_test.cpp +++ b/init/result_test.cpp @@ -74,8 +74,8 @@ TEST(result, result_error) { ASSERT_FALSE(result); ASSERT_FALSE(result.has_value()); - EXPECT_EQ(0, result.error_errno()); - EXPECT_EQ("failure1", result.error_string()); + EXPECT_EQ(0, result.error().as_errno); + EXPECT_EQ("failure1", result.error().as_string); } TEST(result, result_error_empty) { @@ -83,8 +83,8 @@ TEST(result, result_error_empty) { ASSERT_FALSE(result); ASSERT_FALSE(result.has_value()); - EXPECT_EQ(0, result.error_errno()); - EXPECT_EQ("", result.error_string()); + EXPECT_EQ(0, result.error().as_errno); + EXPECT_EQ("", result.error().as_string); } TEST(result, result_error_rvalue) { @@ -98,8 +98,8 @@ TEST(result, result_error_rvalue) { ASSERT_FALSE(MakeRvalueErrorResult()); ASSERT_FALSE(MakeRvalueErrorResult().has_value()); - EXPECT_EQ(0, MakeRvalueErrorResult().error_errno()); - EXPECT_EQ("failure1", MakeRvalueErrorResult().error_string()); + EXPECT_EQ(0, MakeRvalueErrorResult().error().as_errno); + EXPECT_EQ("failure1", MakeRvalueErrorResult().error().as_string); } TEST(result, result_errno_error) { @@ -110,8 +110,8 @@ TEST(result, result_errno_error) { ASSERT_FALSE(result); ASSERT_FALSE(result.has_value()); - EXPECT_EQ(test_errno, result.error_errno()); - EXPECT_EQ("failure1: "s + strerror(test_errno), result.error_string()); + EXPECT_EQ(test_errno, result.error().as_errno); + EXPECT_EQ("failure1: "s + strerror(test_errno), result.error().as_string); } TEST(result, result_errno_error_no_text) { @@ -122,8 +122,8 @@ TEST(result, result_errno_error_no_text) { ASSERT_FALSE(result); ASSERT_FALSE(result.has_value()); - EXPECT_EQ(test_errno, result.error_errno()); - EXPECT_EQ(strerror(test_errno), result.error_string()); + EXPECT_EQ(test_errno, result.error().as_errno); + EXPECT_EQ(strerror(test_errno), result.error().as_string); } TEST(result, result_error_from_other_result) { @@ -138,8 +138,8 @@ TEST(result, result_error_from_other_result) { ASSERT_FALSE(result2); ASSERT_FALSE(result2.has_value()); - EXPECT_EQ(0, result.error_errno()); - EXPECT_EQ(error_text, result.error_string()); + EXPECT_EQ(0, result.error().as_errno); + EXPECT_EQ(error_text, result.error().as_string); } TEST(result, result_error_through_ostream) { @@ -154,8 +154,8 @@ TEST(result, result_error_through_ostream) { ASSERT_FALSE(result2); ASSERT_FALSE(result2.has_value()); - EXPECT_EQ(0, result.error_errno()); - EXPECT_EQ(error_text, result.error_string()); + EXPECT_EQ(0, result.error().as_errno); + EXPECT_EQ(error_text, result.error().as_string); } TEST(result, result_errno_error_through_ostream) { @@ -174,12 +174,12 @@ TEST(result, result_errno_error_through_ostream) { ASSERT_FALSE(result2); ASSERT_FALSE(result2.has_value()); - EXPECT_EQ(test_errno, result.error_errno()); - EXPECT_EQ(error_text + ": " + strerror(test_errno), result.error_string()); + EXPECT_EQ(test_errno, result.error().as_errno); + EXPECT_EQ(error_text + ": " + strerror(test_errno), result.error().as_string); } TEST(result, constructor_forwarding) { - auto result = Result(5, 'a'); + auto result = Result(std::in_place, 5, 'a'); ASSERT_TRUE(result); ASSERT_TRUE(result.has_value()); @@ -298,8 +298,8 @@ TEST(result, result_result_with_failure) { auto result = return_result_result_with_error(); ASSERT_TRUE(result); ASSERT_FALSE(*result); - EXPECT_EQ("failure string", result->error_string()); - EXPECT_EQ(6, result->error_errno()); + EXPECT_EQ("failure string", (*result).error().as_string); + EXPECT_EQ(6, (*result).error().as_errno); } // This test requires that we disable the forwarding reference constructor if Result is the @@ -312,7 +312,9 @@ TEST(result, result_two_parameter_constructor_same_type) { int value_; }; - auto return_test_struct = []() -> Result { return {Result(6), 6}; }; + auto return_test_struct = []() -> Result { + return Result(std::in_place, Result(std::in_place, 6), 6); + }; auto result = return_test_struct(); ASSERT_TRUE(result); @@ -326,7 +328,7 @@ TEST(result, die_on_access_failed_result) { TEST(result, die_on_get_error_succesful_result) { Result result = "success"; - ASSERT_DEATH(result.error_string(), ""); + ASSERT_DEATH(result.error(), ""); } } // namespace init diff --git a/init/rlimit_parser.cpp b/init/rlimit_parser.cpp index 1e0754ac0..476a46ab8 100644 --- a/init/rlimit_parser.cpp +++ b/init/rlimit_parser.cpp @@ -77,7 +77,7 @@ Result> ParseRlimit(const std::vector& args) return Error() << "Could not parse hard limit '" << args[3] << "'"; } - return {resource, limit}; + return std::pair{resource, limit}; } } // namespace init diff --git a/init/rlimit_parser_test.cpp b/init/rlimit_parser_test.cpp index 659ba8acb..e690bf6d6 100644 --- a/init/rlimit_parser_test.cpp +++ b/init/rlimit_parser_test.cpp @@ -43,8 +43,8 @@ void TestRlimitFailure(std::vector input, const std::string& expect auto result = ParseRlimit(input); ASSERT_FALSE(result) << "input: " << input[1]; - EXPECT_EQ(expected_result, result.error_string()); - EXPECT_EQ(0, result.error_errno()); + EXPECT_EQ(expected_result, result.error().as_string); + EXPECT_EQ(0, result.error().as_errno); } TEST(rlimit, RlimitSuccess) { diff --git a/init/service.cpp b/init/service.cpp index ccc37b70c..6887d7b2f 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -1383,7 +1383,7 @@ void ServiceList::MarkServicesUpdate() { continue; } if (auto result = service->Start(); !result) { - LOG(ERROR) << result.error_string(); + LOG(ERROR) << result.error().as_string; } } delayed_service_names_.clear(); diff --git a/init/subcontext.cpp b/init/subcontext.cpp index 092c51ceb..467b0d237 100644 --- a/init/subcontext.cpp +++ b/init/subcontext.cpp @@ -141,8 +141,8 @@ void SubcontextProcess::RunCommand(const SubcontextCommand::ExecuteCommand& exec reply->set_success(true); } else { auto* failure = reply->mutable_failure(); - failure->set_error_string(result.error_string()); - failure->set_error_errno(result.error_errno()); + failure->set_error_string(result.error().as_string); + failure->set_error_errno(result.error().as_errno); } } @@ -177,7 +177,7 @@ void SubcontextProcess::MainLoop() { auto init_message = ReadMessage(init_fd_); if (!init_message) { - if (init_message.error_errno() == 0) { + if (init_message.error().as_errno == 0) { // If the init file descriptor was closed, let's exit quietly. If // this was accidental, init will restart us. If init died, this // avoids calling abort(3) unnecessarily. diff --git a/init/subcontext_benchmark.cpp b/init/subcontext_benchmark.cpp index eae03e37a..630799378 100644 --- a/init/subcontext_benchmark.cpp +++ b/init/subcontext_benchmark.cpp @@ -39,7 +39,7 @@ static void BenchmarkSuccess(benchmark::State& state) { free(context); while (state.KeepRunning()) { - subcontext.Execute(std::vector{"return_success"}).IgnoreError(); + subcontext.Execute(std::vector{"return_success"}); } if (subcontext.pid() > 0) { diff --git a/init/subcontext_test.cpp b/init/subcontext_test.cpp index 230203ae0..c0662a471 100644 --- a/init/subcontext_test.cpp +++ b/init/subcontext_test.cpp @@ -69,7 +69,7 @@ TEST(subcontext, CheckDifferentPid) { auto result = subcontext.Execute(std::vector{"return_pids_as_error"}); ASSERT_FALSE(result); - auto pids = Split(result.error_string(), " "); + auto pids = Split(result.error().as_string, " "); ASSERT_EQ(2U, pids.size()); auto our_pid = std::to_string(getpid()); EXPECT_NE(our_pid, pids[0]); @@ -116,7 +116,7 @@ TEST(subcontext, MultipleCommands) { auto result = subcontext.Execute(std::vector{"return_words_as_error"}); ASSERT_FALSE(result); - EXPECT_EQ(Join(expected_words, " "), result.error_string()); + EXPECT_EQ(Join(expected_words, " "), result.error().as_string); EXPECT_EQ(first_pid, subcontext.pid()); }); } @@ -130,7 +130,7 @@ TEST(subcontext, RecoverAfterAbort) { auto result2 = subcontext.Execute(std::vector{"generate_sane_error"}); ASSERT_FALSE(result2); - EXPECT_EQ("Sane error!", result2.error_string()); + EXPECT_EQ("Sane error!", result2.error().as_string); EXPECT_NE(subcontext.pid(), first_pid); }); } @@ -139,7 +139,7 @@ TEST(subcontext, ContextString) { RunTest([](auto& subcontext, auto& context_string) { auto result = subcontext.Execute(std::vector{"return_context_as_error"}); ASSERT_FALSE(result); - ASSERT_EQ(context_string, result.error_string()); + ASSERT_EQ(context_string, result.error().as_string); }); } @@ -167,7 +167,7 @@ TEST(subcontext, ExpandArgsFailure) { }; auto result = subcontext.ExpandArgs(args); ASSERT_FALSE(result); - EXPECT_EQ("Failed to expand '" + args[1] + "'", result.error_string()); + EXPECT_EQ("Failed to expand '" + args[1] + "'", result.error().as_string); }); } diff --git a/init/util_test.cpp b/init/util_test.cpp index 1b5afba27..8bf672c0e 100644 --- a/init/util_test.cpp +++ b/init/util_test.cpp @@ -34,7 +34,7 @@ TEST(util, ReadFile_ENOENT) { auto file_contents = ReadFile("/proc/does-not-exist"); EXPECT_EQ(ENOENT, errno); ASSERT_FALSE(file_contents); - EXPECT_EQ("open() failed: No such file or directory", file_contents.error_string()); + EXPECT_EQ("open() failed: No such file or directory", file_contents.error().as_string); } TEST(util, ReadFileGroupWriteable) { @@ -45,7 +45,7 @@ TEST(util, ReadFileGroupWriteable) { EXPECT_NE(-1, fchmodat(AT_FDCWD, tf.path, 0620, AT_SYMLINK_NOFOLLOW)) << strerror(errno); auto file_contents = ReadFile(tf.path); ASSERT_FALSE(file_contents) << strerror(errno); - EXPECT_EQ("Skipping insecure file", file_contents.error_string()); + EXPECT_EQ("Skipping insecure file", file_contents.error().as_string); } TEST(util, ReadFileWorldWiteable) { @@ -56,7 +56,7 @@ TEST(util, ReadFileWorldWiteable) { EXPECT_NE(-1, fchmodat(AT_FDCWD, tf.path, 0602, AT_SYMLINK_NOFOLLOW)) << strerror(errno); auto file_contents = ReadFile(tf.path); ASSERT_FALSE(file_contents) << strerror(errno); - EXPECT_EQ("Skipping insecure file", file_contents.error_string()); + EXPECT_EQ("Skipping insecure file", file_contents.error().as_string); } TEST(util, ReadFileSymbolicLink) { @@ -65,7 +65,8 @@ TEST(util, ReadFileSymbolicLink) { auto file_contents = ReadFile("/charger"); EXPECT_EQ(ELOOP, errno); ASSERT_FALSE(file_contents); - EXPECT_EQ("open() failed: Too many symbolic links encountered", file_contents.error_string()); + EXPECT_EQ("open() failed: Too many symbolic links encountered", + file_contents.error().as_string); } TEST(util, ReadFileSuccess) { @@ -130,7 +131,7 @@ TEST(util, DecodeUid) { decoded_uid = DecodeUid("toot"); EXPECT_FALSE(decoded_uid); - EXPECT_EQ("getpwnam failed: No such file or directory", decoded_uid.error_string()); + EXPECT_EQ("getpwnam failed: No such file or directory", decoded_uid.error().as_string); decoded_uid = DecodeUid("123"); EXPECT_TRUE(decoded_uid);