From cecebbbaccd36dc2e9f6e80c8e91e57d979963a5 Mon Sep 17 00:00:00 2001 From: Bernie Innocenti Date: Thu, 6 Feb 2020 03:49:33 +0900 Subject: [PATCH] Convert system/core to Result::ok() No functionality changes, this is a mechanical cleanup. Test: m Test: cd system/core && atest Change-Id: Ifdaa3ce1947ed578f656d5a446978726eb416c36 --- base/expected_test.cpp | 30 +++++++-------- base/result_test.cpp | 58 ++++++++++++++-------------- fs_mgr/libfs_avb/fs_avb.cpp | 2 +- init/action.cpp | 12 +++--- init/action_parser.cpp | 9 +++-- init/builtins.cpp | 60 ++++++++++++++--------------- init/check_builtins.cpp | 16 ++++---- init/firmware_handler.cpp | 2 +- init/host_init_verifier.cpp | 2 +- init/import_parser.cpp | 2 +- init/init.cpp | 24 ++++++------ init/init_test.cpp | 4 +- init/keychords.cpp | 4 +- init/keychords_test.cpp | 4 +- init/mount_handler.cpp | 2 +- init/mount_namespace.cpp | 8 ++-- init/parser.cpp | 12 +++--- init/persistent_properties.cpp | 14 +++---- init/persistent_properties_test.cpp | 11 ++++-- init/property_service.cpp | 24 ++++++------ init/reboot.cpp | 12 +++--- init/rlimit_parser_test.cpp | 4 +- init/selinux.cpp | 2 +- init/service.cpp | 24 ++++++------ init/service_list.cpp | 2 +- init/service_parser.cpp | 22 +++++------ init/service_test.cpp | 18 ++++----- init/service_utils.cpp | 8 ++-- init/subcontext.cpp | 18 ++++----- init/subcontext_test.cpp | 20 +++++----- init/ueventd_parser.cpp | 2 +- init/util.cpp | 4 +- init/util_test.cpp | 37 +++++++++--------- 33 files changed, 240 insertions(+), 233 deletions(-) diff --git a/base/expected_test.cpp b/base/expected_test.cpp index 6c3d42130..47e396a22 100644 --- a/base/expected_test.cpp +++ b/base/expected_test.cpp @@ -408,11 +408,11 @@ TEST(Expected, testDereference) { TEST(Expected, testTest) { exp_int e = 10; - EXPECT_TRUE(e); + EXPECT_TRUE(e.ok()); EXPECT_TRUE(e.has_value()); exp_int e2 = unexpected(10); - EXPECT_FALSE(e2); + EXPECT_FALSE(e2.ok()); EXPECT_FALSE(e2.has_value()); } @@ -571,11 +571,11 @@ TEST(Expected, testDivideExample) { } }; - EXPECT_FALSE(divide(10, 0)); + EXPECT_FALSE(divide(10, 0).ok()); EXPECT_EQ("divide by zero", divide(10, 0).error().message); EXPECT_EQ(-1, divide(10, 0).error().cause); - EXPECT_TRUE(divide(10, 3)); + EXPECT_TRUE(divide(10, 3).ok()); EXPECT_EQ(QR(3, 1), *divide(10, 3)); } @@ -589,7 +589,7 @@ TEST(Expected, testPair) { }; auto r = test(true); - EXPECT_TRUE(r); + EXPECT_TRUE(r.ok()); EXPECT_EQ("yes", r->first); } @@ -603,9 +603,9 @@ TEST(Expected, testVoid) { }; auto r = test(true); - EXPECT_TRUE(r); + EXPECT_TRUE(r.ok()); r = test(false); - EXPECT_FALSE(r); + EXPECT_FALSE(r.ok()); EXPECT_EQ(10, r.error()); } @@ -754,7 +754,7 @@ TEST(Expected, testNoCopyOnReturn) { ConstructorTracker::Reset(); auto result1 = test(""); - ASSERT_TRUE(result1); + ASSERT_TRUE(result1.ok()); EXPECT_EQ("literal string", result1->string); EXPECT_EQ(1U, ConstructorTracker::constructor_called); EXPECT_EQ(0U, ConstructorTracker::copy_constructor_called); @@ -764,7 +764,7 @@ TEST(Expected, testNoCopyOnReturn) { ConstructorTracker::Reset(); auto result2 = test("test2"); - ASSERT_TRUE(result2); + ASSERT_TRUE(result2.ok()); EXPECT_EQ("test2test22", result2->string); EXPECT_EQ(1U, ConstructorTracker::constructor_called); EXPECT_EQ(0U, ConstructorTracker::copy_constructor_called); @@ -774,7 +774,7 @@ TEST(Expected, testNoCopyOnReturn) { ConstructorTracker::Reset(); auto result3 = test("test3"); - ASSERT_TRUE(result3); + ASSERT_TRUE(result3.ok()); EXPECT_EQ("test3 test3", result3->string); EXPECT_EQ(1U, ConstructorTracker::constructor_called); EXPECT_EQ(0U, ConstructorTracker::copy_constructor_called); @@ -786,22 +786,22 @@ TEST(Expected, testNoCopyOnReturn) { TEST(Expected, testNested) { expected e = "hello"; + EXPECT_TRUE(e.ok()); EXPECT_TRUE(e.has_value()); EXPECT_TRUE(e.value().has_value()); - EXPECT_TRUE(e); - EXPECT_TRUE(*e); + EXPECT_TRUE(e->ok()); EXPECT_EQ("hello", e.value().value()); expected e2 = unexpected("world"); EXPECT_FALSE(e2.has_value()); - EXPECT_FALSE(e2); + EXPECT_FALSE(e2.ok()); EXPECT_EQ("world", e2.error()); expected e3 = exp_string(unexpected("world")); EXPECT_TRUE(e3.has_value()); EXPECT_FALSE(e3.value().has_value()); - EXPECT_TRUE(e3); - EXPECT_FALSE(*e3); + EXPECT_TRUE(e3.ok()); + EXPECT_FALSE(e3->ok()); EXPECT_EQ("world", e3.value().error()); } diff --git a/base/result_test.cpp b/base/result_test.cpp index 2ee4057eb..2908477cf 100644 --- a/base/result_test.cpp +++ b/base/result_test.cpp @@ -30,7 +30,7 @@ namespace base { TEST(result, result_accessors) { Result result = "success"; - ASSERT_TRUE(result); + ASSERT_RESULT_OK(result); ASSERT_TRUE(result.has_value()); EXPECT_EQ("success", *result); @@ -40,7 +40,7 @@ TEST(result, result_accessors) { } TEST(result, result_accessors_rvalue) { - ASSERT_TRUE(Result("success")); + ASSERT_TRUE(Result("success").ok()); ASSERT_TRUE(Result("success").has_value()); EXPECT_EQ("success", *Result("success")); @@ -51,12 +51,12 @@ TEST(result, result_accessors_rvalue) { TEST(result, result_void) { Result ok = {}; - EXPECT_TRUE(ok); + EXPECT_RESULT_OK(ok); ok.value(); // should not crash ASSERT_DEATH(ok.error(), ""); Result fail = Error() << "failure" << 1; - EXPECT_FALSE(fail); + EXPECT_FALSE(fail.ok()); EXPECT_EQ("failure1", fail.error().message()); EXPECT_EQ(0, fail.error().code()); EXPECT_TRUE(ok != fail); @@ -66,8 +66,8 @@ TEST(result, result_void) { if (ok) return {}; else return Error() << "failure" << 1; }; - EXPECT_TRUE(test(true)); - EXPECT_FALSE(test(false)); + EXPECT_TRUE(test(true).ok()); + EXPECT_FALSE(test(false).ok()); test(true).value(); // should not crash ASSERT_DEATH(test(true).error(), ""); ASSERT_DEATH(test(false).value(), ""); @@ -76,7 +76,7 @@ TEST(result, result_void) { TEST(result, result_error) { Result result = Error() << "failure" << 1; - ASSERT_FALSE(result); + ASSERT_FALSE(result.ok()); ASSERT_FALSE(result.has_value()); EXPECT_EQ(0, result.error().code()); @@ -85,7 +85,7 @@ TEST(result, result_error) { TEST(result, result_error_empty) { Result result = Error(); - ASSERT_FALSE(result); + ASSERT_FALSE(result.ok()); ASSERT_FALSE(result.has_value()); EXPECT_EQ(0, result.error().code()); @@ -100,7 +100,7 @@ TEST(result, result_error_rvalue) { // create is. auto MakeRvalueErrorResult = []() -> Result { return Error() << "failure" << 1; }; - ASSERT_FALSE(MakeRvalueErrorResult()); + ASSERT_FALSE(MakeRvalueErrorResult().ok()); ASSERT_FALSE(MakeRvalueErrorResult().has_value()); EXPECT_EQ(0, MakeRvalueErrorResult().error().code()); @@ -112,7 +112,7 @@ TEST(result, result_errno_error) { errno = test_errno; Result result = ErrnoError() << "failure" << 1; - ASSERT_FALSE(result); + ASSERT_FALSE(result.ok()); ASSERT_FALSE(result.has_value()); EXPECT_EQ(test_errno, result.error().code()); @@ -124,7 +124,7 @@ TEST(result, result_errno_error_no_text) { errno = test_errno; Result result = ErrnoError(); - ASSERT_FALSE(result); + ASSERT_FALSE(result.ok()); ASSERT_FALSE(result.has_value()); EXPECT_EQ(test_errno, result.error().code()); @@ -135,12 +135,12 @@ TEST(result, result_error_from_other_result) { auto error_text = "test error"s; Result result = Error() << error_text; - ASSERT_FALSE(result); + ASSERT_FALSE(result.ok()); ASSERT_FALSE(result.has_value()); Result result2 = result.error(); - ASSERT_FALSE(result2); + ASSERT_FALSE(result2.ok()); ASSERT_FALSE(result2.has_value()); EXPECT_EQ(0, result2.error().code()); @@ -151,12 +151,12 @@ TEST(result, result_error_through_ostream) { auto error_text = "test error"s; Result result = Error() << error_text; - ASSERT_FALSE(result); + ASSERT_FALSE(result.ok()); ASSERT_FALSE(result.has_value()); Result result2 = Error() << result.error(); - ASSERT_FALSE(result2); + ASSERT_FALSE(result2.ok()); ASSERT_FALSE(result2.has_value()); EXPECT_EQ(0, result2.error().code()); @@ -171,12 +171,12 @@ TEST(result, result_errno_error_through_ostream) { errno = 0; - ASSERT_FALSE(result); + ASSERT_FALSE(result.ok()); ASSERT_FALSE(result.has_value()); Result result2 = Error() << result.error(); - ASSERT_FALSE(result2); + ASSERT_FALSE(result2.ok()); ASSERT_FALSE(result2.has_value()); EXPECT_EQ(test_errno, result2.error().code()); @@ -186,7 +186,7 @@ TEST(result, result_errno_error_through_ostream) { TEST(result, constructor_forwarding) { auto result = Result(std::in_place, 5, 'a'); - ASSERT_TRUE(result); + ASSERT_RESULT_OK(result); ASSERT_TRUE(result.has_value()); EXPECT_EQ("aaaaa", *result); @@ -254,7 +254,7 @@ TEST(result, no_copy_on_return) { // are called. auto result1 = ReturnConstructorTracker(""); - ASSERT_TRUE(result1); + ASSERT_RESULT_OK(result1); EXPECT_EQ("literal string", result1->string); EXPECT_EQ(1U, ConstructorTracker::constructor_called); EXPECT_EQ(0U, ConstructorTracker::copy_constructor_called); @@ -263,7 +263,7 @@ TEST(result, no_copy_on_return) { EXPECT_EQ(0U, ConstructorTracker::move_assignment_called); auto result2 = ReturnConstructorTracker("test2"); - ASSERT_TRUE(result2); + ASSERT_RESULT_OK(result2); EXPECT_EQ("test2test22", result2->string); EXPECT_EQ(2U, ConstructorTracker::constructor_called); EXPECT_EQ(0U, ConstructorTracker::copy_constructor_called); @@ -272,7 +272,7 @@ TEST(result, no_copy_on_return) { EXPECT_EQ(0U, ConstructorTracker::move_assignment_called); auto result3 = ReturnConstructorTracker("test3"); - ASSERT_TRUE(result3); + ASSERT_RESULT_OK(result3); EXPECT_EQ("test3 test3", result3->string); EXPECT_EQ(3U, ConstructorTracker::constructor_called); EXPECT_EQ(0U, ConstructorTracker::copy_constructor_called); @@ -287,11 +287,11 @@ TEST(result, no_copy_on_return) { 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); + ASSERT_RESULT_OK(result); + ASSERT_RESULT_OK(*result); auto inner_result = result.value(); - ASSERT_TRUE(inner_result); + ASSERT_RESULT_OK(inner_result); } TEST(result, result_result_with_failure) { @@ -299,8 +299,8 @@ TEST(result, result_result_with_failure) { return Result(ResultError("failure string", 6)); }; auto result = return_result_result_with_error(); - ASSERT_TRUE(result); - ASSERT_FALSE(*result); + ASSERT_RESULT_OK(result); + ASSERT_FALSE(result->ok()); EXPECT_EQ("failure string", (*result).error().message()); EXPECT_EQ(6, (*result).error().code()); } @@ -320,7 +320,7 @@ TEST(result, result_two_parameter_constructor_same_type) { }; auto result = return_test_struct(); - ASSERT_TRUE(result); + ASSERT_RESULT_OK(result); EXPECT_EQ(36, result->value_); } @@ -344,13 +344,13 @@ TEST(result, preserve_errno) { errno = 1; int old_errno = errno; Result result = Error() << "Failed" << SetErrnoToTwo; - ASSERT_FALSE(result); + ASSERT_FALSE(result.ok()); EXPECT_EQ(old_errno, errno); errno = 1; old_errno = errno; Result result2 = ErrnoError() << "Failed" << SetErrnoToTwo; - ASSERT_FALSE(result2); + ASSERT_FALSE(result2.ok()); EXPECT_EQ(old_errno, errno); EXPECT_EQ(old_errno, result2.error().code()); } diff --git a/fs_mgr/libfs_avb/fs_avb.cpp b/fs_mgr/libfs_avb/fs_avb.cpp index 50de42c06..ed623bc2e 100644 --- a/fs_mgr/libfs_avb/fs_avb.cpp +++ b/fs_mgr/libfs_avb/fs_avb.cpp @@ -329,7 +329,7 @@ AvbUniquePtr AvbHandle::LoadAndVerifyVbmeta(const FstabEntry& fstab_entry, // or a string indicating multiple keys separated by ':'. std::vector allowed_avb_keys; auto list_avb_keys_in_dir = ListFiles(fstab_entry.avb_keys); - if (list_avb_keys_in_dir) { + if (list_avb_keys_in_dir.ok()) { std::sort(list_avb_keys_in_dir->begin(), list_avb_keys_in_dir->end()); allowed_avb_keys = *list_avb_keys_in_dir; } else { diff --git a/init/action.cpp b/init/action.cpp index f05fa7caa..1e998aeeb 100644 --- a/init/action.cpp +++ b/init/action.cpp @@ -36,7 +36,7 @@ Result RunBuiltinFunction(const BuiltinFunction& function, builtin_arguments.args[0] = args[0]; for (std::size_t i = 1; i < args.size(); ++i) { auto expanded_arg = ExpandProps(args[i]); - if (!expanded_arg) { + if (!expanded_arg.ok()) { return expanded_arg.error(); } builtin_arguments.args[i] = std::move(*expanded_arg); @@ -59,7 +59,7 @@ Result Command::InvokeFunc(Subcontext* subcontext) const { } auto expanded_args = subcontext->ExpandArgs(args_); - if (!expanded_args) { + if (!expanded_args.ok()) { return expanded_args.error(); } return RunBuiltinFunction(func_, *expanded_args, subcontext->context()); @@ -75,7 +75,7 @@ Result Command::CheckCommand() const { builtin_arguments.args[0] = args_[0]; for (size_t i = 1; i < args_.size(); ++i) { auto expanded_arg = ExpandProps(args_[i]); - if (!expanded_arg) { + if (!expanded_arg.ok()) { if (expanded_arg.error().message().find("doesn't exist while expanding") != std::string::npos) { // If we failed because we won't have a property, use an empty string, which is @@ -114,7 +114,7 @@ Result Action::AddCommand(std::vector&& args, int line) { } auto map_result = function_map_->Find(args); - if (!map_result) { + if (!map_result.ok()) { return Error() << map_result.error(); } @@ -134,7 +134,7 @@ std::size_t Action::NumCommands() const { size_t Action::CheckAllCommands() const { size_t failures = 0; for (const auto& command : commands_) { - if (auto result = command.CheckCommand(); !result) { + if (auto result = command.CheckCommand(); !result.ok()) { LOG(ERROR) << "Command '" << command.BuildCommandString() << "' (" << filename_ << ":" << command.line() << ") failed: " << result.error(); ++failures; @@ -169,7 +169,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().message()); + << (result.ok() ? "succeeded" : "failed: " + result.error().message()); } } diff --git a/init/action_parser.cpp b/init/action_parser.cpp index a8e1e09f8..f3168716f 100644 --- a/init/action_parser.cpp +++ b/init/action_parser.cpp @@ -110,14 +110,14 @@ Result ParseTriggers(const std::vector& args, Subcontext* sub if (!args[i].compare(0, prop_str.length(), prop_str)) { if (auto result = ParsePropertyTrigger(args[i], subcontext, property_triggers); - !result) { + !result.ok()) { return result; } } else { if (!event_trigger->empty()) { return Error() << "multiple event triggers are not allowed"; } - if (auto result = ValidateEventTrigger(args[i]); !result) { + if (auto result = ValidateEventTrigger(args[i]); !result.ok()) { return result; } @@ -145,8 +145,9 @@ Result ActionParser::ParseSection(std::vector&& args, std::string event_trigger; std::map property_triggers; - if (auto result = ParseTriggers(triggers, action_subcontext, &event_trigger, &property_triggers); - !result) { + if (auto result = + ParseTriggers(triggers, action_subcontext, &event_trigger, &property_triggers); + !result.ok()) { return Error() << "ParseTriggers() failed: " << result.error(); } diff --git a/init/builtins.cpp b/init/builtins.cpp index 742e0896d..60c3d40a7 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -164,7 +164,7 @@ static Result do_class_start(const BuiltinArguments& args) { // They must be started individually. for (const auto& service : ServiceList::GetInstance()) { if (service->classnames().count(args[1])) { - if (auto result = service->StartIfNotDisabled(); !result) { + if (auto result = service->StartIfNotDisabled(); !result.ok()) { LOG(ERROR) << "Could not start service '" << service->name() << "' as part of class '" << args[1] << "': " << result.error(); } @@ -186,7 +186,7 @@ static Result do_class_start_post_data(const BuiltinArguments& args) { } for (const auto& service : ServiceList::GetInstance()) { if (service->classnames().count(args[1])) { - if (auto result = service->StartIfPostData(); !result) { + if (auto result = service->StartIfPostData(); !result.ok()) { LOG(ERROR) << "Could not start service '" << service->name() << "' as part of class '" << args[1] << "': " << result.error(); } @@ -227,7 +227,7 @@ static Result do_class_restart(const BuiltinArguments& args) { } static Result do_domainname(const BuiltinArguments& args) { - if (auto result = WriteFile("/proc/sys/kernel/domainname", args[1]); !result) { + if (auto result = WriteFile("/proc/sys/kernel/domainname", args[1]); !result.ok()) { return Error() << "Unable to write to /proc/sys/kernel/domainname: " << result.error(); } return {}; @@ -237,7 +237,7 @@ static Result do_enable(const BuiltinArguments& args) { Service* svc = ServiceList::GetInstance().FindService(args[1]); if (!svc) return Error() << "Could not find service"; - if (auto result = svc->Enable(); !result) { + if (auto result = svc->Enable(); !result.ok()) { return Error() << "Could not enable service: " << result.error(); } @@ -246,10 +246,10 @@ static Result do_enable(const BuiltinArguments& args) { static Result do_exec(const BuiltinArguments& args) { auto service = Service::MakeTemporaryOneshotService(args.args); - if (!service) { + if (!service.ok()) { return Error() << "Could not create exec service: " << service.error(); } - if (auto result = (*service)->ExecStart(); !result) { + if (auto result = (*service)->ExecStart(); !result.ok()) { return Error() << "Could not start exec service: " << result.error(); } @@ -259,10 +259,10 @@ static Result do_exec(const BuiltinArguments& args) { static Result do_exec_background(const BuiltinArguments& args) { auto service = Service::MakeTemporaryOneshotService(args.args); - if (!service) { + if (!service.ok()) { return Error() << "Could not create exec background service: " << service.error(); } - if (auto result = (*service)->Start(); !result) { + if (auto result = (*service)->Start(); !result.ok()) { return Error() << "Could not start exec background service: " << result.error(); } @@ -276,7 +276,7 @@ static Result do_exec_start(const BuiltinArguments& args) { return Error() << "Service not found"; } - if (auto result = service->ExecStart(); !result) { + if (auto result = service->ExecStart(); !result.ok()) { return Error() << "Could not start exec service: " << result.error(); } @@ -291,7 +291,7 @@ static Result do_export(const BuiltinArguments& args) { } static Result do_hostname(const BuiltinArguments& args) { - if (auto result = WriteFile("/proc/sys/kernel/hostname", args[1]); !result) { + if (auto result = WriteFile("/proc/sys/kernel/hostname", args[1]); !result.ok()) { return Error() << "Unable to write to /proc/sys/kernel/hostname: " << result.error(); } return {}; @@ -349,7 +349,7 @@ static Result do_interface_restart(const BuiltinArguments& args) { static Result do_interface_start(const BuiltinArguments& args) { Service* svc = ServiceList::GetInstance().FindInterface(args[1]); if (!svc) return Error() << "interface " << args[1] << " not found"; - if (auto result = svc->Start(); !result) { + if (auto result = svc->Start(); !result.ok()) { return Error() << "Could not start interface: " << result.error(); } return {}; @@ -413,7 +413,7 @@ static Result make_dir_with_options(const MkdirOptions& options) { // mkdir [mode] [owner] [group] [