openkylin
/
platform_system_core
mirror of https://gitee.com/openkylin/platform_system_core.git
9
0
Fork 0
Code Issues Projects Releases Wiki Activity

init: pass errors from one Result<T> to another better 

Result<T> currently has two problems,
1) A failing Result<T> cannot be easily constructed from a Result<U>'s
error.
2) errno is lost when passing .error() through multiple Result<T>'s

This change fixes both problems having Result<T>::error() return a
ResultError class that contains the std::string error message and int
errno.

It additionally has ostream operators to continue to allow printing
the error string directly to an ostream and also to pass the errno
through to another Result<T> class via Error() creation.

Lastly, it provides a new constructor for Result<T> for ResultError,
such that a Result<T> can be constructed from Result<U>::error().

Test: boot bullhead, init unit tests
Change-Id: Id9614b727cdabd2f5498b0da0e598e9aff7d9ae0
This commit is contained in:
Tom Cherry 2017-08-22 16:07:15 -07:00
parent 6de21f1112
commit 130e3d7204
4 changed files with 166 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
init/action.cpp
View File

@ -98,7 +98,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());
<< (result ? "succeeded" : "failed: " + result.error_string());
}
}

111
init/result.h
View File

@ -21,9 +21,13 @@
// There are 3 classes that implement this functionality and one additional helper type.
//
// Result<T> either contains a member of type T that can be accessed using similar semantics as
// std::optional<T> or it contains a std::string describing an error, which can be accessed via
// std::optional<T> or it contains a ResultError describing an error, which can be accessed via
// Result<T>::error().
//
// ResultError is a type that contains both a std::string describing the error and a copy of errno
// from when the error occurred. ResultError can be used in an ostream directly to print its
// string value.
//
// Success is a typedef that aids in creating Result<T> that do not contain a return value.
// Result<Success> is the correct return type for a function that either returns successfully or
// returns an error value. Returning Success() from a function that returns Result<Success> is the
@ -33,10 +37,20 @@
// to T or from the constructor arguments for T. This allows you to return a type T directly from
// a function that returns Result<T>.
//
// Error and ErrnoError are used to construct a Result<T> that has failed. Each of these classes
// take an ostream as an input and are implicitly cast to a Result<T> containing that failure.
// ErrnoError() additionally appends ": " + strerror(errno) to the end of the failure string to aid
// in interacting with C APIs.
// Error and ErrnoError are used to construct a Result<T> that has failed. The Error class takes
// an ostream as an input and are implicitly cast to a Result<T> containing that failure.
// ErrnoError() is a helper function to create an Error class that appends ": " + strerror(errno)
// to the end of the failure string to aid in interacting with C APIs. Alternatively, an errno
// value can be directly specified via the Error() constructor.
//
// ResultError can be used in the ostream when using Error to construct a Result<T>. In this case,
// the string that the ResultError takes is passed through the stream normally, but the errno is
// passed to the Result<T>. This can be used to pass errno from a failing C function up multiple
// callers.
//
// ResultError can also directly construct a Result<T>. This is particularly useful if you have a
// function that return Result<T> but you have a Result<U> and want to return its error. In this
// case, you can return the .error() from the Result<U> to construct the Result<T>.
// An example of how to use these is below:
// Result<U> CalculateResult(const T& input) {
@ -66,9 +80,29 @@
namespace android {
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) {}
std::string error_string;
int error_errno;
};
inline std::ostream& operator<<(std::ostream& os, const ResultError& t) {
os << t.error_string;
return os;
}
inline std::ostream& operator<<(std::ostream& os, ResultError&& t) {
os << std::move(t.error_string);
return os;
}
class Error {
public:
Error() : append_errno_(0) {}
Error() : errno_(0), append_errno_(false) {}
Error(int errno_to_append) : errno_(errno_to_append), append_errno_(true) {}
template <typename T>
Error&& operator<<(T&& t) {
@ -76,30 +110,45 @@ class Error {
return std::move(*this);
}
const std::string str() const {
if (append_errno_) {
return ss_.str() + ": " + strerror(append_errno_);
}
return ss_.str();
Error&& operator<<(const ResultError& result_error) {
ss_ << result_error.error_string;
errno_ = result_error.error_errno;
return std::move(*this);
}
Error&& operator<<(ResultError&& result_error) {
ss_ << std::move(result_error.error_string);
errno_ = result_error.error_errno;
return std::move(*this);
}
const std::string str() const {
std::string str = ss_.str();
if (append_errno_) {
if (str.empty()) {
return strerror(errno_);
}
return str + ": " + strerror(errno_);
}
return str;
}
int get_errno() const { return errno_; }
Error(const Error&) = delete;
Error(Error&&) = delete;
Error& operator=(const Error&) = delete;
Error& operator=(Error&&) = delete;
protected:
Error(int append_errno) : append_errno_(append_errno) {}
private:
std::stringstream ss_;
int append_errno_;
int errno_;
bool append_errno_;
};
class ErrnoError : public Error {
public:
ErrnoError() : Error(errno) {}
};
inline Error ErrnoError() {
return Error(errno);
}
template <typename T>
class Result {
@ -107,7 +156,13 @@ class Result {
template <typename... U>
Result(U&&... result) : contents_(std::in_place_index_t<0>(), std::forward<U>(result)...) {}
Result(Error&& fb) : contents_(std::in_place_index_t<1>(), fb.str()) {}
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) {}
bool has_value() const { return contents_.index() == 0; }
@ -116,9 +171,17 @@ class Result {
T&& value() && { return std::get<0>(std::move(contents_)); }
const T&& value() const && { return std::get<0>(std::move(contents_)); }
const std::string& error() const & { return std::get<1>(contents_); }
std::string&& error() && { return std::get<1>(std::move(contents_)); }
const std::string&& error() const && { return std::get<1>(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(); }
@ -131,7 +194,7 @@ class Result {
const T* operator->() const { return &value(); }
private:
std::variant<T, std::string> contents_;
std::variant<T, ResultError> contents_;
};
using Success = std::monostate;

78
init/result_test.cpp
View File

@ -74,7 +74,8 @@ TEST(result, result_error) {
ASSERT_FALSE(result);
ASSERT_FALSE(result.has_value());
EXPECT_EQ("failure1", result.error());
EXPECT_EQ(0, result.error_errno());
EXPECT_EQ("failure1", result.error_string());
}
TEST(result, result_error_empty) {
@ -82,7 +83,8 @@ TEST(result, result_error_empty) {
ASSERT_FALSE(result);
ASSERT_FALSE(result.has_value());
EXPECT_EQ("", result.error());
EXPECT_EQ(0, result.error_errno());
EXPECT_EQ("", result.error_string());
}
TEST(result, result_error_rvalue) {
@ -96,7 +98,8 @@ TEST(result, result_error_rvalue) {
ASSERT_FALSE(MakeRvalueErrorResult());
ASSERT_FALSE(MakeRvalueErrorResult().has_value());
EXPECT_EQ("failure1", MakeRvalueErrorResult().error());
EXPECT_EQ(0, MakeRvalueErrorResult().error_errno());
EXPECT_EQ("failure1", MakeRvalueErrorResult().error_string());
}
TEST(result, result_errno_error) {
@ -107,7 +110,72 @@ TEST(result, result_errno_error) {
ASSERT_FALSE(result);
ASSERT_FALSE(result.has_value());
EXPECT_EQ("failure1: "s + strerror(test_errno), result.error());
EXPECT_EQ(test_errno, result.error_errno());
EXPECT_EQ("failure1: "s + strerror(test_errno), result.error_string());
}
TEST(result, result_errno_error_no_text) {
constexpr int test_errno = 6;
errno = test_errno;
Result<Success> result = ErrnoError();
ASSERT_FALSE(result);
ASSERT_FALSE(result.has_value());
EXPECT_EQ(test_errno, result.error_errno());
EXPECT_EQ(strerror(test_errno), result.error_string());
}
TEST(result, result_error_from_other_result) {
auto error_text = "test error"s;
Result<Success> result = Error() << error_text;
ASSERT_FALSE(result);
ASSERT_FALSE(result.has_value());
Result<std::string> result2 = result.error();
ASSERT_FALSE(result2);
ASSERT_FALSE(result2.has_value());
EXPECT_EQ(0, result.error_errno());
EXPECT_EQ(error_text, result.error_string());
}
TEST(result, result_error_through_ostream) {
auto error_text = "test error"s;
Result<Success> result = Error() << error_text;
ASSERT_FALSE(result);
ASSERT_FALSE(result.has_value());
Result<std::string> result2 = Error() << result.error();
ASSERT_FALSE(result2);
ASSERT_FALSE(result2.has_value());
EXPECT_EQ(0, result.error_errno());
EXPECT_EQ(error_text, result.error_string());
}
TEST(result, result_errno_error_through_ostream) {
auto error_text = "test error"s;
constexpr int test_errno = 6;
errno = 6;
Result<Success> result = ErrnoError() << error_text;
errno = 0;
ASSERT_FALSE(result);
ASSERT_FALSE(result.has_value());
Result<std::string> result2 = Error() << result.error();
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());
}
TEST(result, constructor_forwarding) {
@ -215,7 +283,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(), "");
ASSERT_DEATH(result.error_string(), "");
}
} // namespace init

10
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());
EXPECT_EQ("open() failed: No such file or directory", file_contents.error_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());
EXPECT_EQ("Skipping insecure file", file_contents.error_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());
EXPECT_EQ("Skipping insecure file", file_contents.error_string());
}
TEST(util, ReadFileSymbolicLink) {
@ -65,7 +65,7 @@ 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());
EXPECT_EQ("open() failed: Too many symbolic links encountered", file_contents.error_string());
}
TEST(util, ReadFileSuccess) {
@ -130,7 +130,7 @@ TEST(util, DecodeUid) {
decoded_uid = DecodeUid("toot");
EXPECT_FALSE(decoded_uid);
EXPECT_EQ("getpwnam failed: No such file or directory", decoded_uid.error());
EXPECT_EQ("getpwnam failed: No such file or directory", decoded_uid.error_string());
decoded_uid = DecodeUid("123");
EXPECT_TRUE(decoded_uid);