From 8fd64c8af11e29a270bff6688f52589afb2b0466 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Fri, 31 May 2019 03:43:34 +0900 Subject: [PATCH] Move result.h from init to libbase The Result, Error, ErrnoError are quite generic. Moving them from init to libbase so that they can be used from other places. Bug: 132145659 Test: libbase_test Change-Id: Id774a587f74380fadd7a0fc88c0aa892c3d9a489 --- base/Android.bp | 1 + base/include/android-base/result.h | 164 +++++++++++++ base/result_test.cpp | 357 +++++++++++++++++++++++++++++ init/Android.bp | 1 - init/action.cpp | 4 +- init/result.h | 107 +-------- init/result_test.cpp | 335 --------------------------- init/rlimit_parser_test.cpp | 4 +- init/service.cpp | 2 +- init/subcontext.cpp | 6 +- init/subcontext_test.cpp | 10 +- init/util_test.cpp | 10 +- 12 files changed, 548 insertions(+), 453 deletions(-) create mode 100644 base/include/android-base/result.h create mode 100644 base/result_test.cpp delete mode 100644 init/result_test.cpp diff --git a/base/Android.bp b/base/Android.bp index 58d3a5043..25a9f6884 100644 --- a/base/Android.bp +++ b/base/Android.bp @@ -141,6 +141,7 @@ cc_test { "parsenetaddress_test.cpp", "properties_test.cpp", "quick_exit_test.cpp", + "result_test.cpp", "scopeguard_test.cpp", "stringprintf_test.cpp", "strings_test.cpp", diff --git a/base/include/android-base/result.h b/base/include/android-base/result.h new file mode 100644 index 000000000..3cb39e6e2 --- /dev/null +++ b/base/include/android-base/result.h @@ -0,0 +1,164 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// This file contains classes for returning a successful result along with an optional +// arbitrarily typed return value or for returning a failure result along with an optional string +// indicating why the function failed. + +// There are 3 classes that implement this functionality and one additional helper type. +// +// Result either contains a member of type T that can be accessed using similar semantics as +// std::optional or it contains a ResultError describing an error, which can be accessed via +// Result::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 that do not contain a return value. +// Result 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 is the +// correct way to indicate that a function without a return type has completed successfully. +// +// A successful Result is constructed implicitly from any type that can be implicitly converted +// to T or from the constructor arguments for T. This allows you to return a type T directly from +// a function that returns Result. +// +// Error and ErrnoError are used to construct a Result that has failed. The Error class takes +// an ostream as an input and are implicitly cast to a Result 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. In this case, +// the string that the ResultError takes is passed through the stream normally, but the errno is +// passed to the Result. This can be used to pass errno from a failing C function up multiple +// callers. +// +// ResultError can also directly construct a Result. This is particularly useful if you have a +// function that return Result but you have a Result and want to return its error. In this +// case, you can return the .error() from the Result to construct the Result. + +// An example of how to use these is below: +// Result CalculateResult(const T& input) { +// U output; +// if (!SomeOtherCppFunction(input, &output)) { +// return Error() << "SomeOtherCppFunction(" << input << ") failed"; +// } +// if (!c_api_function(output)) { +// return ErrnoError() << "c_api_function(" << output << ") failed"; +// } +// return output; +// } +// +// auto output = CalculateResult(input); +// if (!output) return Error() << "CalculateResult failed: " << output.error(); +// UseOutput(*output); + +#pragma once + +#include + +#include +#include + +#include "android-base/expected.h" + +namespace android { +namespace base { + +struct ResultError { + template + ResultError(T&& message, int code) + : message_(std::forward(message)), code_(code) {} + + template + operator android::base::expected() { + return android::base::unexpected(ResultError(message_, code_)); + } + + std::string message() const { return message_; } + int code() const { return code_; } + + private: + std::string message_; + int code_; +}; + +inline std::ostream& operator<<(std::ostream& os, const ResultError& t) { + os << t.message(); + return os; +} + +class Error { + public: + 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) { + int saved = errno; + ss_ << t; + errno = saved; + return *this; + } + + Error& operator<<(const ResultError& result_error) { + (*this) << result_error.message(); + errno_ = result_error.code(); + return *this; + } + + const std::string str() const { + std::string str = ss_.str(); + if (append_errno_) { + if (str.empty()) { + return strerror(errno_); + } + return std::move(str) + ": " + strerror(errno_); + } + return str; + } + + Error(const Error&) = delete; + Error(Error&&) = delete; + Error& operator=(const Error&) = delete; + Error& operator=(Error&&) = delete; + + private: + std::stringstream ss_; + int errno_; + bool append_errno_; +}; + +inline Error ErrnoError() { + return Error(errno); +} + +template +using Result = android::base::expected; + +// Usage: `Result` as a result type that doesn't contain a value. +// Use `return {}` or `return Success()` to return with success. +using Success = std::monostate; + +} // namespace base +} // namespace android diff --git a/base/result_test.cpp b/base/result_test.cpp new file mode 100644 index 000000000..687488991 --- /dev/null +++ b/base/result_test.cpp @@ -0,0 +1,357 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "android-base/result.h" + +#include "errno.h" + +#include +#include + +#include + +using namespace std::string_literals; + +namespace android { +namespace base { + +TEST(result, result_accessors) { + Result result = "success"; + ASSERT_TRUE(result); + ASSERT_TRUE(result.has_value()); + + EXPECT_EQ("success", *result); + EXPECT_EQ("success", result.value()); + + EXPECT_EQ('s', result->data()[0]); +} + +TEST(result, result_accessors_rvalue) { + ASSERT_TRUE(Result("success")); + ASSERT_TRUE(Result("success").has_value()); + + EXPECT_EQ("success", *Result("success")); + EXPECT_EQ("success", Result("success").value()); + + EXPECT_EQ('s', Result("success")->data()[0]); +} + +TEST(result, result_success) { + Result result = Success(); + ASSERT_TRUE(result); + ASSERT_TRUE(result.has_value()); + + EXPECT_EQ(Success(), *result); + EXPECT_EQ(Success(), result.value()); +} + +TEST(result, result_success_rvalue) { + // Success() doesn't actually create a Result object, but rather an object that can be + // implicitly constructed into a Result object. + + auto MakeRvalueSuccessResult = []() -> Result { return Success(); }; + ASSERT_TRUE(MakeRvalueSuccessResult()); + ASSERT_TRUE(MakeRvalueSuccessResult().has_value()); + + EXPECT_EQ(Success(), *MakeRvalueSuccessResult()); + EXPECT_EQ(Success(), MakeRvalueSuccessResult().value()); +} + +TEST(result, result_error) { + Result result = Error() << "failure" << 1; + ASSERT_FALSE(result); + ASSERT_FALSE(result.has_value()); + + EXPECT_EQ(0, result.error().code()); + EXPECT_EQ("failure1", result.error().message()); +} + +TEST(result, result_error_empty) { + Result result = Error(); + ASSERT_FALSE(result); + ASSERT_FALSE(result.has_value()); + + EXPECT_EQ(0, result.error().code()); + EXPECT_EQ("", result.error().message()); +} + +TEST(result, result_error_rvalue) { + // Error() and ErrnoError() aren't actually used to create a Result object. + // Under the hood, they are an intermediate class that can be implicitly constructed into a + // Result. This is needed both to create the ostream and because Error() itself, by + // definition will not know what the type, T, of the underlying Result object that it would + // create is. + + auto MakeRvalueErrorResult = []() -> Result { return Error() << "failure" << 1; }; + ASSERT_FALSE(MakeRvalueErrorResult()); + ASSERT_FALSE(MakeRvalueErrorResult().has_value()); + + EXPECT_EQ(0, MakeRvalueErrorResult().error().code()); + EXPECT_EQ("failure1", MakeRvalueErrorResult().error().message()); +} + +TEST(result, result_errno_error) { + constexpr int test_errno = 6; + errno = test_errno; + Result result = ErrnoError() << "failure" << 1; + + ASSERT_FALSE(result); + ASSERT_FALSE(result.has_value()); + + EXPECT_EQ(test_errno, result.error().code()); + EXPECT_EQ("failure1: "s + strerror(test_errno), result.error().message()); +} + +TEST(result, result_errno_error_no_text) { + constexpr int test_errno = 6; + errno = test_errno; + Result result = ErrnoError(); + + ASSERT_FALSE(result); + ASSERT_FALSE(result.has_value()); + + EXPECT_EQ(test_errno, result.error().code()); + EXPECT_EQ(strerror(test_errno), result.error().message()); +} + +TEST(result, result_error_from_other_result) { + auto error_text = "test error"s; + Result result = Error() << error_text; + + ASSERT_FALSE(result); + ASSERT_FALSE(result.has_value()); + + Result result2 = result.error(); + + ASSERT_FALSE(result2); + ASSERT_FALSE(result2.has_value()); + + EXPECT_EQ(0, result.error().code()); + EXPECT_EQ(error_text, result.error().message()); +} + +TEST(result, result_error_through_ostream) { + auto error_text = "test error"s; + Result result = Error() << error_text; + + ASSERT_FALSE(result); + ASSERT_FALSE(result.has_value()); + + Result result2 = Error() << result.error(); + + ASSERT_FALSE(result2); + ASSERT_FALSE(result2.has_value()); + + EXPECT_EQ(0, result.error().code()); + EXPECT_EQ(error_text, result.error().message()); +} + +TEST(result, result_errno_error_through_ostream) { + auto error_text = "test error"s; + constexpr int test_errno = 6; + errno = 6; + Result result = ErrnoError() << error_text; + + errno = 0; + + ASSERT_FALSE(result); + ASSERT_FALSE(result.has_value()); + + Result result2 = Error() << result.error(); + + ASSERT_FALSE(result2); + ASSERT_FALSE(result2.has_value()); + + EXPECT_EQ(test_errno, result.error().code()); + EXPECT_EQ(error_text + ": " + strerror(test_errno), result.error().message()); +} + +TEST(result, constructor_forwarding) { + auto result = Result(std::in_place, 5, 'a'); + + ASSERT_TRUE(result); + ASSERT_TRUE(result.has_value()); + + EXPECT_EQ("aaaaa", *result); +} + +struct ConstructorTracker { + static size_t constructor_called; + static size_t copy_constructor_called; + static size_t move_constructor_called; + static size_t copy_assignment_called; + static size_t move_assignment_called; + + template + ConstructorTracker(T&& string) : string(string) { + ++constructor_called; + } + + ConstructorTracker(const ConstructorTracker& ct) { + ++copy_constructor_called; + string = ct.string; + } + ConstructorTracker(ConstructorTracker&& ct) noexcept { + ++move_constructor_called; + string = std::move(ct.string); + } + ConstructorTracker& operator=(const ConstructorTracker& ct) { + ++copy_assignment_called; + string = ct.string; + return *this; + } + ConstructorTracker& operator=(ConstructorTracker&& ct) noexcept { + ++move_assignment_called; + string = std::move(ct.string); + return *this; + } + + std::string string; +}; + +size_t ConstructorTracker::constructor_called = 0; +size_t ConstructorTracker::copy_constructor_called = 0; +size_t ConstructorTracker::move_constructor_called = 0; +size_t ConstructorTracker::copy_assignment_called = 0; +size_t ConstructorTracker::move_assignment_called = 0; + +Result ReturnConstructorTracker(const std::string& in) { + if (in.empty()) { + return "literal string"; + } + if (in == "test2") { + return ConstructorTracker(in + in + "2"); + } + ConstructorTracker result(in + " " + in); + return result; +}; + +TEST(result, no_copy_on_return) { + // If returning parameters that may be used to implicitly construct the type T of Result, + // then those parameters are forwarded to the construction of Result. + + // If returning an prvalue or xvalue, it will be move constructed during the construction of + // Result. + + // This check ensures that that is the case, and particularly that no copy constructors + // are called. + + auto result1 = ReturnConstructorTracker(""); + ASSERT_TRUE(result1); + EXPECT_EQ("literal string", result1->string); + EXPECT_EQ(1U, ConstructorTracker::constructor_called); + EXPECT_EQ(0U, ConstructorTracker::copy_constructor_called); + EXPECT_EQ(0U, ConstructorTracker::move_constructor_called); + EXPECT_EQ(0U, ConstructorTracker::copy_assignment_called); + EXPECT_EQ(0U, ConstructorTracker::move_assignment_called); + + auto result2 = ReturnConstructorTracker("test2"); + ASSERT_TRUE(result2); + EXPECT_EQ("test2test22", result2->string); + EXPECT_EQ(2U, ConstructorTracker::constructor_called); + EXPECT_EQ(0U, ConstructorTracker::copy_constructor_called); + EXPECT_EQ(1U, ConstructorTracker::move_constructor_called); + EXPECT_EQ(0U, ConstructorTracker::copy_assignment_called); + EXPECT_EQ(0U, ConstructorTracker::move_assignment_called); + + auto result3 = ReturnConstructorTracker("test3"); + ASSERT_TRUE(result3); + EXPECT_EQ("test3 test3", result3->string); + EXPECT_EQ(3U, ConstructorTracker::constructor_called); + EXPECT_EQ(0U, ConstructorTracker::copy_constructor_called); + EXPECT_EQ(2U, ConstructorTracker::move_constructor_called); + EXPECT_EQ(0U, ConstructorTracker::copy_assignment_called); + EXPECT_EQ(0U, ConstructorTracker::move_assignment_called); +} + +// Below two tests require that we do not hide the move constructor with our forwarding reference +// constructor. This is done with by disabling the forwarding reference constructor if its first +// and only type is Result. +TEST(result, result_result_with_success) { + auto return_result_result_with_success = []() -> Result> { + return Result(); + }; + auto result = return_result_result_with_success(); + ASSERT_TRUE(result); + ASSERT_TRUE(*result); + + auto inner_result = result.value(); + ASSERT_TRUE(inner_result); +} + +TEST(result, result_result_with_failure) { + auto return_result_result_with_error = []() -> Result> { + return Result(ResultError("failure string", 6)); + }; + auto result = return_result_result_with_error(); + ASSERT_TRUE(result); + ASSERT_FALSE(*result); + EXPECT_EQ("failure string", (*result).error().message()); + EXPECT_EQ(6, (*result).error().code()); +} + +// This test requires that we disable the forwarding reference constructor if Result is the +// *only* type that we are forwarding. In otherwords, if we are forwarding Result, int to +// construct a Result, then we still need the constructor. +TEST(result, result_two_parameter_constructor_same_type) { + struct TestStruct { + TestStruct(int value) : value_(value) {} + TestStruct(Result result, int value) : value_(result->value_ * value) {} + int value_; + }; + + auto return_test_struct = []() -> Result { + return Result(std::in_place, Result(std::in_place, 6), 6); + }; + + auto result = return_test_struct(); + ASSERT_TRUE(result); + EXPECT_EQ(36, result->value_); +} + +TEST(result, die_on_access_failed_result) { + Result result = Error(); + ASSERT_DEATH(*result, ""); +} + +TEST(result, die_on_get_error_succesful_result) { + Result result = "success"; + ASSERT_DEATH(result.error(), ""); +} + +template +std::basic_ostream& SetErrnoToTwo(std::basic_ostream& ss) { + errno = 2; + return ss; +} + +TEST(result, preserve_errno) { + errno = 1; + int old_errno = errno; + Result result = Error() << "Failed" << SetErrnoToTwo; + ASSERT_FALSE(result); + EXPECT_EQ(old_errno, errno); + + errno = 1; + old_errno = errno; + Result result2 = ErrnoError() << "Failed" << SetErrnoToTwo; + ASSERT_FALSE(result2); + EXPECT_EQ(old_errno, errno); + EXPECT_EQ(old_errno, result2.error().code()); +} + +} // namespace base +} // namespace android diff --git a/init/Android.bp b/init/Android.bp index eaa7fd73c..fa0a35c48 100644 --- a/init/Android.bp +++ b/init/Android.bp @@ -190,7 +190,6 @@ cc_test { "persistent_properties_test.cpp", "property_service_test.cpp", "property_type_test.cpp", - "result_test.cpp", "rlimit_parser_test.cpp", "service_test.cpp", "subcontext_test.cpp", diff --git a/init/action.cpp b/init/action.cpp index a4f69367c..a40172e68 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().as_errno == ENOENT) { + result.error().code() == 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().as_string); + << (result ? "succeeded" : "failed: " + result.error().message()); } } diff --git a/init/result.h b/init/result.h index baa680dfe..984b25792 100644 --- a/init/result.h +++ b/init/result.h @@ -29,8 +29,8 @@ // string value. // // Success is a typedef that aids in creating Result that do not contain a return value. -// Result 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 is the +// Result is the correct return type for a function that either returns successfully or +// returns an error value. Returning Nothing() from a function that returns Result is the // correct way to indicate that a function without a return type has completed successfully. // // A successful Result is constructed implicitly from any type that can be implicitly converted @@ -70,101 +70,10 @@ #pragma once -#include - -#include -#include - -#include - -namespace android { -namespace init { - -struct ResultError { - template - ResultError(T&& error_string, int error_errno) - : as_string(std::forward(error_string)), as_errno(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.as_string; - return os; -} - -inline std::ostream& operator<<(std::ostream& os, ResultError&& t) { - os << std::move(t.as_string); - return os; -} - -class Error { - public: - 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); - return std::move(*this); - } - - Error&& operator<<(const ResultError& result_error) { - ss_ << result_error.as_string; - errno_ = result_error.as_errno; - return std::move(*this); - } - - Error&& operator<<(ResultError&& result_error) { - ss_ << std::move(result_error.as_string); - errno_ = result_error.as_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; - - private: - std::stringstream ss_; - int errno_; - bool append_errno_; -}; - -inline Error ErrnoError() { - return Error(errno); -} - -template -using Result = android::base::expected; - -using Success = std::monostate; - -} // namespace init -} // namespace android +#include +using android::base::ErrnoError; +using android::base::Error; +using android::base::Result; +using android::base::ResultError; +using android::base::Success; diff --git a/init/result_test.cpp b/init/result_test.cpp deleted file mode 100644 index d3d04a015..000000000 --- a/init/result_test.cpp +++ /dev/null @@ -1,335 +0,0 @@ -/* - * Copyright (C) 2017 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include "result.h" - -#include "errno.h" - -#include - -#include - -using namespace std::string_literals; - -namespace android { -namespace init { - -TEST(result, result_accessors) { - Result result = "success"; - ASSERT_TRUE(result); - ASSERT_TRUE(result.has_value()); - - EXPECT_EQ("success", *result); - EXPECT_EQ("success", result.value()); - - EXPECT_EQ('s', result->data()[0]); -} - -TEST(result, result_accessors_rvalue) { - ASSERT_TRUE(Result("success")); - ASSERT_TRUE(Result("success").has_value()); - - EXPECT_EQ("success", *Result("success")); - EXPECT_EQ("success", Result("success").value()); - - EXPECT_EQ('s', Result("success")->data()[0]); -} - -TEST(result, result_success) { - Result result = Success(); - ASSERT_TRUE(result); - ASSERT_TRUE(result.has_value()); - - EXPECT_EQ(Success(), *result); - EXPECT_EQ(Success(), result.value()); -} - -TEST(result, result_success_rvalue) { - // Success() doesn't actually create a Result object, but rather an object that can be - // implicitly constructed into a Result object. - - auto MakeRvalueSuccessResult = []() -> Result { return Success(); }; - ASSERT_TRUE(MakeRvalueSuccessResult()); - ASSERT_TRUE(MakeRvalueSuccessResult().has_value()); - - EXPECT_EQ(Success(), *MakeRvalueSuccessResult()); - EXPECT_EQ(Success(), MakeRvalueSuccessResult().value()); -} - -TEST(result, result_error) { - Result result = Error() << "failure" << 1; - ASSERT_FALSE(result); - ASSERT_FALSE(result.has_value()); - - EXPECT_EQ(0, result.error().as_errno); - EXPECT_EQ("failure1", result.error().as_string); -} - -TEST(result, result_error_empty) { - Result result = Error(); - ASSERT_FALSE(result); - ASSERT_FALSE(result.has_value()); - - EXPECT_EQ(0, result.error().as_errno); - EXPECT_EQ("", result.error().as_string); -} - -TEST(result, result_error_rvalue) { - // Error() and ErrnoError() aren't actually used to create a Result object. - // Under the hood, they are an intermediate class that can be implicitly constructed into a - // Result. This is needed both to create the ostream and because Error() itself, by - // definition will not know what the type, T, of the underlying Result object that it would - // create is. - - auto MakeRvalueErrorResult = []() -> Result { return Error() << "failure" << 1; }; - ASSERT_FALSE(MakeRvalueErrorResult()); - ASSERT_FALSE(MakeRvalueErrorResult().has_value()); - - EXPECT_EQ(0, MakeRvalueErrorResult().error().as_errno); - EXPECT_EQ("failure1", MakeRvalueErrorResult().error().as_string); -} - -TEST(result, result_errno_error) { - constexpr int test_errno = 6; - errno = test_errno; - Result result = ErrnoError() << "failure" << 1; - - ASSERT_FALSE(result); - ASSERT_FALSE(result.has_value()); - - 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) { - constexpr int test_errno = 6; - errno = test_errno; - Result result = ErrnoError(); - - ASSERT_FALSE(result); - ASSERT_FALSE(result.has_value()); - - EXPECT_EQ(test_errno, result.error().as_errno); - EXPECT_EQ(strerror(test_errno), result.error().as_string); -} - -TEST(result, result_error_from_other_result) { - auto error_text = "test error"s; - Result result = Error() << error_text; - - ASSERT_FALSE(result); - ASSERT_FALSE(result.has_value()); - - Result result2 = result.error(); - - ASSERT_FALSE(result2); - ASSERT_FALSE(result2.has_value()); - - EXPECT_EQ(0, result.error().as_errno); - EXPECT_EQ(error_text, result.error().as_string); -} - -TEST(result, result_error_through_ostream) { - auto error_text = "test error"s; - Result result = Error() << error_text; - - ASSERT_FALSE(result); - ASSERT_FALSE(result.has_value()); - - Result result2 = Error() << result.error(); - - ASSERT_FALSE(result2); - ASSERT_FALSE(result2.has_value()); - - EXPECT_EQ(0, result.error().as_errno); - EXPECT_EQ(error_text, result.error().as_string); -} - -TEST(result, result_errno_error_through_ostream) { - auto error_text = "test error"s; - constexpr int test_errno = 6; - errno = 6; - Result result = ErrnoError() << error_text; - - errno = 0; - - ASSERT_FALSE(result); - ASSERT_FALSE(result.has_value()); - - Result result2 = Error() << result.error(); - - ASSERT_FALSE(result2); - ASSERT_FALSE(result2.has_value()); - - 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::in_place, 5, 'a'); - - ASSERT_TRUE(result); - ASSERT_TRUE(result.has_value()); - - EXPECT_EQ("aaaaa", *result); -} - -struct ConstructorTracker { - static size_t constructor_called; - static size_t copy_constructor_called; - static size_t move_constructor_called; - static size_t copy_assignment_called; - static size_t move_assignment_called; - - template - ConstructorTracker(T&& string) : string(string) { - ++constructor_called; - } - - ConstructorTracker(const ConstructorTracker& ct) { - ++copy_constructor_called; - string = ct.string; - } - ConstructorTracker(ConstructorTracker&& ct) noexcept { - ++move_constructor_called; - string = std::move(ct.string); - } - ConstructorTracker& operator=(const ConstructorTracker& ct) { - ++copy_assignment_called; - string = ct.string; - return *this; - } - ConstructorTracker& operator=(ConstructorTracker&& ct) noexcept { - ++move_assignment_called; - string = std::move(ct.string); - return *this; - } - - std::string string; -}; - -size_t ConstructorTracker::constructor_called = 0; -size_t ConstructorTracker::copy_constructor_called = 0; -size_t ConstructorTracker::move_constructor_called = 0; -size_t ConstructorTracker::copy_assignment_called = 0; -size_t ConstructorTracker::move_assignment_called = 0; - -Result ReturnConstructorTracker(const std::string& in) { - if (in.empty()) { - return "literal string"; - } - if (in == "test2") { - return ConstructorTracker(in + in + "2"); - } - ConstructorTracker result(in + " " + in); - return result; -}; - -TEST(result, no_copy_on_return) { - // If returning parameters that may be used to implicitly construct the type T of Result, - // then those parameters are forwarded to the construction of Result. - - // If returning an prvalue or xvalue, it will be move constructed during the construction of - // Result. - - // This check ensures that that is the case, and particularly that no copy constructors - // are called. - - auto result1 = ReturnConstructorTracker(""); - ASSERT_TRUE(result1); - EXPECT_EQ("literal string", result1->string); - EXPECT_EQ(1U, ConstructorTracker::constructor_called); - EXPECT_EQ(0U, ConstructorTracker::copy_constructor_called); - EXPECT_EQ(0U, ConstructorTracker::move_constructor_called); - EXPECT_EQ(0U, ConstructorTracker::copy_assignment_called); - EXPECT_EQ(0U, ConstructorTracker::move_assignment_called); - - auto result2 = ReturnConstructorTracker("test2"); - ASSERT_TRUE(result2); - EXPECT_EQ("test2test22", result2->string); - EXPECT_EQ(2U, ConstructorTracker::constructor_called); - EXPECT_EQ(0U, ConstructorTracker::copy_constructor_called); - EXPECT_EQ(1U, ConstructorTracker::move_constructor_called); - EXPECT_EQ(0U, ConstructorTracker::copy_assignment_called); - EXPECT_EQ(0U, ConstructorTracker::move_assignment_called); - - auto result3 = ReturnConstructorTracker("test3"); - ASSERT_TRUE(result3); - EXPECT_EQ("test3 test3", result3->string); - EXPECT_EQ(3U, ConstructorTracker::constructor_called); - EXPECT_EQ(0U, ConstructorTracker::copy_constructor_called); - EXPECT_EQ(2U, ConstructorTracker::move_constructor_called); - EXPECT_EQ(0U, ConstructorTracker::copy_assignment_called); - EXPECT_EQ(0U, ConstructorTracker::move_assignment_called); -} - -// Below two tests require that we do not hide the move constructor with our forwarding reference -// constructor. This is done with by disabling the forwarding reference constructor if its first -// and only type is Result. -TEST(result, result_result_with_success) { - auto return_result_result_with_success = []() -> Result> { - return Result(); - }; - auto result = return_result_result_with_success(); - ASSERT_TRUE(result); - ASSERT_TRUE(*result); - - auto inner_result = result.value(); - ASSERT_TRUE(inner_result); -} - -TEST(result, result_result_with_failure) { - auto return_result_result_with_error = []() -> Result> { - return Result(ResultError("failure string", 6)); - }; - auto result = return_result_result_with_error(); - ASSERT_TRUE(result); - ASSERT_FALSE(*result); - 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 -// *only* type that we are forwarding. In otherwords, if we are forwarding Result, int to -// construct a Result, then we still need the constructor. -TEST(result, result_two_parameter_constructor_same_type) { - struct TestStruct { - TestStruct(int value) : value_(value) {} - TestStruct(Result result, int value) : value_(result->value_ * value) {} - int value_; - }; - - auto return_test_struct = []() -> Result { - return Result(std::in_place, Result(std::in_place, 6), 6); - }; - - auto result = return_test_struct(); - ASSERT_TRUE(result); - EXPECT_EQ(36, result->value_); -} - -TEST(result, die_on_access_failed_result) { - Result result = Error(); - ASSERT_DEATH(*result, ""); -} - -TEST(result, die_on_get_error_succesful_result) { - Result result = "success"; - ASSERT_DEATH(result.error(), ""); -} - -} // namespace init -} // namespace android diff --git a/init/rlimit_parser_test.cpp b/init/rlimit_parser_test.cpp index e690bf6d6..6a16d3b57 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().as_string); - EXPECT_EQ(0, result.error().as_errno); + EXPECT_EQ(expected_result, result.error().message()); + EXPECT_EQ(0, result.error().code()); } TEST(rlimit, RlimitSuccess) { diff --git a/init/service.cpp b/init/service.cpp index 8fe5b309a..a54cb6bdf 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -1196,7 +1196,7 @@ void ServiceList::MarkServicesUpdate() { continue; } if (auto result = service->Start(); !result) { - LOG(ERROR) << result.error().as_string; + LOG(ERROR) << result.error().message(); } } delayed_service_names_.clear(); diff --git a/init/subcontext.cpp b/init/subcontext.cpp index f9eb83def..0ff479a22 100644 --- a/init/subcontext.cpp +++ b/init/subcontext.cpp @@ -142,8 +142,8 @@ void SubcontextProcess::RunCommand(const SubcontextCommand::ExecuteCommand& exec reply->set_success(true); } else { auto* failure = reply->mutable_failure(); - failure->set_error_string(result.error().as_string); - failure->set_error_errno(result.error().as_errno); + failure->set_error_string(result.error().message()); + failure->set_error_errno(result.error().code()); } } @@ -178,7 +178,7 @@ void SubcontextProcess::MainLoop() { auto init_message = ReadMessage(init_fd_); if (!init_message) { - if (init_message.error().as_errno == 0) { + if (init_message.error().code() == 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_test.cpp b/init/subcontext_test.cpp index c0662a471..263568300 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().as_string, " "); + auto pids = Split(result.error().message(), " "); 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().as_string); + EXPECT_EQ(Join(expected_words, " "), result.error().message()); 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().as_string); + EXPECT_EQ("Sane error!", result2.error().message()); 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().as_string); + ASSERT_EQ(context_string, result.error().message()); }); } @@ -167,7 +167,7 @@ TEST(subcontext, ExpandArgsFailure) { }; auto result = subcontext.ExpandArgs(args); ASSERT_FALSE(result); - EXPECT_EQ("Failed to expand '" + args[1] + "'", result.error().as_string); + EXPECT_EQ("Failed to expand '" + args[1] + "'", result.error().message()); }); } diff --git a/init/util_test.cpp b/init/util_test.cpp index 8bf672c0e..8947256cf 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().as_string); + EXPECT_EQ("open() failed: No such file or directory", file_contents.error().message()); } 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().as_string); + EXPECT_EQ("Skipping insecure file", file_contents.error().message()); } 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().as_string); + EXPECT_EQ("Skipping insecure file", file_contents.error().message()); } TEST(util, ReadFileSymbolicLink) { @@ -66,7 +66,7 @@ TEST(util, ReadFileSymbolicLink) { EXPECT_EQ(ELOOP, errno); ASSERT_FALSE(file_contents); EXPECT_EQ("open() failed: Too many symbolic links encountered", - file_contents.error().as_string); + file_contents.error().message()); } TEST(util, ReadFileSuccess) { @@ -131,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().as_string); + EXPECT_EQ("getpwnam failed: No such file or directory", decoded_uid.error().message()); decoded_uid = DecodeUid("123"); EXPECT_TRUE(decoded_uid);