diff --git a/init/Android.bp b/init/Android.bp index db64f71d4..b1f02793f 100644 --- a/init/Android.bp +++ b/init/Android.bp @@ -159,6 +159,7 @@ cc_test { "devices_test.cpp", "init_test.cpp", "property_service_test.cpp", + "result_test.cpp", "service_test.cpp", "ueventd_test.cpp", "util_test.cpp", diff --git a/init/action.cpp b/init/action.cpp index 4ec5f1739..f687074c6 100644 --- a/init/action.cpp +++ b/init/action.cpp @@ -31,14 +31,13 @@ namespace init { Command::Command(BuiltinFunction f, const std::vector& args, int line) : func_(f), args_(args), line_(line) {} -int Command::InvokeFunc() const { +Result Command::InvokeFunc() const { std::vector expanded_args; expanded_args.resize(args_.size()); expanded_args[0] = args_[0]; for (std::size_t i = 1; i < args_.size(); ++i) { if (!expand_props(args_[i], &expanded_args[i])) { - LOG(ERROR) << args_[0] << ": cannot expand '" << args_[i] << "'"; - return -EINVAL; + return Error() << "cannot expand '" << args_[i] << "'"; } } @@ -54,19 +53,16 @@ Action::Action(bool oneshot, const std::string& filename, int line) const KeywordMap* Action::function_map_ = nullptr; -bool Action::AddCommand(const std::vector& args, int line, std::string* err) { +Result Action::AddCommand(const std::vector& args, int line) { if (!function_map_) { - *err = "no function map available"; - return false; + return Error() << "no function map available"; } - auto function = function_map_->FindFunction(args, err); - if (!function) { - return false; - } + auto function = function_map_->FindFunction(args); + if (!function) return Error() << function.error(); - AddCommand(function, args, line); - return true; + AddCommand(*function, args, line); + return Success(); } void Action::AddCommand(BuiltinFunction f, const std::vector& args, int line) { @@ -92,81 +88,74 @@ void Action::ExecuteAllCommands() const { void Action::ExecuteCommand(const Command& command) const { android::base::Timer t; - int result = command.InvokeFunc(); - + auto result = command.InvokeFunc(); auto duration = t.duration(); + // Any action longer than 50ms will be warned to user as slow operation if (duration > 50ms || android::base::GetMinimumLogSeverity() <= android::base::DEBUG) { std::string trigger_name = BuildTriggersString(); std::string cmd_str = command.BuildCommandString(); LOG(INFO) << "Command '" << cmd_str << "' action=" << trigger_name << " (" << filename_ - << ":" << command.line() << ") returned " << result << " took " - << duration.count() << "ms."; + << ":" << command.line() << ") took " << duration.count() << "ms and " + << (result ? "succeeded" : "failed: " + result.error()); } } -bool Action::ParsePropertyTrigger(const std::string& trigger, std::string* err) { +Result Action::ParsePropertyTrigger(const std::string& trigger) { const static std::string prop_str("property:"); std::string prop_name(trigger.substr(prop_str.length())); size_t equal_pos = prop_name.find('='); if (equal_pos == std::string::npos) { - *err = "property trigger found without matching '='"; - return false; + return Error() << "property trigger found without matching '='"; } std::string prop_value(prop_name.substr(equal_pos + 1)); prop_name.erase(equal_pos); if (auto [it, inserted] = property_triggers_.emplace(prop_name, prop_value); !inserted) { - *err = "multiple property triggers found for same property"; - return false; + return Error() << "multiple property triggers found for same property"; } - return true; + return Success(); } -bool Action::InitTriggers(const std::vector& args, std::string* err) { +Result Action::InitTriggers(const std::vector& args) { const static std::string prop_str("property:"); for (std::size_t i = 0; i < args.size(); ++i) { if (args[i].empty()) { - *err = "empty trigger is not valid"; - return false; + return Error() << "empty trigger is not valid"; } if (i % 2) { if (args[i] != "&&") { - *err = "&& is the only symbol allowed to concatenate actions"; - return false; + return Error() << "&& is the only symbol allowed to concatenate actions"; } else { continue; } } if (!args[i].compare(0, prop_str.length(), prop_str)) { - if (!ParsePropertyTrigger(args[i], err)) { - return false; + if (auto result = ParsePropertyTrigger(args[i]); !result) { + return result; } } else { if (!event_trigger_.empty()) { - *err = "multiple event triggers are not allowed"; - return false; + return Error() << "multiple event triggers are not allowed"; } event_trigger_ = args[i]; } } - return true; + return Success(); } -bool Action::InitSingleTrigger(const std::string& trigger) { +Result Action::InitSingleTrigger(const std::string& trigger) { std::vector name_vector{trigger}; - std::string err; - bool ret = InitTriggers(name_vector, &err); - if (!ret) { - LOG(ERROR) << "InitSingleTrigger failed due to: " << err; + if (auto result = InitTriggers(name_vector); !result) { + return Error() << "InitTriggers() failed: " << result.error(); } - return ret; + return Success(); } // This function checks that all property triggers are satisfied, that is @@ -264,7 +253,8 @@ void ActionManager::QueueBuiltinAction(BuiltinFunction func, const std::string& auto action = std::make_unique(true, "", 0); std::vector name_vector{name}; - if (!action->InitSingleTrigger(name)) { + if (auto result = action->InitSingleTrigger(name); !result) { + LOG(ERROR) << "Cannot queue BuiltinAction for " << name << ": " << result.error(); return; } @@ -333,25 +323,25 @@ void ActionManager::ClearQueue() { current_command_ = 0; } -bool ActionParser::ParseSection(std::vector&& args, const std::string& filename, - int line, std::string* err) { +Result ActionParser::ParseSection(std::vector&& args, + const std::string& filename, int line) { std::vector triggers(args.begin() + 1, args.end()); if (triggers.size() < 1) { - *err = "actions must have a trigger"; - return false; + return Error() << "Actions must have a trigger"; } auto action = std::make_unique(false, filename, line); - if (!action->InitTriggers(triggers, err)) { - return false; + + if (auto result = action->InitTriggers(triggers); !result) { + return Error() << "InitTriggers() failed: " << result.error(); } action_ = std::move(action); - return true; + return Success(); } -bool ActionParser::ParseLineSection(std::vector&& args, int line, std::string* err) { - return action_ ? action_->AddCommand(std::move(args), line, err) : false; +Result ActionParser::ParseLineSection(std::vector&& args, int line) { + return action_ ? action_->AddCommand(std::move(args), line) : Success(); } void ActionParser::EndSection() { diff --git a/init/action.h b/init/action.h index 50cae7171..d977f827a 100644 --- a/init/action.h +++ b/init/action.h @@ -26,6 +26,7 @@ #include "builtins.h" #include "keyword_map.h" #include "parser.h" +#include "result.h" namespace android { namespace init { @@ -34,7 +35,7 @@ class Command { public: Command(BuiltinFunction f, const std::vector& args, int line); - int InvokeFunc() const; + Result InvokeFunc() const; std::string BuildCommandString() const; int line() const { return line_; } @@ -53,10 +54,10 @@ class Action { public: explicit Action(bool oneshot, const std::string& filename, int line); - bool AddCommand(const std::vector& args, int line, std::string* err); + Result AddCommand(const std::vector& args, int line); void AddCommand(BuiltinFunction f, const std::vector& args, int line); - bool InitTriggers(const std::vector& args, std::string* err); - bool InitSingleTrigger(const std::string& trigger); + Result InitTriggers(const std::vector& args); + Result InitSingleTrigger(const std::string& trigger); std::size_t NumCommands() const; void ExecuteOneCommand(std::size_t command) const; void ExecuteAllCommands() const; @@ -78,7 +79,7 @@ private: void ExecuteCommand(const Command& command) const; bool CheckPropertyTriggers(const std::string& name = "", const std::string& value = "") const; - bool ParsePropertyTrigger(const std::string& trigger, std::string* err); + Result ParsePropertyTrigger(const std::string& trigger); std::map property_triggers_; std::string event_trigger_; @@ -120,9 +121,9 @@ class ActionParser : public SectionParser { public: ActionParser(ActionManager* action_manager) : action_manager_(action_manager), action_(nullptr) {} - bool ParseSection(std::vector&& args, const std::string& filename, int line, - std::string* err) override; - bool ParseLineSection(std::vector&& args, int line, std::string* err) override; + Result ParseSection(std::vector&& args, const std::string& filename, + int line) override; + Result ParseLineSection(std::vector&& args, int line) override; void EndSection() override; private: diff --git a/init/bootchart.cpp b/init/bootchart.cpp index 4727f92e9..ec84317c3 100644 --- a/init/bootchart.cpp +++ b/init/bootchart.cpp @@ -163,37 +163,37 @@ static void bootchart_thread_main() { LOG(INFO) << "Bootcharting finished"; } -static int do_bootchart_start() { - // We don't care about the content, but we do care that /data/bootchart/enabled actually exists. - std::string start; - if (!android::base::ReadFileToString("/data/bootchart/enabled", &start)) { - LOG(VERBOSE) << "Not bootcharting"; - return 0; - } +static Result do_bootchart_start() { + // We don't care about the content, but we do care that /data/bootchart/enabled actually exists. + std::string start; + if (!android::base::ReadFileToString("/data/bootchart/enabled", &start)) { + LOG(VERBOSE) << "Not bootcharting"; + return Success(); + } - g_bootcharting_thread = new std::thread(bootchart_thread_main); - return 0; + g_bootcharting_thread = new std::thread(bootchart_thread_main); + return Success(); } -static int do_bootchart_stop() { - if (!g_bootcharting_thread) return 0; +static Result do_bootchart_stop() { + if (!g_bootcharting_thread) return Success(); - // Tell the worker thread it's time to quit. - { - std::lock_guard lock(g_bootcharting_finished_mutex); - g_bootcharting_finished = true; - g_bootcharting_finished_cv.notify_one(); - } + // Tell the worker thread it's time to quit. + { + std::lock_guard lock(g_bootcharting_finished_mutex); + g_bootcharting_finished = true; + g_bootcharting_finished_cv.notify_one(); + } - g_bootcharting_thread->join(); - delete g_bootcharting_thread; - g_bootcharting_thread = nullptr; - return 0; + g_bootcharting_thread->join(); + delete g_bootcharting_thread; + g_bootcharting_thread = nullptr; + return Success(); } -int do_bootchart(const std::vector& args) { - if (args[1] == "start") return do_bootchart_start(); - return do_bootchart_stop(); +Result do_bootchart(const std::vector& args) { + if (args[1] == "start") return do_bootchart_start(); + return do_bootchart_stop(); } } // namespace init diff --git a/init/bootchart.h b/init/bootchart.h index e4f7b59b2..f614f712f 100644 --- a/init/bootchart.h +++ b/init/bootchart.h @@ -20,10 +20,12 @@ #include #include +#include "result.h" + namespace android { namespace init { -int do_bootchart(const std::vector& args); +Result do_bootchart(const std::vector& args); } // namespace init } // namespace android diff --git a/init/builtins.cpp b/init/builtins.cpp index 944fcee22..f807343ab 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -78,47 +78,13 @@ namespace init { static constexpr std::chrono::nanoseconds kCommandRetryTimeout = 5s; -static int insmod(const char *filename, const char *options, int flags) { - unique_fd fd(TEMP_FAILURE_RETRY(open(filename, O_RDONLY | O_NOFOLLOW | O_CLOEXEC))); - if (fd == -1) { - PLOG(ERROR) << "insmod: open(\"" << filename << "\") failed"; - return -1; - } - int rc = syscall(__NR_finit_module, fd.get(), options, flags); - if (rc == -1) { - PLOG(ERROR) << "finit_module for \"" << filename << "\" failed"; - } - return rc; -} - -static int __ifupdown(const char *interface, int up) { - struct ifreq ifr; - - strlcpy(ifr.ifr_name, interface, IFNAMSIZ); - - unique_fd s(TEMP_FAILURE_RETRY(socket(AF_INET, SOCK_DGRAM, 0))); - if (s < 0) return -1; - - int ret = ioctl(s, SIOCGIFFLAGS, &ifr); - if (ret < 0) return ret; - - if (up) { - ifr.ifr_flags |= IFF_UP; - } else { - ifr.ifr_flags &= ~IFF_UP; - } - - return ioctl(s, SIOCSIFFLAGS, &ifr); -} - -static int reboot_into_recovery(const std::vector& options) { +static Result reboot_into_recovery(const std::vector& options) { std::string err; if (!write_bootloader_message(options, &err)) { - LOG(ERROR) << "failed to set bootloader message: " << err; - return -1; + return Error() << "Failed to set bootloader message: " << err; } property_set("sys.powerctl", "reboot,recovery"); - return 0; + return Success(); } template @@ -128,90 +94,106 @@ static void ForEachServiceInClass(const std::string& classname, F function) { } } -static int do_class_start(const std::vector& args) { +static Result do_class_start(const std::vector& args) { // Starting a class does not start services which are explicitly disabled. // They must be started individually. ForEachServiceInClass(args[1], &Service::StartIfNotDisabled); - return 0; + return Success(); } -static int do_class_stop(const std::vector& args) { +static Result do_class_stop(const std::vector& args) { ForEachServiceInClass(args[1], &Service::Stop); - return 0; + return Success(); } -static int do_class_reset(const std::vector& args) { +static Result do_class_reset(const std::vector& args) { ForEachServiceInClass(args[1], &Service::Reset); - return 0; + return Success(); } -static int do_class_restart(const std::vector& args) { +static Result do_class_restart(const std::vector& args) { ForEachServiceInClass(args[1], &Service::Restart); - return 0; + return Success(); } -static int do_domainname(const std::vector& args) { - std::string err; - if (!WriteFile("/proc/sys/kernel/domainname", args[1], &err)) { - LOG(ERROR) << err; - return -1; +static Result do_domainname(const std::vector& args) { + if (auto result = WriteFile("/proc/sys/kernel/domainname", args[1]); !result) { + return Error() << "Unable to write to /proc/sys/kernel/domainname: " << result.error(); } - return 0; + return Success(); } -static int do_enable(const std::vector& args) { +static Result do_enable(const std::vector& args) { Service* svc = ServiceList::GetInstance().FindService(args[1]); - if (!svc) { - return -1; - } - return svc->Enable(); + if (!svc) return Error() << "Could not find service"; + + if (!svc->Enable()) return Error() << "Could not enable service"; + + return Success(); } -static int do_exec(const std::vector& args) { +static Result do_exec(const std::vector& args) { auto service = Service::MakeTemporaryOneshotService(args); if (!service) { - LOG(ERROR) << "Failed to create exec service: " << android::base::Join(args, " "); - return -1; + return Error() << "Could not create exec service"; } if (!service->ExecStart()) { - LOG(ERROR) << "Failed to Start exec service"; - return -1; + return Error() << "Could not start exec service"; } + ServiceList::GetInstance().AddService(std::move(service)); - return 0; + return Success(); } -static int do_exec_start(const std::vector& args) { +static Result do_exec_start(const std::vector& args) { Service* service = ServiceList::GetInstance().FindService(args[1]); if (!service) { - LOG(ERROR) << "ExecStart(" << args[1] << "): Service not found"; - return -1; + return Error() << "Service not found"; } + if (!service->ExecStart()) { - LOG(ERROR) << "ExecStart(" << args[1] << "): Could not start Service"; - return -1; + return Error() << "Could not start Service"; } - return 0; + + return Success(); } -static int do_export(const std::vector& args) { - return add_environment(args[1].c_str(), args[2].c_str()); -} - -static int do_hostname(const std::vector& args) { - std::string err; - if (!WriteFile("/proc/sys/kernel/hostname", args[1], &err)) { - LOG(ERROR) << err; - return -1; +static Result do_export(const std::vector& args) { + if (!add_environment(args[1].c_str(), args[2].c_str())) { + return Error(); } - return 0; + return Success(); } -static int do_ifup(const std::vector& args) { - return __ifupdown(args[1].c_str(), 1); +static Result do_hostname(const std::vector& args) { + if (auto result = WriteFile("/proc/sys/kernel/hostname", args[1]); !result) { + return Error() << "Unable to write to /proc/sys/kernel/hostname: " << result.error(); + } + return Success(); } -static int do_insmod(const std::vector& args) { +static Result do_ifup(const std::vector& args) { + struct ifreq ifr; + + strlcpy(ifr.ifr_name, args[1].c_str(), IFNAMSIZ); + + unique_fd s(TEMP_FAILURE_RETRY(socket(AF_INET, SOCK_DGRAM, 0))); + if (s < 0) return ErrnoError() << "opening socket failed"; + + if (ioctl(s, SIOCGIFFLAGS, &ifr) < 0) { + return ErrnoError() << "ioctl(..., SIOCGIFFLAGS, ...) failed"; + } + + ifr.ifr_flags |= IFF_UP; + + if (ioctl(s, SIOCSIFFLAGS, &ifr) < 0) { + return ErrnoError() << "ioctl(..., SIOCSIFFLAGS, ...) failed"; + } + + return Success(); +} + +static Result do_insmod(const std::vector& args) { int flags = 0; auto it = args.begin() + 1; @@ -222,11 +204,18 @@ static int do_insmod(const std::vector& args) { std::string filename = *it++; std::string options = android::base::Join(std::vector(it, args.end()), ' '); - return insmod(filename.c_str(), options.c_str(), flags); + + unique_fd fd(TEMP_FAILURE_RETRY(open(filename.c_str(), O_RDONLY | O_NOFOLLOW | O_CLOEXEC))); + if (fd == -1) return ErrnoError() << "open(\"" << filename << "\") failed"; + + int rc = syscall(__NR_finit_module, fd.get(), options.c_str(), flags); + if (rc == -1) return ErrnoError() << "finit_module for \"" << filename << "\" failed"; + + return Success(); } // mkdir [mode] [owner] [group] -static int do_mkdir(const std::vector& args) { +static Result do_mkdir(const std::vector& args) { mode_t mode = 0755; if (args.size() >= 3) { mode = std::strtoul(args[2].c_str(), 0, 8); @@ -236,37 +225,35 @@ static int do_mkdir(const std::vector& args) { /* chmod in case the directory already exists */ if (errno == EEXIST) { if (fchmodat(AT_FDCWD, args[1].c_str(), mode, AT_SYMLINK_NOFOLLOW) == -1) { - return -errno; + return ErrnoError() << "fchmodat() failed"; } } else { - return -errno; + return ErrnoError() << "mkdir() failed"; } } if (args.size() >= 4) { - uid_t uid; - std::string decode_uid_err; - if (!DecodeUid(args[3], &uid, &decode_uid_err)) { - LOG(ERROR) << "Unable to find UID for '" << args[3] << "': " << decode_uid_err; - return -1; + auto uid = DecodeUid(args[3]); + if (!uid) { + return Error() << "Unable to decode UID for '" << args[3] << "': " << uid.error(); } - gid_t gid = -1; + Result gid = -1; if (args.size() == 5) { - if (!DecodeUid(args[4], &gid, &decode_uid_err)) { - LOG(ERROR) << "Unable to find GID for '" << args[3] << "': " << decode_uid_err; - return -1; + gid = DecodeUid(args[4]); + if (!gid) { + return Error() << "Unable to decode GID for '" << args[3] << "': " << gid.error(); } } - if (lchown(args[1].c_str(), uid, gid) == -1) { - return -errno; + if (lchown(args[1].c_str(), *uid, *gid) == -1) { + return ErrnoError() << "lchown failed"; } /* chown may have cleared S_ISUID and S_ISGID, chmod again */ if (mode & (S_ISUID | S_ISGID)) { if (fchmodat(AT_FDCWD, args[1].c_str(), mode, AT_SYMLINK_NOFOLLOW) == -1) { - return -errno; + return ErrnoError() << "fchmodat failed"; } } } @@ -277,15 +264,18 @@ static int do_mkdir(const std::vector& args) { "--prompt_and_wipe_data", "--reason=set_policy_failed:"s + args[1]}; reboot_into_recovery(options); - return -1; + return Error() << "reboot into recovery failed"; } } - return 0; + return Success(); } /* umount */ -static int do_umount(const std::vector& args) { - return umount(args[1].c_str()); +static Result do_umount(const std::vector& args) { + if (umount(args[1].c_str()) < 0) { + return ErrnoError() << "umount() failed"; + } + return Success(); } static struct { @@ -313,7 +303,7 @@ static struct { #define DATA_MNT_POINT "/data" /* mount */ -static int do_mount(const std::vector& args) { +static Result do_mount(const std::vector& args) { const char* options = nullptr; unsigned flags = 0; bool wait = false; @@ -344,12 +334,12 @@ static int do_mount(const std::vector& args) { if (android::base::StartsWith(source, "loop@")) { int mode = (flags & MS_RDONLY) ? O_RDONLY : O_RDWR; unique_fd fd(TEMP_FAILURE_RETRY(open(source + 5, mode | O_CLOEXEC))); - if (fd < 0) return -1; + if (fd < 0) return ErrnoError() << "open(" << source + 5 << ", " << mode << ") failed"; for (size_t n = 0;; n++) { std::string tmp = android::base::StringPrintf("/dev/block/loop%zu", n); unique_fd loop(TEMP_FAILURE_RETRY(open(tmp.c_str(), mode | O_CLOEXEC))); - if (loop < 0) return -1; + if (loop < 0) return ErrnoError() << "open(" << tmp << ", " << mode << ") failed"; loop_info info; /* if it is a blank loop device */ @@ -358,26 +348,24 @@ static int do_mount(const std::vector& args) { if (ioctl(loop, LOOP_SET_FD, fd.get()) >= 0) { if (mount(tmp.c_str(), target, system, flags, options) < 0) { ioctl(loop, LOOP_CLR_FD, 0); - return -1; + return ErrnoError() << "mount() failed"; } - return 0; + return Success(); } } } - LOG(ERROR) << "out of loopback devices"; - return -1; + return Error() << "out of loopback devices"; } else { if (wait) wait_for_file(source, kCommandRetryTimeout); if (mount(source, target, system, flags, options) < 0) { - return -1; + return ErrnoError() << "mount() failed"; } } - return 0; - + return Success(); } /* Imports .rc files from the specified paths. Default ones are applied if none is given. @@ -409,9 +397,7 @@ static void import_late(const std::vector& args, size_t start_index * * Call fs_mgr_mount_all() to mount the given fstab */ -static int mount_fstab(const char* fstabfile, int mount_mode) { - int ret = -1; - +static Result mount_fstab(const char* fstabfile, int mount_mode) { /* * Call fs_mgr_mount_all() to mount all filesystems. We fork(2) and * do the call in the child to provide protection to the main init @@ -429,9 +415,9 @@ static int mount_fstab(const char* fstabfile, int mount_mode) { } if (WIFEXITED(status)) { - ret = WEXITSTATUS(status); + return WEXITSTATUS(status); } else { - ret = -1; + return Error() << "child aborted"; } } else if (pid == 0) { /* child, call fs_mgr_mount_all() */ @@ -448,10 +434,8 @@ static int mount_fstab(const char* fstabfile, int mount_mode) { } _exit(child_ret); } else { - /* fork failed, return an error */ - return -1; + return Error() << "fork() failed"; } - return ret; } /* Queue event based on fs_mgr return code. @@ -463,29 +447,33 @@ static int mount_fstab(const char* fstabfile, int mount_mode) { * * return code is processed based on input code */ -static int queue_fs_event(int code) { - int ret = code; +static Result queue_fs_event(int code) { if (code == FS_MGR_MNTALL_DEV_NEEDS_ENCRYPTION) { ActionManager::GetInstance().QueueEventTrigger("encrypt"); + return Success(); } else if (code == FS_MGR_MNTALL_DEV_MIGHT_BE_ENCRYPTED) { property_set("ro.crypto.state", "encrypted"); property_set("ro.crypto.type", "block"); ActionManager::GetInstance().QueueEventTrigger("defaultcrypto"); + return Success(); } else if (code == FS_MGR_MNTALL_DEV_NOT_ENCRYPTED) { property_set("ro.crypto.state", "unencrypted"); ActionManager::GetInstance().QueueEventTrigger("nonencrypted"); + return Success(); } else if (code == FS_MGR_MNTALL_DEV_NOT_ENCRYPTABLE) { property_set("ro.crypto.state", "unsupported"); ActionManager::GetInstance().QueueEventTrigger("nonencrypted"); + return Success(); } else if (code == FS_MGR_MNTALL_DEV_NEEDS_RECOVERY) { /* Setup a wipe via recovery, and reboot into recovery */ PLOG(ERROR) << "fs_mgr_mount_all suggested recovery, so wiping data via recovery."; const std::vector options = {"--wipe_data", "--reason=fs_mgr_mount_all" }; - ret = reboot_into_recovery(options); + reboot_into_recovery(options); + return Error() << "reboot_into_recovery() failed"; /* If reboot worked, there is no return. */ } else if (code == FS_MGR_MNTALL_DEV_FILE_ENCRYPTED) { if (e4crypt_install_keyring()) { - return -1; + return Error() << "e4crypt_install_keyring() failed"; } property_set("ro.crypto.state", "encrypted"); property_set("ro.crypto.type", "file"); @@ -493,12 +481,13 @@ static int queue_fs_event(int code) { // Although encrypted, we have device key, so we do not need to // do anything different from the nonencrypted case. ActionManager::GetInstance().QueueEventTrigger("nonencrypted"); + return Success(); } else if (code > 0) { - PLOG(ERROR) << "fs_mgr_mount_all returned unexpected error " << code; + Error() << "fs_mgr_mount_all() returned unexpected error " << code; } /* else ... < 0: error */ - return ret; + return Error() << "Invalid code: " << code; } /* mount_all [ ]* [--]* @@ -506,7 +495,7 @@ static int queue_fs_event(int code) { * This function might request a reboot, in which case it will * not return. */ -static int do_mount_all(const std::vector& args) { +static Result do_mount_all(const std::vector& args) { std::size_t na = 0; bool import_rc = true; bool queue_event = true; @@ -531,7 +520,10 @@ static int do_mount_all(const std::vector& args) { std::string prop_name = "ro.boottime.init.mount_all."s + prop_post_fix; android::base::Timer t; - int ret = mount_fstab(fstabfile, mount_mode); + auto mount_fstab_return_code = mount_fstab(fstabfile, mount_mode); + if (!mount_fstab_return_code) { + return Error() << "mount_fstab() failed " << mount_fstab_return_code.error(); + } property_set(prop_name, std::to_string(t.duration().count())); if (import_rc) { @@ -542,13 +534,16 @@ static int do_mount_all(const std::vector& args) { if (queue_event) { /* queue_fs_event will queue event based on mount_fstab return code * and return processed return code*/ - ret = queue_fs_event(ret); + auto queue_fs_result = queue_fs_event(*mount_fstab_return_code); + if (!queue_fs_result) { + return Error() << "queue_fs_event() failed: " << queue_fs_result.error(); + } } - return ret; + return Success(); } -static int do_swapon_all(const std::vector& args) { +static Result do_swapon_all(const std::vector& args) { struct fstab *fstab; int ret; @@ -556,89 +551,103 @@ static int do_swapon_all(const std::vector& args) { ret = fs_mgr_swapon_all(fstab); fs_mgr_free_fstab(fstab); - return ret; + if (ret != 0) return Error() << "fs_mgr_swapon_all() failed"; + return Success(); } -static int do_setprop(const std::vector& args) { +static Result do_setprop(const std::vector& args) { property_set(args[1], args[2]); - return 0; + return Success(); } -static int do_setrlimit(const std::vector& args) { - struct rlimit limit; +static Result do_setrlimit(const std::vector& args) { int resource; - if (android::base::ParseInt(args[1], &resource) && - android::base::ParseUint(args[2], &limit.rlim_cur) && - android::base::ParseUint(args[3], &limit.rlim_max)) { - return setrlimit(resource, &limit); + if (!android::base::ParseInt(args[1], &resource)) { + return Error() << "unable to parse resource, " << args[1]; } - LOG(WARNING) << "ignoring setrlimit " << args[1] << " " << args[2] << " " << args[3]; - return -1; + + struct rlimit limit; + if (!android::base::ParseUint(args[2], &limit.rlim_cur)) { + return Error() << "unable to parse rlim_cur, " << args[2]; + } + if (!android::base::ParseUint(args[3], &limit.rlim_max)) { + return Error() << "unable to parse rlim_max, " << args[3]; + } + + if (setrlimit(resource, &limit) == -1) { + return ErrnoError() << "setrlimit failed"; + } + return Success(); } -static int do_start(const std::vector& args) { +static Result do_start(const std::vector& args) { Service* svc = ServiceList::GetInstance().FindService(args[1]); - if (!svc) { - LOG(ERROR) << "do_start: Service " << args[1] << " not found"; - return -1; - } - if (!svc->Start()) - return -1; - return 0; + if (!svc) return Error() << "service " << args[1] << " not found"; + if (!svc->Start()) return Error() << "failed to start service"; + return Success(); } -static int do_stop(const std::vector& args) { +static Result do_stop(const std::vector& args) { Service* svc = ServiceList::GetInstance().FindService(args[1]); - if (!svc) { - LOG(ERROR) << "do_stop: Service " << args[1] << " not found"; - return -1; - } + if (!svc) return Error() << "service " << args[1] << " not found"; svc->Stop(); - return 0; + return Success(); } -static int do_restart(const std::vector& args) { +static Result do_restart(const std::vector& args) { Service* svc = ServiceList::GetInstance().FindService(args[1]); - if (!svc) { - LOG(ERROR) << "do_restart: Service " << args[1] << " not found"; - return -1; - } + if (!svc) return Error() << "service " << args[1] << " not found"; svc->Restart(); - return 0; + return Success(); } -static int do_trigger(const std::vector& args) { +static Result do_trigger(const std::vector& args) { ActionManager::GetInstance().QueueEventTrigger(args[1]); - return 0; + return Success(); } -static int do_symlink(const std::vector& args) { - return symlink(args[1].c_str(), args[2].c_str()); -} - -static int do_rm(const std::vector& args) { - return unlink(args[1].c_str()); -} - -static int do_rmdir(const std::vector& args) { - return rmdir(args[1].c_str()); -} - -static int do_sysclktz(const std::vector& args) { - struct timezone tz = {}; - if (android::base::ParseInt(args[1], &tz.tz_minuteswest) && settimeofday(nullptr, &tz) != -1) { - return 0; +static Result do_symlink(const std::vector& args) { + if (symlink(args[1].c_str(), args[2].c_str()) < 0) { + return ErrnoError() << "symlink() failed"; } - return -1; + return Success(); } -static int do_verity_load_state(const std::vector& args) { +static Result do_rm(const std::vector& args) { + if (unlink(args[1].c_str()) < 0) { + return ErrnoError() << "unlink() failed"; + } + return Success(); +} + +static Result do_rmdir(const std::vector& args) { + if (rmdir(args[1].c_str()) < 0) { + return ErrnoError() << "rmdir() failed"; + } + return Success(); +} + +static Result do_sysclktz(const std::vector& args) { + struct timezone tz = {}; + if (!android::base::ParseInt(args[1], &tz.tz_minuteswest)) { + return Error() << "Unable to parse mins_west_of_gmt"; + } + + if (settimeofday(nullptr, &tz) == -1) { + return ErrnoError() << "settimeofday() failed"; + } + return Success(); +} + +static Result do_verity_load_state(const std::vector& args) { int mode = -1; bool loaded = fs_mgr_load_verity_state(&mode); if (loaded && mode != VERITY_MODE_DEFAULT) { ActionManager::GetInstance().QueueEventTrigger("verity-logging"); } - return loaded ? 0 : 1; + if (!loaded) return Error() << "Could not load verity state"; + + return Success(); } static void verity_update_property(fstab_rec *fstab, const char *mount_point, @@ -646,25 +655,26 @@ static void verity_update_property(fstab_rec *fstab, const char *mount_point, property_set("partition."s + mount_point + ".verified", std::to_string(mode)); } -static int do_verity_update_state(const std::vector& args) { - return fs_mgr_update_verity_state(verity_update_property) ? 0 : 1; -} - -static int do_write(const std::vector& args) { - std::string err; - if (!WriteFile(args[1], args[2], &err)) { - LOG(ERROR) << err; - return -1; +static Result do_verity_update_state(const std::vector& args) { + if (!fs_mgr_update_verity_state(verity_update_property)) { + return Error() << "fs_mgr_update_verity_state() failed"; } - return 0; + return Success(); } -static int do_readahead(const std::vector& args) { +static Result do_write(const std::vector& args) { + if (auto result = WriteFile(args[1], args[2]); !result) { + return Error() << "Unable to write to file '" << args[1] << "': " << result.error(); + } + + return Success(); +} + +static Result do_readahead(const std::vector& args) { struct stat sb; if (stat(args[1].c_str(), &sb)) { - PLOG(ERROR) << "Error opening " << args[1]; - return -1; + return ErrnoError() << "Error opening " << args[1]; } // We will do readahead in a forked process in order not to block init @@ -713,48 +723,45 @@ static int do_readahead(const std::vector& args) { LOG(INFO) << "Readahead " << args[1] << " took " << t; _exit(0); } else if (pid < 0) { - PLOG(ERROR) << "Fork failed"; - return -1; + return ErrnoError() << "Fork failed"; } - return 0; + return Success(); } -static int do_copy(const std::vector& args) { - std::string data; - std::string err; - if (!ReadFile(args[1], &data, &err)) { - LOG(ERROR) << err; - return -1; +static Result do_copy(const std::vector& args) { + auto file_contents = ReadFile(args[1]); + if (!file_contents) { + return Error() << "Could not read input file '" << args[1] << "': " << file_contents.error(); } - if (!WriteFile(args[2], data, &err)) { - LOG(ERROR) << err; - return -1; + if (auto result = WriteFile(args[2], *file_contents); !result) { + return Error() << "Could not write to output file '" << args[2] << "': " << result.error(); } - return 0; + + return Success(); } -static int do_chown(const std::vector& args) { - uid_t uid; - std::string decode_uid_err; - if (!DecodeUid(args[1], &uid, &decode_uid_err)) { - LOG(ERROR) << "Unable to find UID for '" << args[1] << "': " << decode_uid_err; - return -1; +static Result do_chown(const std::vector& args) { + auto uid = DecodeUid(args[1]); + if (!uid) { + return Error() << "Unable to decode UID for '" << args[1] << "': " << uid.error(); } // GID is optional and pushes the index of path out by one if specified. const std::string& path = (args.size() == 4) ? args[3] : args[2]; - gid_t gid = -1; + Result gid = -1; if (args.size() == 4) { - if (!DecodeUid(args[2], &gid, &decode_uid_err)) { - LOG(ERROR) << "Unable to find GID for '" << args[2] << "': " << decode_uid_err; - return -1; + gid = DecodeUid(args[2]); + if (!gid) { + return Error() << "Unable to decode GID for '" << args[2] << "': " << gid.error(); } } - if (lchown(path.c_str(), uid, gid) == -1) return -errno; + if (lchown(path.c_str(), *uid, *gid) == -1) { + return ErrnoError() << "lchown() failed"; + } - return 0; + return Success(); } static mode_t get_mode(const char *s) { @@ -770,15 +777,15 @@ static mode_t get_mode(const char *s) { return mode; } -static int do_chmod(const std::vector& args) { +static Result do_chmod(const std::vector& args) { mode_t mode = get_mode(args[1].c_str()); if (fchmodat(AT_FDCWD, args[2].c_str(), mode, AT_SYMLINK_NOFOLLOW) < 0) { - return -errno; + return ErrnoError() << "fchmodat() failed"; } - return 0; + return Success(); } -static int do_restorecon(const std::vector& args) { +static Result do_restorecon(const std::vector& args) { int ret = 0; struct flag_type {const char* name; int value;}; @@ -795,8 +802,7 @@ static int do_restorecon(const std::vector& args) { for (size_t i = 1; i < args.size(); ++i) { if (android::base::StartsWith(args[i], "--")) { if (!in_flags) { - LOG(ERROR) << "restorecon - flags must precede paths"; - return -1; + return Error() << "flags must precede paths"; } bool found = false; for (size_t j = 0; flags[j].name; ++j) { @@ -807,26 +813,27 @@ static int do_restorecon(const std::vector& args) { } } if (!found) { - LOG(ERROR) << "restorecon - bad flag " << args[i]; - return -1; + return Error() << "bad flag " << args[i]; } } else { in_flags = false; if (selinux_android_restorecon(args[i].c_str(), flag) < 0) { - ret = -errno; + ret = errno; } } } - return ret; + + if (ret) return ErrnoError() << "selinux_android_restorecon() failed"; + return Success(); } -static int do_restorecon_recursive(const std::vector& args) { +static Result do_restorecon_recursive(const std::vector& args) { std::vector non_const_args(args); non_const_args.insert(std::next(non_const_args.begin()), "--recursive"); return do_restorecon(non_const_args); } -static int do_loglevel(const std::vector& args) { +static Result do_loglevel(const std::vector& args) { // TODO: support names instead/as well? int log_level = -1; android::base::ParseInt(args[1], &log_level); @@ -841,77 +848,73 @@ static int do_loglevel(const std::vector& args) { case 1: case 0: severity = android::base::FATAL; break; default: - LOG(ERROR) << "loglevel: invalid log level " << log_level; - return -EINVAL; + return Error() << "invalid log level " << log_level; } android::base::SetMinimumLogSeverity(severity); - return 0; + return Success(); } -static int do_load_persist_props(const std::vector& args) { +static Result do_load_persist_props(const std::vector& args) { load_persist_props(); - return 0; + return Success(); } -static int do_load_system_props(const std::vector& args) { +static Result do_load_system_props(const std::vector& args) { load_system_props(); - return 0; + return Success(); } -static int do_wait(const std::vector& args) { - if (args.size() == 2) { - return wait_for_file(args[1].c_str(), kCommandRetryTimeout); - } else if (args.size() == 3) { - int timeout; - if (android::base::ParseInt(args[2], &timeout)) { - return wait_for_file(args[1].c_str(), std::chrono::seconds(timeout)); +static Result do_wait(const std::vector& args) { + auto timeout = kCommandRetryTimeout; + if (args.size() == 3) { + int timeout_int; + if (!android::base::ParseInt(args[2], &timeout_int)) { + return Error() << "failed to parse timeout"; } + timeout = std::chrono::seconds(timeout_int); } - return -1; + + if (wait_for_file(args[1].c_str(), timeout) != 0) { + return Error() << "wait_for_file() failed"; + } + + return Success(); } -static int do_wait_for_prop(const std::vector& args) { +static Result do_wait_for_prop(const std::vector& args) { const char* name = args[1].c_str(); const char* value = args[2].c_str(); size_t value_len = strlen(value); if (!is_legal_property_name(name)) { - LOG(ERROR) << "do_wait_for_prop(\"" << name << "\", \"" << value - << "\") failed: bad name"; - return -1; + return Error() << "is_legal_property_name(" << name << ") failed"; } if (value_len >= PROP_VALUE_MAX) { - LOG(ERROR) << "do_wait_for_prop(\"" << name << "\", \"" << value - << "\") failed: value too long"; - return -1; + return Error() << "value too long"; } if (!start_waiting_for_property(name, value)) { - LOG(ERROR) << "do_wait_for_prop(\"" << name << "\", \"" << value - << "\") failed: init already in waiting"; - return -1; + return Error() << "already waiting for a property"; } - return 0; + return Success(); } static bool is_file_crypto() { return android::base::GetProperty("ro.crypto.type", "") == "file"; } -static int do_installkey(const std::vector& args) { - if (!is_file_crypto()) { - return 0; - } +static Result do_installkey(const std::vector& args) { + if (!is_file_crypto()) return Success(); + auto unencrypted_dir = args[1] + e4crypt_unencrypted_folder; if (!make_dir(unencrypted_dir, 0700) && errno != EEXIST) { - PLOG(ERROR) << "Failed to create " << unencrypted_dir; - return -1; + return ErrnoError() << "Failed to create " << unencrypted_dir; } std::vector exec_args = {"exec", "/system/bin/vdc", "--wait", "cryptfs", "enablefilecrypto"}; return do_exec(exec_args); } -static int do_init_user0(const std::vector& args) { +static Result do_init_user0(const std::vector& args) { std::vector exec_args = {"exec", "/system/bin/vdc", "--wait", "cryptfs", "init_user0"}; return do_exec(exec_args); diff --git a/init/builtins.h b/init/builtins.h index b110f619f..f66ae1940 100644 --- a/init/builtins.h +++ b/init/builtins.h @@ -23,11 +23,12 @@ #include #include "keyword_map.h" +#include "result.h" namespace android { namespace init { -using BuiltinFunction = std::function&)>; +using BuiltinFunction = std::function(const std::vector&)>; class BuiltinFunctionMap : public KeywordMap { public: BuiltinFunctionMap() {} diff --git a/init/import_parser.cpp b/init/import_parser.cpp index b9fa2cea7..e335fd111 100644 --- a/init/import_parser.cpp +++ b/init/import_parser.cpp @@ -23,24 +23,22 @@ namespace android { namespace init { -bool ImportParser::ParseSection(std::vector&& args, const std::string& filename, - int line, std::string* err) { +Result ImportParser::ParseSection(std::vector&& args, + const std::string& filename, int line) { if (args.size() != 2) { - *err = "single argument needed for import\n"; - return false; + return Error() << "single argument needed for import\n"; } std::string conf_file; bool ret = expand_props(args[1], &conf_file); if (!ret) { - *err = "error while expanding import"; - return false; + return Error() << "error while expanding import"; } LOG(INFO) << "Added '" << conf_file << "' to import list"; if (filename_.empty()) filename_ = filename; imports_.emplace_back(std::move(conf_file), line); - return true; + return Success(); } void ImportParser::EndFile() { diff --git a/init/import_parser.h b/init/import_parser.h index 0d04e0e27..5a2f89498 100644 --- a/init/import_parser.h +++ b/init/import_parser.h @@ -28,8 +28,8 @@ namespace init { class ImportParser : public SectionParser { public: ImportParser(Parser* parser) : parser_(parser) {} - bool ParseSection(std::vector&& args, const std::string& filename, int line, - std::string* err) override; + Result ParseSection(std::vector&& args, const std::string& filename, + int line) override; void EndFile() override; private: diff --git a/init/init.cpp b/init/init.cpp index ee3a84c29..c65d8460b 100644 --- a/init/init.cpp +++ b/init/init.cpp @@ -238,7 +238,7 @@ void handle_control_message(const std::string& msg, const std::string& name) { } } -static int wait_for_coldboot_done_action(const std::vector& args) { +static Result wait_for_coldboot_done_action(const std::vector& args) { Timer t; LOG(VERBOSE) << "Waiting for " COLDBOOT_DONE "..."; @@ -257,22 +257,20 @@ static int wait_for_coldboot_done_action(const std::vector& args) { } property_set("ro.boottime.init.cold_boot_wait", std::to_string(t.duration().count())); - return 0; + return Success(); } -static int keychord_init_action(const std::vector& args) -{ +static Result keychord_init_action(const std::vector& args) { keychord_init(); - return 0; + return Success(); } -static int console_init_action(const std::vector& args) -{ +static Result console_init_action(const std::vector& args) { std::string console = GetProperty("ro.boot.console", ""); if (!console.empty()) { default_console = "/dev/" + console; } - return 0; + return Success(); } static void import_kernel_nv(const std::string& key, const std::string& value, bool for_emulator) { @@ -354,18 +352,16 @@ static void process_kernel_cmdline() { if (qemu[0]) import_kernel_cmdline(true, import_kernel_nv); } -static int property_enable_triggers_action(const std::vector& args) -{ +static Result property_enable_triggers_action(const std::vector& args) { /* Enable property triggers. */ property_triggers_enabled = 1; - return 0; + return Success(); } -static int queue_property_triggers_action(const std::vector& args) -{ +static Result queue_property_triggers_action(const std::vector& args) { ActionManager::GetInstance().QueueBuiltinAction(property_enable_triggers_action, "enable_property_trigger"); ActionManager::GetInstance().QueueAllPropertyActions(); - return 0; + return Success(); } static void global_seccomp() { diff --git a/init/init_test.cpp b/init/init_test.cpp index 20622901c..27659f917 100644 --- a/init/init_test.cpp +++ b/init/init_test.cpp @@ -37,7 +37,7 @@ class TestFunctionMap : public KeywordMap { void Add(const std::string& name, const BuiltinFunctionNoArgs function) { Add(name, 0, 0, [function](const std::vector&) { function(); - return 0; + return Success(); }); } @@ -156,10 +156,9 @@ TEST(init, EventTriggerOrderMultipleFiles) { "execute 3"; // clang-format on // WriteFile() ensures the right mode is set - std::string err; - ASSERT_TRUE(WriteFile(std::string(dir.path) + "/a.rc", dir_a_script, &err)); + ASSERT_TRUE(WriteFile(std::string(dir.path) + "/a.rc", dir_a_script)); - ASSERT_TRUE(WriteFile(std::string(dir.path) + "/b.rc", "on boot\nexecute 5", &err)); + ASSERT_TRUE(WriteFile(std::string(dir.path) + "/b.rc", "on boot\nexecute 5")); // clang-format off std::string start_script = "import " + std::string(first_import.path) + "\n" @@ -175,7 +174,7 @@ TEST(init, EventTriggerOrderMultipleFiles) { auto execute_command = [&num_executed](const std::vector& args) { EXPECT_EQ(2U, args.size()); EXPECT_EQ(++num_executed, std::stoi(args[1])); - return 0; + return Success(); }; TestFunctionMap test_function_map; diff --git a/init/keyword_map.h b/init/keyword_map.h index 481d63702..c95fc7318 100644 --- a/init/keyword_map.h +++ b/init/keyword_map.h @@ -22,6 +22,8 @@ #include +#include "result.h" + namespace android { namespace init { @@ -34,20 +36,17 @@ class KeywordMap { virtual ~KeywordMap() { } - const Function FindFunction(const std::vector& args, std::string* err) const { + const Result FindFunction(const std::vector& args) const { using android::base::StringPrintf; - if (args.empty()) { - *err = "keyword needed, but not provided"; - return nullptr; - } + if (args.empty()) return Error() << "Keyword needed, but not provided"; + auto& keyword = args[0]; auto num_args = args.size() - 1; auto function_info_it = map().find(keyword); if (function_info_it == map().end()) { - *err = StringPrintf("invalid keyword '%s'", keyword.c_str()); - return nullptr; + return Error() << StringPrintf("Invalid keyword '%s'", keyword.c_str()); } auto function_info = function_info_it->second; @@ -55,22 +54,18 @@ class KeywordMap { auto min_args = std::get<0>(function_info); auto max_args = std::get<1>(function_info); if (min_args == max_args && num_args != min_args) { - *err = StringPrintf("%s requires %zu argument%s", - keyword.c_str(), min_args, - (min_args > 1 || min_args == 0) ? "s" : ""); - return nullptr; + return Error() << StringPrintf("%s requires %zu argument%s", keyword.c_str(), min_args, + (min_args > 1 || min_args == 0) ? "s" : ""); } if (num_args < min_args || num_args > max_args) { if (max_args == std::numeric_limits::max()) { - *err = StringPrintf("%s requires at least %zu argument%s", - keyword.c_str(), min_args, - min_args > 1 ? "s" : ""); + return Error() << StringPrintf("%s requires at least %zu argument%s", + keyword.c_str(), min_args, min_args > 1 ? "s" : ""); } else { - *err = StringPrintf("%s requires between %zu and %zu arguments", - keyword.c_str(), min_args, max_args); + return Error() << StringPrintf("%s requires between %zu and %zu arguments", + keyword.c_str(), min_args, max_args); } - return nullptr; } return std::get(function_info); diff --git a/init/parser.cpp b/init/parser.cpp index c6f4f459b..8a4e798d0 100644 --- a/init/parser.cpp +++ b/init/parser.cpp @@ -67,9 +67,8 @@ void Parser::ParseData(const std::string& filename, const std::string& data) { if (android::base::StartsWith(args[0], prefix.c_str())) { if (section_parser) section_parser->EndSection(); - std::string ret_err; - if (!callback(std::move(args), &ret_err)) { - LOG(ERROR) << filename << ": " << state.line << ": " << ret_err; + if (auto result = callback(std::move(args)); !result) { + LOG(ERROR) << filename << ": " << state.line << ": " << result.error(); } section_parser = nullptr; break; @@ -78,16 +77,16 @@ void Parser::ParseData(const std::string& filename, const std::string& data) { if (section_parsers_.count(args[0])) { if (section_parser) section_parser->EndSection(); section_parser = section_parsers_[args[0]].get(); - std::string ret_err; - if (!section_parser->ParseSection(std::move(args), filename, state.line, - &ret_err)) { - LOG(ERROR) << filename << ": " << state.line << ": " << ret_err; + if (auto result = + section_parser->ParseSection(std::move(args), filename, state.line); + !result) { + LOG(ERROR) << filename << ": " << state.line << ": " << result.error(); section_parser = nullptr; } } else if (section_parser) { - std::string ret_err; - if (!section_parser->ParseLineSection(std::move(args), state.line, &ret_err)) { - LOG(ERROR) << filename << ": " << state.line << ": " << ret_err; + if (auto result = section_parser->ParseLineSection(std::move(args), state.line); + !result) { + LOG(ERROR) << filename << ": " << state.line << ": " << result.error(); } } args.clear(); @@ -102,15 +101,14 @@ void Parser::ParseData(const std::string& filename, const std::string& data) { bool Parser::ParseConfigFile(const std::string& path) { LOG(INFO) << "Parsing file " << path << "..."; android::base::Timer t; - std::string data; - std::string err; - if (!ReadFile(path, &data, &err)) { - LOG(ERROR) << err; + auto config_contents = ReadFile(path); + if (!config_contents) { + LOG(ERROR) << "Unable to read config file '" << path << "': " << config_contents.error(); return false; } - data.push_back('\n'); // TODO: fix parse_config. - ParseData(path, data); + config_contents->push_back('\n'); // TODO: fix parse_config. + ParseData(path, *config_contents); for (const auto& [section_name, section_parser] : section_parsers_) { section_parser->EndFile(); } diff --git a/init/parser.h b/init/parser.h index fd65ad6ff..4ab24a4ee 100644 --- a/init/parser.h +++ b/init/parser.h @@ -22,6 +22,8 @@ #include #include +#include "result.h" + // SectionParser is an interface that can parse a given 'section' in init. // // You can implement up to 4 functions below, with ParseSection() being mandatory. @@ -51,9 +53,9 @@ namespace init { class SectionParser { public: virtual ~SectionParser() {} - virtual bool ParseSection(std::vector&& args, const std::string& filename, - int line, std::string* err) = 0; - virtual bool ParseLineSection(std::vector&&, int, std::string*) { return true; }; + virtual Result ParseSection(std::vector&& args, + const std::string& filename, int line) = 0; + virtual Result ParseLineSection(std::vector&&, int) { return Success(); }; virtual void EndSection(){}; virtual void EndFile(){}; }; @@ -67,7 +69,7 @@ class Parser { // Similar to ParseSection() and ParseLineSection(), this function returns bool with false // indicating a failure and has an std::string* err parameter into which an error string can // be written. - using LineCallback = std::function&&, std::string*)>; + using LineCallback = std::function(std::vector&&)>; Parser(); diff --git a/init/property_service.cpp b/init/property_service.cpp index 1db8cb7d3..e07550a39 100644 --- a/init/property_service.cpp +++ b/init/property_service.cpp @@ -588,14 +588,14 @@ static void load_properties(char *data, const char *filter) // "ro.foo.*" is a prefix match, and "ro.foo.bar" is an exact match. static void load_properties_from_file(const char* filename, const char* filter) { Timer t; - std::string data; - std::string err; - if (!ReadFile(filename, &data, &err)) { - PLOG(WARNING) << "Couldn't load property file: " << err; + auto file_contents = ReadFile(filename); + if (!file_contents) { + PLOG(WARNING) << "Couldn't load property file '" << filename + << "': " << file_contents.error(); return; } - data.push_back('\n'); - load_properties(&data[0], filter); + file_contents->push_back('\n'); + load_properties(file_contents->data(), filter); LOG(VERBOSE) << "(Loading properties from " << filename << " took " << t << ".)"; } diff --git a/init/reboot.cpp b/init/reboot.cpp index 32816fe40..24ccdfc6a 100644 --- a/init/reboot.cpp +++ b/init/reboot.cpp @@ -520,7 +520,7 @@ bool HandlePowerctlMessage(const std::string& command) { auto shutdown_handler = [cmd, command, reboot_target, run_fsck](const std::vector&) { DoReboot(cmd, command, reboot_target, run_fsck); - return 0; + return Success(); }; ActionManager::GetInstance().QueueBuiltinAction(shutdown_handler, "shutdown_done"); diff --git a/init/result.h b/init/result.h new file mode 100644 index 000000000..64fa69f26 --- /dev/null +++ b/init/result.h @@ -0,0 +1,142 @@ +/* + * 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 std::string describing an error, which can be accessed via +// Result::error(). +// +// 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. Each of these classes +// take an ostream as an input and are implicitly cast to a Result containing that failure. +// ErrnoError() additionally appends ": " + strerror(errno) to the end of the failure string to aid +// in interacting with C APIs. + +// 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); + +#ifndef _INIT_RESULT_H +#define _INIT_RESULT_H + +#include + +#include +#include +#include + +namespace android { +namespace init { + +class Error { + public: + Error() : append_errno_(0) {} + + template + Error&& operator<<(T&& t) { + ss_ << std::forward(t); + return std::move(*this); + } + + const std::string str() const { + if (append_errno_) { + return ss_.str() + ": " + strerror(append_errno_); + } + return ss_.str(); + } + + 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_; +}; + +class ErrnoError : public Error { + public: + ErrnoError() : Error(errno) {} +}; + +template +class Result { + public: + template + Result(U&&... result) : contents_(std::in_place_index_t<0>(), std::forward(result)...) {} + + Result(Error&& fb) : contents_(std::in_place_index_t<1>(), fb.str()) {} + + 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 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_)); } + + 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 Success = std::monostate; + +} // namespace init +} // namespace android + +#endif diff --git a/init/result_test.cpp b/init/result_test.cpp new file mode 100644 index 000000000..ca6501369 --- /dev/null +++ b/init/result_test.cpp @@ -0,0 +1,222 @@ +/* + * 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("failure1", result.error()); +} + +TEST(result, result_error_empty) { + Result result = Error(); + ASSERT_FALSE(result); + ASSERT_FALSE(result.has_value()); + + EXPECT_EQ("", result.error()); +} + +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("failure1", MakeRvalueErrorResult().error()); +} + +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("failure1: "s + strerror(test_errno), result.error()); +} + +TEST(result, constructor_forwarding) { + auto result = Result(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); +} + +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/security.cpp b/init/security.cpp index 45a5412bf..f8976dec2 100644 --- a/init/security.cpp +++ b/init/security.cpp @@ -45,24 +45,22 @@ namespace init { // devices/configurations where these I/O operations are blocking for a long // time. We do not reboot or halt on failures, as this is a best-effort // attempt. -int MixHwrngIntoLinuxRngAction(const std::vector& args) { +Result MixHwrngIntoLinuxRngAction(const std::vector& args) { unique_fd hwrandom_fd( TEMP_FAILURE_RETRY(open("/dev/hw_random", O_RDONLY | O_NOFOLLOW | O_CLOEXEC))); if (hwrandom_fd == -1) { if (errno == ENOENT) { LOG(INFO) << "/dev/hw_random not found"; // It's not an error to not have a Hardware RNG. - return 0; + return Success(); } - PLOG(ERROR) << "Failed to open /dev/hw_random"; - return -1; + return ErrnoError() << "Failed to open /dev/hw_random"; } unique_fd urandom_fd( TEMP_FAILURE_RETRY(open("/dev/urandom", O_WRONLY | O_NOFOLLOW | O_CLOEXEC))); if (urandom_fd == -1) { - PLOG(ERROR) << "Failed to open /dev/urandom"; - return -1; + return ErrnoError() << "Failed to open /dev/urandom"; } char buf[512]; @@ -71,23 +69,20 @@ int MixHwrngIntoLinuxRngAction(const std::vector& args) { ssize_t chunk_size = TEMP_FAILURE_RETRY(read(hwrandom_fd, buf, sizeof(buf) - total_bytes_written)); if (chunk_size == -1) { - PLOG(ERROR) << "Failed to read from /dev/hw_random"; - return -1; + return ErrnoError() << "Failed to read from /dev/hw_random"; } else if (chunk_size == 0) { - LOG(ERROR) << "Failed to read from /dev/hw_random: EOF"; - return -1; + return Error() << "Failed to read from /dev/hw_random: EOF"; } chunk_size = TEMP_FAILURE_RETRY(write(urandom_fd, buf, chunk_size)); if (chunk_size == -1) { - PLOG(ERROR) << "Failed to write to /dev/urandom"; - return -1; + return ErrnoError() << "Failed to write to /dev/urandom"; } total_bytes_written += chunk_size; } LOG(INFO) << "Mixed " << total_bytes_written << " bytes from /dev/hw_random into /dev/urandom"; - return 0; + return Success(); } static bool SetHighestAvailableOptionValue(std::string path, int min, int max) { @@ -154,38 +149,38 @@ static bool __attribute__((unused)) SetMmapRndBitsMin(int start, int min, bool c // 9e08f57d684a x86: mm: support ARCH_MMAP_RND_BITS // ec9ee4acd97c drivers: char: random: add get_random_long() // 5ef11c35ce86 mm: ASLR: use get_random_long() -int SetMmapRndBitsAction(const std::vector& args) { +Result SetMmapRndBitsAction(const std::vector& args) { // values are arch-dependent #if defined(USER_MODE_LINUX) // uml does not support mmap_rnd_bits - return 0; + return Success(); #elif defined(__aarch64__) // arm64 supports 18 - 33 bits depending on pagesize and VA_SIZE if (SetMmapRndBitsMin(33, 24, false) && SetMmapRndBitsMin(16, 16, true)) { - return 0; + return Success(); } #elif defined(__x86_64__) // x86_64 supports 28 - 32 bits if (SetMmapRndBitsMin(32, 32, false) && SetMmapRndBitsMin(16, 16, true)) { - return 0; + return Success(); } #elif defined(__arm__) || defined(__i386__) // check to see if we're running on 64-bit kernel bool h64 = !access(MMAP_RND_COMPAT_PATH, F_OK); // supported 32-bit architecture must have 16 bits set if (SetMmapRndBitsMin(16, 16, h64)) { - return 0; + return Success(); } #elif defined(__mips__) || defined(__mips64__) // TODO: add mips support b/27788820 - return 0; + return Success(); #else LOG(ERROR) << "Unknown architecture"; #endif LOG(ERROR) << "Unable to set adequate mmap entropy value!"; panic(); - return -1; + return Error(); } #define KPTR_RESTRICT_PATH "/proc/sys/kernel/kptr_restrict" @@ -195,14 +190,15 @@ int SetMmapRndBitsAction(const std::vector& args) { // Set kptr_restrict to the highest available level. // // Aborts if unable to set this to an acceptable value. -int SetKptrRestrictAction(const std::vector& args) { +Result SetKptrRestrictAction(const std::vector& args) { std::string path = KPTR_RESTRICT_PATH; if (!SetHighestAvailableOptionValue(path, KPTR_RESTRICT_MINVALUE, KPTR_RESTRICT_MAXVALUE)) { LOG(ERROR) << "Unable to set adequate kptr_restrict value!"; panic(); + return Error(); } - return 0; + return Success(); } } // namespace init diff --git a/init/security.h b/init/security.h index c489de143..31e5790f0 100644 --- a/init/security.h +++ b/init/security.h @@ -20,12 +20,14 @@ #include #include +#include "result.h" + namespace android { namespace init { -int MixHwrngIntoLinuxRngAction(const std::vector& args); -int SetMmapRndBitsAction(const std::vector& args); -int SetKptrRestrictAction(const std::vector& args); +Result MixHwrngIntoLinuxRngAction(const std::vector& args); +Result SetMmapRndBitsAction(const std::vector& args); +Result SetKptrRestrictAction(const std::vector& args); } // namespace init } // namespace android diff --git a/init/selinux.cpp b/init/selinux.cpp index 3a4a71548..b9305ed47 100644 --- a/init/selinux.cpp +++ b/init/selinux.cpp @@ -339,9 +339,8 @@ void SelinuxInitialize() { } } - std::string err; - if (!WriteFile("/sys/fs/selinux/checkreqprot", "0", &err)) { - LOG(ERROR) << err; + if (auto result = WriteFile("/sys/fs/selinux/checkreqprot", "0"); !result) { + LOG(ERROR) << "Unable to write to /sys/fs/selinux/checkreqprot: " << result.error(); panic(); } diff --git a/init/service.cpp b/init/service.cpp index 6f756fa6b..1b5cc19e4 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -333,12 +333,12 @@ void Service::DumpState() const { [] (const auto& info) { LOG(INFO) << *info; }); } -bool Service::ParseCapabilities(const std::vector& args, std::string* err) { +Result Service::ParseCapabilities(const std::vector& args) { capabilities_ = 0; if (!CapAmbientSupported()) { - *err = "capabilities requested but the kernel does not support ambient capabilities"; - return false; + return Error() + << "capabilities requested but the kernel does not support ambient capabilities"; } unsigned int last_valid_cap = GetLastValidCap(); @@ -350,74 +350,71 @@ bool Service::ParseCapabilities(const std::vector& args, std::strin const std::string& arg = args[i]; int res = LookupCap(arg); if (res < 0) { - *err = StringPrintf("invalid capability '%s'", arg.c_str()); - return false; + return Error() << StringPrintf("invalid capability '%s'", arg.c_str()); } unsigned int cap = static_cast(res); // |res| is >= 0. if (cap > last_valid_cap) { - *err = StringPrintf("capability '%s' not supported by the kernel", arg.c_str()); - return false; + return Error() << StringPrintf("capability '%s' not supported by the kernel", + arg.c_str()); } capabilities_[cap] = true; } - return true; + return Success(); } -bool Service::ParseClass(const std::vector& args, std::string* err) { +Result Service::ParseClass(const std::vector& args) { classnames_ = std::set(args.begin() + 1, args.end()); - return true; + return Success(); } -bool Service::ParseConsole(const std::vector& args, std::string* err) { +Result Service::ParseConsole(const std::vector& args) { flags_ |= SVC_CONSOLE; console_ = args.size() > 1 ? "/dev/" + args[1] : ""; - return true; + return Success(); } -bool Service::ParseCritical(const std::vector& args, std::string* err) { +Result Service::ParseCritical(const std::vector& args) { flags_ |= SVC_CRITICAL; - return true; + return Success(); } -bool Service::ParseDisabled(const std::vector& args, std::string* err) { +Result Service::ParseDisabled(const std::vector& args) { flags_ |= SVC_DISABLED; flags_ |= SVC_RC_DISABLED; - return true; + return Success(); } -bool Service::ParseGroup(const std::vector& args, std::string* err) { - std::string decode_uid_err; - if (!DecodeUid(args[1], &gid_, &decode_uid_err)) { - *err = "Unable to find GID for '" + args[1] + "': " + decode_uid_err; - return false; +Result Service::ParseGroup(const std::vector& args) { + auto gid = DecodeUid(args[1]); + if (!gid) { + return Error() << "Unable to decode GID for '" << args[1] << "': " << gid.error(); } + gid_ = *gid; + for (std::size_t n = 2; n < args.size(); n++) { - gid_t gid; - if (!DecodeUid(args[n], &gid, &decode_uid_err)) { - *err = "Unable to find GID for '" + args[n] + "': " + decode_uid_err; - return false; + gid = DecodeUid(args[n]); + if (!gid) { + return Error() << "Unable to decode GID for '" << args[n] << "': " << gid.error(); } - supp_gids_.emplace_back(gid); + supp_gids_.emplace_back(*gid); } - return true; + return Success(); } -bool Service::ParsePriority(const std::vector& args, std::string* err) { +Result Service::ParsePriority(const std::vector& args) { priority_ = 0; if (!ParseInt(args[1], &priority_, static_cast(ANDROID_PRIORITY_HIGHEST), // highest is negative static_cast(ANDROID_PRIORITY_LOWEST))) { - *err = StringPrintf("process priority value must be range %d - %d", - ANDROID_PRIORITY_HIGHEST, ANDROID_PRIORITY_LOWEST); - return false; + return Error() << StringPrintf("process priority value must be range %d - %d", + ANDROID_PRIORITY_HIGHEST, ANDROID_PRIORITY_LOWEST); } - return true; + return Success(); } -bool Service::ParseIoprio(const std::vector& args, std::string* err) { +Result Service::ParseIoprio(const std::vector& args) { if (!ParseInt(args[2], &ioprio_pri_, 0, 7)) { - *err = "priority value must be range 0 - 7"; - return false; + return Error() << "priority value must be range 0 - 7"; } if (args[1] == "rt") { @@ -427,14 +424,13 @@ bool Service::ParseIoprio(const std::vector& args, std::string* err } else if (args[1] == "idle") { ioprio_class_ = IoSchedClass_IDLE; } else { - *err = "ioprio option usage: ioprio <0-7>"; - return false; + return Error() << "ioprio option usage: ioprio <0-7>"; } - return true; + return Success(); } -bool Service::ParseKeycodes(const std::vector& args, std::string* err) { +Result Service::ParseKeycodes(const std::vector& args) { for (std::size_t i = 1; i < args.size(); i++) { int code; if (ParseInt(args[i], &code)) { @@ -443,22 +439,24 @@ bool Service::ParseKeycodes(const std::vector& args, std::string* e LOG(WARNING) << "ignoring invalid keycode: " << args[i]; } } - return true; + return Success(); } -bool Service::ParseOneshot(const std::vector& args, std::string* err) { +Result Service::ParseOneshot(const std::vector& args) { flags_ |= SVC_ONESHOT; - return true; + return Success(); } -bool Service::ParseOnrestart(const std::vector& args, std::string* err) { +Result Service::ParseOnrestart(const std::vector& args) { std::vector str_args(args.begin() + 1, args.end()); int line = onrestart_.NumCommands() + 1; - onrestart_.AddCommand(str_args, line, err); - return true; + if (auto result = onrestart_.AddCommand(str_args, line); !result) { + return Error() << "cannot add Onrestart command: " << result.error(); + } + return Success(); } -bool Service::ParseNamespace(const std::vector& args, std::string* err) { +Result Service::ParseNamespace(const std::vector& args) { for (size_t i = 1; i < args.size(); i++) { if (args[i] == "pid") { namespace_flags_ |= CLONE_NEWPID; @@ -467,135 +465,125 @@ bool Service::ParseNamespace(const std::vector& args, std::string* } else if (args[i] == "mnt") { namespace_flags_ |= CLONE_NEWNS; } else { - *err = "namespace must be 'pid' or 'mnt'"; - return false; + return Error() << "namespace must be 'pid' or 'mnt'"; } } - return true; + return Success(); } -bool Service::ParseOomScoreAdjust(const std::vector& args, std::string* err) { +Result Service::ParseOomScoreAdjust(const std::vector& args) { if (!ParseInt(args[1], &oom_score_adjust_, -1000, 1000)) { - *err = "oom_score_adjust value must be in range -1000 - +1000"; - return false; + return Error() << "oom_score_adjust value must be in range -1000 - +1000"; } - return true; + return Success(); } -bool Service::ParseMemcgSwappiness(const std::vector& args, std::string* err) { +Result Service::ParseMemcgSwappiness(const std::vector& args) { if (!ParseInt(args[1], &swappiness_, 0)) { - *err = "swappiness value must be equal or greater than 0"; - return false; + return Error() << "swappiness value must be equal or greater than 0"; } - return true; + return Success(); } -bool Service::ParseMemcgLimitInBytes(const std::vector& args, std::string* err) { +Result Service::ParseMemcgLimitInBytes(const std::vector& args) { if (!ParseInt(args[1], &limit_in_bytes_, 0)) { - *err = "limit_in_bytes value must be equal or greater than 0"; - return false; + return Error() << "limit_in_bytes value must be equal or greater than 0"; } - return true; + return Success(); } -bool Service::ParseMemcgSoftLimitInBytes(const std::vector& args, std::string* err) { +Result Service::ParseMemcgSoftLimitInBytes(const std::vector& args) { if (!ParseInt(args[1], &soft_limit_in_bytes_, 0)) { - *err = "soft_limit_in_bytes value must be equal or greater than 0"; - return false; + return Error() << "soft_limit_in_bytes value must be equal or greater than 0"; } - return true; + return Success(); } -bool Service::ParseSeclabel(const std::vector& args, std::string* err) { +Result Service::ParseSeclabel(const std::vector& args) { seclabel_ = args[1]; - return true; + return Success(); } -bool Service::ParseSetenv(const std::vector& args, std::string* err) { +Result Service::ParseSetenv(const std::vector& args) { envvars_.emplace_back(args[1], args[2]); - return true; + return Success(); } -bool Service::ParseShutdown(const std::vector& args, std::string* err) { +Result Service::ParseShutdown(const std::vector& args) { if (args[1] == "critical") { flags_ |= SVC_SHUTDOWN_CRITICAL; - return true; + return Success(); } - return false; + return Error() << "Invalid shutdown option"; } template -bool Service::AddDescriptor(const std::vector& args, std::string* err) { +Result Service::AddDescriptor(const std::vector& args) { int perm = args.size() > 3 ? std::strtoul(args[3].c_str(), 0, 8) : -1; - uid_t uid = 0; - gid_t gid = 0; + Result uid = 0; + Result gid = 0; std::string context = args.size() > 6 ? args[6] : ""; - std::string decode_uid_err; if (args.size() > 4) { - if (!DecodeUid(args[4], &uid, &decode_uid_err)) { - *err = "Unable to find UID for '" + args[4] + "': " + decode_uid_err; - return false; + uid = DecodeUid(args[4]); + if (!uid) { + return Error() << "Unable to find UID for '" << args[4] << "': " << uid.error(); } } if (args.size() > 5) { - if (!DecodeUid(args[5], &gid, &decode_uid_err)) { - *err = "Unable to find GID for '" + args[5] + "': " + decode_uid_err; - return false; + gid = DecodeUid(args[5]); + if (!gid) { + return Error() << "Unable to find GID for '" << args[5] << "': " << gid.error(); } } - auto descriptor = std::make_unique(args[1], args[2], uid, gid, perm, context); + auto descriptor = std::make_unique(args[1], args[2], *uid, *gid, perm, context); auto old = std::find_if(descriptors_.begin(), descriptors_.end(), [&descriptor] (const auto& other) { return descriptor.get() == other.get(); }); if (old != descriptors_.end()) { - *err = "duplicate descriptor " + args[1] + " " + args[2]; - return false; + return Error() << "duplicate descriptor " << args[1] << " " << args[2]; } descriptors_.emplace_back(std::move(descriptor)); - return true; + return Success(); } // name type perm [ uid gid context ] -bool Service::ParseSocket(const std::vector& args, std::string* err) { +Result Service::ParseSocket(const std::vector& args) { if (!StartsWith(args[2], "dgram") && !StartsWith(args[2], "stream") && !StartsWith(args[2], "seqpacket")) { - *err = "socket type must be 'dgram', 'stream' or 'seqpacket'"; - return false; + return Error() << "socket type must be 'dgram', 'stream' or 'seqpacket'"; } - return AddDescriptor(args, err); + return AddDescriptor(args); } // name type perm [ uid gid context ] -bool Service::ParseFile(const std::vector& args, std::string* err) { +Result Service::ParseFile(const std::vector& args) { if (args[2] != "r" && args[2] != "w" && args[2] != "rw") { - *err = "file type must be 'r', 'w' or 'rw'"; - return false; + return Error() << "file type must be 'r', 'w' or 'rw'"; } if ((args[1][0] != '/') || (args[1].find("../") != std::string::npos)) { - *err = "file name must not be relative"; - return false; + return Error() << "file name must not be relative"; } - return AddDescriptor(args, err); + return AddDescriptor(args); } -bool Service::ParseUser(const std::vector& args, std::string* err) { - std::string decode_uid_err; - if (!DecodeUid(args[1], &uid_, &decode_uid_err)) { - *err = "Unable to find UID for '" + args[1] + "': " + decode_uid_err; - return false; +Result Service::ParseUser(const std::vector& args) { + auto uid = DecodeUid(args[1]); + if (!uid) { + return Error() << "Unable to find UID for '" << args[1] << "': " << uid.error(); } - return true; + uid_ = *uid; + return Success(); } -bool Service::ParseWritepid(const std::vector& args, std::string* err) { +Result Service::ParseWritepid(const std::vector& args) { writepid_files_.assign(args.begin() + 1, args.end()); - return true; + return Success(); } class Service::OptionParserMap : public KeywordMap { @@ -643,15 +631,13 @@ const Service::OptionParserMap::Map& Service::OptionParserMap::map() const { return option_parsers; } -bool Service::ParseLine(const std::vector& args, std::string* err) { +Result Service::ParseLine(const std::vector& args) { static const OptionParserMap parser_map; - auto parser = parser_map.FindFunction(args, err); + auto parser = parser_map.FindFunction(args); - if (!parser) { - return false; - } + if (!parser) return Error() << parser.error(); - return (this->*parser)(args, err); + return std::invoke(*parser, this, args); } bool Service::ExecStart() { @@ -979,34 +965,35 @@ std::unique_ptr Service::MakeTemporaryOneshotService(const std::vector< if (command_arg > 2 && args[1] != "-") { seclabel = args[1]; } - uid_t uid = 0; + Result uid = 0; if (command_arg > 3) { - std::string decode_uid_err; - if (!DecodeUid(args[2], &uid, &decode_uid_err)) { - LOG(ERROR) << "Unable to find UID for '" << args[2] << "': " << decode_uid_err; + uid = DecodeUid(args[2]); + if (!uid) { + LOG(ERROR) << "Unable to decode UID for '" << args[2] << "': " << uid.error(); return nullptr; } } - gid_t gid = 0; + Result gid = 0; std::vector supp_gids; if (command_arg > 4) { - std::string decode_uid_err; - if (!DecodeUid(args[3], &gid, &decode_uid_err)) { - LOG(ERROR) << "Unable to find GID for '" << args[3] << "': " << decode_uid_err; + gid = DecodeUid(args[3]); + if (!gid) { + LOG(ERROR) << "Unable to decode GID for '" << args[3] << "': " << gid.error(); return nullptr; } std::size_t nr_supp_gids = command_arg - 1 /* -- */ - 4 /* exec SECLABEL UID GID */; for (size_t i = 0; i < nr_supp_gids; ++i) { - gid_t supp_gid; - if (!DecodeUid(args[4 + i], &supp_gid, &decode_uid_err)) { - LOG(ERROR) << "Unable to find UID for '" << args[4 + i] << "': " << decode_uid_err; + auto supp_gid = DecodeUid(args[4 + i]); + if (!supp_gid) { + LOG(ERROR) << "Unable to decode GID for '" << args[4 + i] + << "': " << supp_gid.error(); return nullptr; } - supp_gids.push_back(supp_gid); + supp_gids.push_back(*supp_gid); } } - return std::make_unique(name, flags, uid, gid, supp_gids, no_capabilities, + return std::make_unique(name, flags, *uid, *gid, supp_gids, no_capabilities, namespace_flags, seclabel, str_args); } @@ -1039,32 +1026,29 @@ void ServiceList::DumpState() const { } } -bool ServiceParser::ParseSection(std::vector&& args, const std::string& filename, - int line, std::string* err) { +Result ServiceParser::ParseSection(std::vector&& args, + const std::string& filename, int line) { if (args.size() < 3) { - *err = "services must have a name and a program"; - return false; + return Error() << "services must have a name and a program"; } const std::string& name = args[1]; if (!IsValidName(name)) { - *err = StringPrintf("invalid service name '%s'", name.c_str()); - return false; + return Error() << "invalid service name '" << name << "'"; } Service* old_service = service_list_->FindService(name); if (old_service) { - *err = "ignored duplicate definition of service '" + name + "'"; - return false; + return Error() << "ignored duplicate definition of service '" << name << "'"; } std::vector str_args(args.begin() + 2, args.end()); service_ = std::make_unique(name, str_args); - return true; + return Success(); } -bool ServiceParser::ParseLineSection(std::vector&& args, int line, std::string* err) { - return service_ ? service_->ParseLine(std::move(args), err) : false; +Result ServiceParser::ParseLineSection(std::vector&& args, int line) { + return service_ ? service_->ParseLine(std::move(args)) : Success(); } void ServiceParser::EndSection() { diff --git a/init/service.h b/init/service.h index 6c143cb61..fe851299e 100644 --- a/init/service.h +++ b/init/service.h @@ -76,7 +76,7 @@ class Service { static std::unique_ptr MakeTemporaryOneshotService(const std::vector& args); bool IsRunning() { return (flags_ & SVC_RUNNING) != 0; } - bool ParseLine(const std::vector& args, std::string* err); + Result ParseLine(const std::vector& args); bool ExecStart(); bool Start(); bool StartIfNotDisabled(); @@ -119,8 +119,7 @@ class Service { const std::vector& args() const { return args_; } private: - using OptionParser = bool (Service::*) (const std::vector& args, - std::string* err); + using OptionParser = Result (Service::*)(const std::vector& args); class OptionParserMap; void NotifyStateChange(const std::string& new_state) const; @@ -130,32 +129,32 @@ class Service { void KillProcessGroup(int signal); void SetProcessAttributes(); - bool ParseCapabilities(const std::vector& args, std::string *err); - bool ParseClass(const std::vector& args, std::string* err); - bool ParseConsole(const std::vector& args, std::string* err); - bool ParseCritical(const std::vector& args, std::string* err); - bool ParseDisabled(const std::vector& args, std::string* err); - bool ParseGroup(const std::vector& args, std::string* err); - bool ParsePriority(const std::vector& args, std::string* err); - bool ParseIoprio(const std::vector& args, std::string* err); - bool ParseKeycodes(const std::vector& args, std::string* err); - bool ParseOneshot(const std::vector& args, std::string* err); - bool ParseOnrestart(const std::vector& args, std::string* err); - bool ParseOomScoreAdjust(const std::vector& args, std::string* err); - bool ParseMemcgLimitInBytes(const std::vector& args, std::string* err); - bool ParseMemcgSoftLimitInBytes(const std::vector& args, std::string* err); - bool ParseMemcgSwappiness(const std::vector& args, std::string* err); - bool ParseNamespace(const std::vector& args, std::string* err); - bool ParseSeclabel(const std::vector& args, std::string* err); - bool ParseSetenv(const std::vector& args, std::string* err); - bool ParseShutdown(const std::vector& args, std::string* err); - bool ParseSocket(const std::vector& args, std::string* err); - bool ParseFile(const std::vector& args, std::string* err); - bool ParseUser(const std::vector& args, std::string* err); - bool ParseWritepid(const std::vector& args, std::string* err); + Result ParseCapabilities(const std::vector& args); + Result ParseClass(const std::vector& args); + Result ParseConsole(const std::vector& args); + Result ParseCritical(const std::vector& args); + Result ParseDisabled(const std::vector& args); + Result ParseGroup(const std::vector& args); + Result ParsePriority(const std::vector& args); + Result ParseIoprio(const std::vector& args); + Result ParseKeycodes(const std::vector& args); + Result ParseOneshot(const std::vector& args); + Result ParseOnrestart(const std::vector& args); + Result ParseOomScoreAdjust(const std::vector& args); + Result ParseMemcgLimitInBytes(const std::vector& args); + Result ParseMemcgSoftLimitInBytes(const std::vector& args); + Result ParseMemcgSwappiness(const std::vector& args); + Result ParseNamespace(const std::vector& args); + Result ParseSeclabel(const std::vector& args); + Result ParseSetenv(const std::vector& args); + Result ParseShutdown(const std::vector& args); + Result ParseSocket(const std::vector& args); + Result ParseFile(const std::vector& args); + Result ParseUser(const std::vector& args); + Result ParseWritepid(const std::vector& args); template - bool AddDescriptor(const std::vector& args, std::string* err); + Result AddDescriptor(const std::vector& args); static unsigned long next_start_order_; static bool is_exec_service_running_; @@ -242,9 +241,9 @@ class ServiceList { class ServiceParser : public SectionParser { public: ServiceParser(ServiceList* service_list) : service_list_(service_list), service_(nullptr) {} - bool ParseSection(std::vector&& args, const std::string& filename, int line, - std::string* err) override; - bool ParseLineSection(std::vector&& args, int line, std::string* err) override; + Result ParseSection(std::vector&& args, const std::string& filename, + int line) override; + Result ParseLineSection(std::vector&& args, int line) override; void EndSection() override; private: diff --git a/init/service_test.cpp b/init/service_test.cpp index 62e46f4cd..98d876f51 100644 --- a/init/service_test.cpp +++ b/init/service_test.cpp @@ -132,29 +132,29 @@ static void Test_make_temporary_oneshot_service(bool dash_dash, bool seclabel, b ASSERT_EQ("", svc->seclabel()); } if (uid) { - uid_t decoded_uid; - std::string err; - ASSERT_TRUE(DecodeUid("log", &decoded_uid, &err)); - ASSERT_EQ(decoded_uid, svc->uid()); + auto decoded_uid = DecodeUid("log"); + ASSERT_TRUE(decoded_uid); + ASSERT_EQ(*decoded_uid, svc->uid()); } else { ASSERT_EQ(0U, svc->uid()); } if (gid) { - uid_t decoded_uid; - std::string err; - ASSERT_TRUE(DecodeUid("shell", &decoded_uid, &err)); - ASSERT_EQ(decoded_uid, svc->gid()); + auto decoded_uid = DecodeUid("shell"); + ASSERT_TRUE(decoded_uid); + ASSERT_EQ(*decoded_uid, svc->gid()); } else { ASSERT_EQ(0U, svc->gid()); } if (supplementary_gids) { ASSERT_EQ(2U, svc->supp_gids().size()); - uid_t decoded_uid; - std::string err; - ASSERT_TRUE(DecodeUid("system", &decoded_uid, &err)); - ASSERT_EQ(decoded_uid, svc->supp_gids()[0]); - ASSERT_TRUE(DecodeUid("adb", &decoded_uid, &err)); - ASSERT_EQ(decoded_uid, svc->supp_gids()[1]); + + auto decoded_uid = DecodeUid("system"); + ASSERT_TRUE(decoded_uid); + ASSERT_EQ(*decoded_uid, svc->supp_gids()[0]); + + decoded_uid = DecodeUid("adb"); + ASSERT_TRUE(decoded_uid); + ASSERT_EQ(*decoded_uid, svc->supp_gids()[1]); } else { ASSERT_EQ(0U, svc->supp_gids().size()); } diff --git a/init/ueventd.cpp b/init/ueventd.cpp index b71945acc..1435d82ef 100644 --- a/init/ueventd.cpp +++ b/init/ueventd.cpp @@ -223,10 +223,10 @@ DeviceHandler CreateDeviceHandler() { using namespace std::placeholders; std::vector sysfs_permissions; std::vector dev_permissions; - parser.AddSingleLineParser( - "/sys/", std::bind(ParsePermissionsLine, _1, _2, &sysfs_permissions, nullptr)); + parser.AddSingleLineParser("/sys/", + std::bind(ParsePermissionsLine, _1, &sysfs_permissions, nullptr)); parser.AddSingleLineParser("/dev/", - std::bind(ParsePermissionsLine, _1, _2, nullptr, &dev_permissions)); + std::bind(ParsePermissionsLine, _1, nullptr, &dev_permissions)); parser.ParseConfig("/ueventd.rc"); parser.ParseConfig("/vendor/ueventd.rc"); diff --git a/init/ueventd_parser.cpp b/init/ueventd_parser.cpp index 02e0d42e5..e831b8b63 100644 --- a/init/ueventd_parser.cpp +++ b/init/ueventd_parser.cpp @@ -24,18 +24,16 @@ namespace android { namespace init { -bool ParsePermissionsLine(std::vector&& args, std::string* err, - std::vector* out_sysfs_permissions, - std::vector* out_dev_permissions) { +Result ParsePermissionsLine(std::vector&& args, + std::vector* out_sysfs_permissions, + std::vector* out_dev_permissions) { bool is_sysfs = out_sysfs_permissions != nullptr; if (is_sysfs && args.size() != 5) { - *err = "/sys/ lines must have 5 entries"; - return false; + return Error() << "/sys/ lines must have 5 entries"; } if (!is_sysfs && args.size() != 4) { - *err = "/dev/ lines must have 4 entries"; - return false; + return Error() << "/dev/ lines must have 4 entries"; } auto it = args.begin(); @@ -49,23 +47,20 @@ bool ParsePermissionsLine(std::vector&& args, std::string* err, char* end_pointer = 0; mode_t perm = strtol(perm_string.c_str(), &end_pointer, 8); if (end_pointer == nullptr || *end_pointer != '\0') { - *err = "invalid mode '" + perm_string + "'"; - return false; + return Error() << "invalid mode '" << perm_string << "'"; } std::string& uid_string = *it++; passwd* pwd = getpwnam(uid_string.c_str()); if (!pwd) { - *err = "invalid uid '" + uid_string + "'"; - return false; + return Error() << "invalid uid '" << uid_string << "'"; } uid_t uid = pwd->pw_uid; std::string& gid_string = *it++; struct group* grp = getgrnam(gid_string.c_str()); if (!grp) { - *err = "invalid gid '" + gid_string + "'"; - return false; + return Error() << "invalid gid '" << gid_string << "'"; } gid_t gid = grp->gr_gid; @@ -74,53 +69,49 @@ bool ParsePermissionsLine(std::vector&& args, std::string* err, } else { out_dev_permissions->emplace_back(name, perm, uid, gid); } - return true; + return Success(); } -bool SubsystemParser::ParseSection(std::vector&& args, const std::string& filename, - int line, std::string* err) { +Result SubsystemParser::ParseSection(std::vector&& args, + const std::string& filename, int line) { if (args.size() != 2) { - *err = "subsystems must have exactly one name"; - return false; + return Error() << "subsystems must have exactly one name"; } if (std::find(subsystems_->begin(), subsystems_->end(), args[1]) != subsystems_->end()) { - *err = "ignoring duplicate subsystem entry"; - return false; + return Error() << "ignoring duplicate subsystem entry"; } subsystem_.name_ = args[1]; - return true; + return Success(); } -bool SubsystemParser::ParseDevName(std::vector&& args, std::string* err) { +Result SubsystemParser::ParseDevName(std::vector&& args) { if (args[1] == "uevent_devname") { subsystem_.devname_source_ = Subsystem::DevnameSource::DEVNAME_UEVENT_DEVNAME; - return true; + return Success(); } if (args[1] == "uevent_devpath") { subsystem_.devname_source_ = Subsystem::DevnameSource::DEVNAME_UEVENT_DEVPATH; - return true; + return Success(); } - *err = "invalid devname '" + args[1] + "'"; - return false; + return Error() << "invalid devname '" << args[1] << "'"; } -bool SubsystemParser::ParseDirName(std::vector&& args, std::string* err) { +Result SubsystemParser::ParseDirName(std::vector&& args) { if (args[1].front() != '/') { - *err = "dirname '" + args[1] + " ' does not start with '/'"; - return false; + return Error() << "dirname '" << args[1] << " ' does not start with '/'"; } subsystem_.dir_name_ = args[1]; - return true; + return Success(); } -bool SubsystemParser::ParseLineSection(std::vector&& args, int line, std::string* err) { - using OptionParser = - bool (SubsystemParser::*)(std::vector && args, std::string * err); +Result SubsystemParser::ParseLineSection(std::vector&& args, int line) { + using OptionParser = Result (SubsystemParser::*)(std::vector && args); + static class OptionParserMap : public KeywordMap { private: const Map& map() const override { @@ -134,13 +125,11 @@ bool SubsystemParser::ParseLineSection(std::vector&& args, int line } } parser_map; - auto parser = parser_map.FindFunction(args, err); + auto parser = parser_map.FindFunction(args); - if (!parser) { - return false; - } + if (!parser) return Error() << parser.error(); - return (this->*parser)(std::move(args), err); + return std::invoke(*parser, this, std::move(args)); } void SubsystemParser::EndSection() { diff --git a/init/ueventd_parser.h b/init/ueventd_parser.h index 51d83ef00..18d1027b4 100644 --- a/init/ueventd_parser.h +++ b/init/ueventd_parser.h @@ -29,22 +29,22 @@ namespace init { class SubsystemParser : public SectionParser { public: SubsystemParser(std::vector* subsystems) : subsystems_(subsystems) {} - bool ParseSection(std::vector&& args, const std::string& filename, int line, - std::string* err) override; - bool ParseLineSection(std::vector&& args, int line, std::string* err) override; + Result ParseSection(std::vector&& args, const std::string& filename, + int line) override; + Result ParseLineSection(std::vector&& args, int line) override; void EndSection() override; private: - bool ParseDevName(std::vector&& args, std::string* err); - bool ParseDirName(std::vector&& args, std::string* err); + Result ParseDevName(std::vector&& args); + Result ParseDirName(std::vector&& args); Subsystem subsystem_; std::vector* subsystems_; }; -bool ParsePermissionsLine(std::vector&& args, std::string* err, - std::vector* out_sysfs_permissions, - std::vector* out_dev_permissions); +Result ParsePermissionsLine(std::vector&& args, + std::vector* out_sysfs_permissions, + std::vector* out_dev_permissions); } // namespace init } // namespace android diff --git a/init/util.cpp b/init/util.cpp index e0379876d..fcf7ca882 100644 --- a/init/util.cpp +++ b/init/util.cpp @@ -57,30 +57,20 @@ namespace init { const std::string kDefaultAndroidDtDir("/proc/device-tree/firmware/android/"); // DecodeUid() - decodes and returns the given string, which can be either the -// numeric or name representation, into the integer uid or gid. Returns -// UINT_MAX on error. -bool DecodeUid(const std::string& name, uid_t* uid, std::string* err) { - *uid = UINT_MAX; - *err = ""; - +// numeric or name representation, into the integer uid or gid. +Result DecodeUid(const std::string& name) { if (isalpha(name[0])) { passwd* pwd = getpwnam(name.c_str()); - if (!pwd) { - *err = "getpwnam failed: "s + strerror(errno); - return false; - } - *uid = pwd->pw_uid; - return true; + if (!pwd) return ErrnoError() << "getpwnam failed"; + + return pwd->pw_uid; } errno = 0; uid_t result = static_cast(strtoul(name.c_str(), 0, 0)); - if (errno) { - *err = "strtoul failed: "s + strerror(errno); - return false; - } - *uid = result; - return true; + if (errno) return ErrnoError() << "strtoul failed"; + + return result; } /* @@ -164,50 +154,40 @@ out_unlink: return -1; } -bool ReadFile(const std::string& path, std::string* content, std::string* err) { - content->clear(); - *err = ""; - +Result ReadFile(const std::string& path) { android::base::unique_fd fd( TEMP_FAILURE_RETRY(open(path.c_str(), O_RDONLY | O_NOFOLLOW | O_CLOEXEC))); if (fd == -1) { - *err = "Unable to open '" + path + "': " + strerror(errno); - return false; + return ErrnoError() << "open() failed"; } // For security reasons, disallow world-writable // or group-writable files. struct stat sb; if (fstat(fd, &sb) == -1) { - *err = "fstat failed for '" + path + "': " + strerror(errno); - return false; + return ErrnoError() << "fstat failed()"; } if ((sb.st_mode & (S_IWGRP | S_IWOTH)) != 0) { - *err = "Skipping insecure file '" + path + "'"; - return false; + return Error() << "Skipping insecure file"; } - if (!android::base::ReadFdToString(fd, content)) { - *err = "Unable to read '" + path + "': " + strerror(errno); - return false; + std::string content; + if (!android::base::ReadFdToString(fd, &content)) { + return ErrnoError() << "Unable to read file contents"; } - return true; + return content; } -bool WriteFile(const std::string& path, const std::string& content, std::string* err) { - *err = ""; - +Result WriteFile(const std::string& path, const std::string& content) { android::base::unique_fd fd(TEMP_FAILURE_RETRY( open(path.c_str(), O_WRONLY | O_CREAT | O_NOFOLLOW | O_TRUNC | O_CLOEXEC, 0600))); if (fd == -1) { - *err = "Unable to open '" + path + "': " + strerror(errno); - return false; + return ErrnoError() << "open() failed"; } if (!android::base::WriteStringToFd(content, fd)) { - *err = "Unable to write to '" + path + "': " + strerror(errno); - return false; + return ErrnoError() << "Unable to write file contents"; } - return true; + return Success(); } bool mkdir_recursive(const std::string& path, mode_t mode) { diff --git a/init/util.h b/init/util.h index a81df478a..298aa1cb2 100644 --- a/init/util.h +++ b/init/util.h @@ -28,6 +28,8 @@ #include #include +#include "result.h" + #define COLDBOOT_DONE "/dev/.coldboot_done" using android::base::boot_clock; @@ -39,10 +41,10 @@ namespace init { int CreateSocket(const char* name, int type, bool passcred, mode_t perm, uid_t uid, gid_t gid, const char* socketcon); -bool ReadFile(const std::string& path, std::string* content, std::string* err); -bool WriteFile(const std::string& path, const std::string& content, std::string* err); +Result ReadFile(const std::string& path); +Result WriteFile(const std::string& path, const std::string& content); -bool DecodeUid(const std::string& name, uid_t* uid, std::string* err); +Result DecodeUid(const std::string& name); bool mkdir_recursive(const std::string& pathname, mode_t mode); int wait_for_file(const char *filename, std::chrono::nanoseconds timeout); diff --git a/init/util_test.cpp b/init/util_test.cpp index 5dd271cd0..007d10ee3 100644 --- a/init/util_test.cpp +++ b/init/util_test.cpp @@ -30,61 +30,51 @@ namespace android { namespace init { TEST(util, ReadFile_ENOENT) { - std::string s("hello"); - std::string err; errno = 0; - EXPECT_FALSE(ReadFile("/proc/does-not-exist", &s, &err)); - EXPECT_EQ("Unable to open '/proc/does-not-exist': No such file or directory", err); + auto file_contents = ReadFile("/proc/does-not-exist"); EXPECT_EQ(ENOENT, errno); - EXPECT_EQ("", s); // s was cleared. + ASSERT_FALSE(file_contents); + EXPECT_EQ("open() failed: No such file or directory", file_contents.error()); } TEST(util, ReadFileGroupWriteable) { std::string s("hello"); TemporaryFile tf; - std::string err; ASSERT_TRUE(tf.fd != -1); - EXPECT_TRUE(WriteFile(tf.path, s, &err)) << strerror(errno); - EXPECT_EQ("", err); + EXPECT_TRUE(WriteFile(tf.path, s)) << strerror(errno); EXPECT_NE(-1, fchmodat(AT_FDCWD, tf.path, 0620, AT_SYMLINK_NOFOLLOW)) << strerror(errno); - EXPECT_FALSE(ReadFile(tf.path, &s, &err)) << strerror(errno); - EXPECT_EQ("Skipping insecure file '"s + tf.path + "'", err); - EXPECT_EQ("", s); // s was cleared. + auto file_contents = ReadFile(tf.path); + ASSERT_FALSE(file_contents) << strerror(errno); + EXPECT_EQ("Skipping insecure file", file_contents.error()); } TEST(util, ReadFileWorldWiteable) { std::string s("hello"); TemporaryFile tf; - std::string err; ASSERT_TRUE(tf.fd != -1); - EXPECT_TRUE(WriteFile(tf.path, s, &err)) << strerror(errno); - EXPECT_EQ("", err); + EXPECT_TRUE(WriteFile(tf.path, s)) << strerror(errno); EXPECT_NE(-1, fchmodat(AT_FDCWD, tf.path, 0602, AT_SYMLINK_NOFOLLOW)) << strerror(errno); - EXPECT_FALSE(ReadFile(tf.path, &s, &err)) << strerror(errno); - EXPECT_EQ("Skipping insecure file '"s + tf.path + "'", err); - EXPECT_EQ("", s); // s was cleared. + auto file_contents = ReadFile(tf.path); + ASSERT_FALSE(file_contents) << strerror(errno); + EXPECT_EQ("Skipping insecure file", file_contents.error()); } TEST(util, ReadFileSymbolicLink) { - std::string s("hello"); errno = 0; // lrwxrwxrwx 1 root root 13 1970-01-01 00:00 charger -> /sbin/healthd - std::string err; - EXPECT_FALSE(ReadFile("/charger", &s, &err)); - EXPECT_EQ("Unable to open '/charger': Too many symbolic links encountered", err); + auto file_contents = ReadFile("/charger"); EXPECT_EQ(ELOOP, errno); - EXPECT_EQ("", s); // s was cleared. + ASSERT_FALSE(file_contents); + EXPECT_EQ("open() failed: Too many symbolic links encountered", file_contents.error()); } TEST(util, ReadFileSuccess) { - std::string s("hello"); - std::string err; - EXPECT_TRUE(ReadFile("/proc/version", &s, &err)); - EXPECT_EQ("", err); - EXPECT_GT(s.length(), 6U); - EXPECT_EQ('\n', s[s.length() - 1]); - s[5] = 0; - EXPECT_STREQ("Linux", s.c_str()); + auto file_contents = ReadFile("/proc/version"); + ASSERT_TRUE(file_contents); + EXPECT_GT(file_contents->length(), 6U); + EXPECT_EQ('\n', file_contents->at(file_contents->length() - 1)); + (*file_contents)[5] = 0; + EXPECT_STREQ("Linux", file_contents->c_str()); } TEST(util, WriteFileBinary) { @@ -95,29 +85,23 @@ TEST(util, WriteFileBinary) { ASSERT_EQ(10u, contents.size()); TemporaryFile tf; - std::string err; ASSERT_TRUE(tf.fd != -1); - EXPECT_TRUE(WriteFile(tf.path, contents, &err)) << strerror(errno); - EXPECT_EQ("", err); + EXPECT_TRUE(WriteFile(tf.path, contents)) << strerror(errno); - std::string read_back_contents; - EXPECT_TRUE(ReadFile(tf.path, &read_back_contents, &err)) << strerror(errno); - EXPECT_EQ("", err); - EXPECT_EQ(contents, read_back_contents); - EXPECT_EQ(10u, read_back_contents.size()); + auto read_back_contents = ReadFile(tf.path); + ASSERT_TRUE(read_back_contents) << strerror(errno); + EXPECT_EQ(contents, *read_back_contents); + EXPECT_EQ(10u, read_back_contents->size()); } TEST(util, WriteFileNotExist) { std::string s("hello"); - std::string s2("hello"); TemporaryDir test_dir; std::string path = android::base::StringPrintf("%s/does-not-exist", test_dir.path); - std::string err; - EXPECT_TRUE(WriteFile(path, s, &err)); - EXPECT_EQ("", err); - EXPECT_TRUE(ReadFile(path, &s2, &err)); - EXPECT_EQ("", err); - EXPECT_EQ(s, s2); + EXPECT_TRUE(WriteFile(path, s)); + auto file_contents = ReadFile(path); + ASSERT_TRUE(file_contents); + EXPECT_EQ(s, *file_contents); struct stat sb; int fd = open(path.c_str(), O_RDONLY | O_NOFOLLOW | O_CLOEXEC); EXPECT_NE(-1, fd); @@ -127,37 +111,30 @@ TEST(util, WriteFileNotExist) { } TEST(util, WriteFileExist) { - std::string s2(""); TemporaryFile tf; ASSERT_TRUE(tf.fd != -1); - std::string err; - EXPECT_TRUE(WriteFile(tf.path, "1hello1", &err)) << strerror(errno); - EXPECT_EQ("", err); - EXPECT_TRUE(ReadFile(tf.path, &s2, &err)); - EXPECT_EQ("", err); - EXPECT_STREQ("1hello1", s2.c_str()); - EXPECT_TRUE(WriteFile(tf.path, "2ll2", &err)); - EXPECT_EQ("", err); - EXPECT_TRUE(ReadFile(tf.path, &s2, &err)); - EXPECT_EQ("", err); - EXPECT_STREQ("2ll2", s2.c_str()); + EXPECT_TRUE(WriteFile(tf.path, "1hello1")) << strerror(errno); + auto file_contents = ReadFile(tf.path); + ASSERT_TRUE(file_contents); + EXPECT_EQ("1hello1", *file_contents); + EXPECT_TRUE(WriteFile(tf.path, "2ll2")); + file_contents = ReadFile(tf.path); + ASSERT_TRUE(file_contents); + EXPECT_EQ("2ll2", *file_contents); } TEST(util, DecodeUid) { - uid_t decoded_uid; - std::string err; + auto decoded_uid = DecodeUid("root"); + EXPECT_TRUE(decoded_uid); + EXPECT_EQ(0U, *decoded_uid); - EXPECT_TRUE(DecodeUid("root", &decoded_uid, &err)); - EXPECT_EQ("", err); - EXPECT_EQ(0U, decoded_uid); + decoded_uid = DecodeUid("toot"); + EXPECT_FALSE(decoded_uid); + EXPECT_EQ("getpwnam failed: No such file or directory", decoded_uid.error()); - EXPECT_FALSE(DecodeUid("toot", &decoded_uid, &err)); - EXPECT_EQ("getpwnam failed: No such file or directory", err); - EXPECT_EQ(UINT_MAX, decoded_uid); - - EXPECT_TRUE(DecodeUid("123", &decoded_uid, &err)); - EXPECT_EQ("", err); - EXPECT_EQ(123U, decoded_uid); + decoded_uid = DecodeUid("123"); + EXPECT_TRUE(decoded_uid); + EXPECT_EQ(123U, *decoded_uid); } TEST(util, is_dir) {