tequilaOS/platform_system_core
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

init: replace Result<> with expected<>

Browse source 
Android-base has an implementation of the future std::expected<>.
This provides the same baseline functionality as Result<>, so use it
instead of our own version.

Bug: 132145659
Test: boot, init unit tests
Change-Id: I11e61bcb5719b262a6420483ed51a762826a9e23
This commit is contained in:
Tom Cherry 2019-05-16 16:54:49 -07:00
parent 691e0e154a
commit 9949ec5f56
14 changed files with 73 additions and 113 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
init/action.cpp
View file

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

4
init/keychords.cpp
View file

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

2
init/keychords_test.cpp
View file

@ -213,7 +213,7 @@ TestFrame::TestFrame(const std::vector<const std::vector<int>>& chords, EventHan
}
void TestFrame::RelaxForMs(std::chrono::milliseconds wait) {
epoll_.Wait(wait).IgnoreError();
epoll_.Wait(wait);
}
void TestFrame::SetChord(int key, bool value) {

2
init/mount_handler.cpp
View file

@ -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() {

4
init/reboot.cpp
View file

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

89
init/result.h
View file

@ -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 <errno.h>
#include <sstream>
#include <string>
#include <variant>
#include <android-base/expected.h>
namespace android {
namespace init {
@ -83,19 +83,24 @@ namespace init {
struct ResultError {
template <typename T>
ResultError(T&& error_string, int error_errno)
: error_string(std::forward<T>(error_string)), error_errno(error_errno) {}
: as_string(std::forward<T>(error_string)), as_errno(error_errno) {}
std::string error_string;
int error_errno;
template <typename T>
operator android::base::expected<T, ResultError>() {
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 <typename T>
operator android::base::expected<T, ResultError>() {
return android::base::unexpected(ResultError(str(), errno_));
}
template <typename T>
Error&& operator<<(T&& t) {
ss_ << std::forward<T>(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 <typename T>
class [[nodiscard]] Result {
public:
Result() {}
template <typename U, typename... V,
typename = std::enable_if_t<!(std::is_same_v<std::decay_t<U>, Result<T>> &&
sizeof...(V) == 0)>>
Result(U&& result, V&&... results)
: contents_(std::in_place_index_t<0>(), std::forward<U>(result),
std::forward<V>(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<T, ResultError> contents_;
};
using Result = android::base::expected<T, ResultError>;
using Success = std::monostate;
} // namespace init
} // namespace android
#endif

44
init/result_test.cpp
View file

@ -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<std::string>(5, 'a');
auto result = Result<std::string>(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<T> is the
@ -312,7 +312,9 @@ TEST(result, result_two_parameter_constructor_same_type) {
int value_;
};
auto return_test_struct = []() -> Result<TestStruct> { return {Result<TestStruct>(6), 6}; };
auto return_test_struct = []() -> Result<TestStruct> {
return Result<TestStruct>(std::in_place, Result<TestStruct>(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<std::string> result = "success";
ASSERT_DEATH(result.error_string(), "");
ASSERT_DEATH(result.error(), "");
}
} // namespace init

2
init/rlimit_parser.cpp
View file

@ -77,7 +77,7 @@ Result<std::pair<int, rlimit>> ParseRlimit(const std::vector<std::string>& args)
return Error() << "Could not parse hard limit '" << args[3] << "'";
}
return {resource, limit};
return std::pair{resource, limit};
}
} // namespace init

4
init/rlimit_parser_test.cpp
View file

@ -43,8 +43,8 @@ void TestRlimitFailure(std::vector<std::string> 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) {

2
init/service.cpp
View file

@ -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();

6
init/subcontext.cpp
View file

@ -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.

2
init/subcontext_benchmark.cpp
View file

@ -39,7 +39,7 @@ static void BenchmarkSuccess(benchmark::State& state) {
free(context);
while (state.KeepRunning()) {
subcontext.Execute(std::vector<std::string>{"return_success"}).IgnoreError();
subcontext.Execute(std::vector<std::string>{"return_success"});
}
if (subcontext.pid() > 0) {

10
init/subcontext_test.cpp
View file

@ -69,7 +69,7 @@ TEST(subcontext, CheckDifferentPid) {
auto result = subcontext.Execute(std::vector<std::string>{"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<std::string>{"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<std::string>{"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<std::string>{"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);
});
}

11
init/util_test.cpp
View file

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