From 3d69f0df961b8a79d247ca91dc272c60e504b538 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Thu, 20 Dec 2018 09:44:06 -0800 Subject: Clean up the arg setup for exec(3). Test: Build and boot into recovery on marlin. Factory reset. Test: Build and install a non-A/B OTA that calls format. Change-Id: I72416e775e237fc15ca5eff1036175a9eef43b76 --- install.cpp | 10 ++---- roots.cpp | 10 ++---- updater/install.cpp | 88 +++++++++++++++++++++-------------------------------- 3 files changed, 40 insertions(+), 68 deletions(-) diff --git a/install.cpp b/install.cpp index ce3d4f6ad..9d8943f7d 100644 --- a/install.cpp +++ b/install.cpp @@ -395,12 +395,8 @@ static int try_update_binary(const std::string& package, ZipArchiveHandle zip, b // update attempt. // - // Convert the vector to a NULL-terminated char* array suitable for execv. - const char* chr_args[args.size() + 1]; - chr_args[args.size()] = nullptr; - for (size_t i = 0; i < args.size(); i++) { - chr_args[i] = args[i].c_str(); - } + // Convert the std::string vector to a NULL-terminated char* vector suitable for execv. + auto chr_args = StringVectorToNullTerminatedArray(args); pid_t pid = fork(); @@ -415,7 +411,7 @@ static int try_update_binary(const std::string& package, ZipArchiveHandle zip, b if (pid == 0) { umask(022); close(pipefd[0]); - execv(chr_args[0], const_cast(chr_args)); + execv(chr_args[0], chr_args.data()); // Bug: 34769056 // We shouldn't use LOG/PLOG in the forked process, since they may cause // the child process to hang. This deadlock results from an improperly diff --git a/roots.cpp b/roots.cpp index e988da0a8..6db0ca519 100644 --- a/roots.cpp +++ b/roots.cpp @@ -28,7 +28,6 @@ #include #include -#include #include #include #include @@ -44,6 +43,7 @@ #include #include "otautil/mounts.h" +#include "otautil/sysutil.h" static Fstab fstab; @@ -90,12 +90,8 @@ int ensure_path_unmounted(const std::string& path) { } static int exec_cmd(const std::vector& args) { - CHECK_NE(static_cast(0), args.size()); - - std::vector argv(args.size()); - std::transform(args.cbegin(), args.cend(), argv.begin(), - [](const std::string& arg) { return const_cast(arg.c_str()); }); - argv.push_back(nullptr); + CHECK(!args.empty()); + auto argv = StringVectorToNullTerminatedArray(args); pid_t child; if ((child = fork()) == 0) { diff --git a/updater/install.cpp b/updater/install.cpp index ccde409df..0e1028bd1 100644 --- a/updater/install.cpp +++ b/updater/install.cpp @@ -393,17 +393,20 @@ Value* UnmountFn(const char* name, State* state, const std::vector& args) { + CHECK(!args.empty()); + auto argv = StringVectorToNullTerminatedArray(args); + pid_t child; if ((child = vfork()) == 0) { - execv(path, argv); + execv(argv[0], argv.data()); _exit(EXIT_FAILURE); } int status; waitpid(child, &status, 0); if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) { - LOG(ERROR) << path << " failed with status " << WEXITSTATUS(status); + LOG(ERROR) << args[0] << " failed with status " << WEXITSTATUS(status); } return WEXITSTATUS(status); } @@ -453,62 +456,52 @@ Value* FormatFn(const char* name, State* state, const std::vector mke2fs_args = { + "/system/bin/mke2fs", "-t", "ext4", "-b", "4096", location + }; if (size != 0) { - size_str = std::to_string(size / 4096LL); - mke2fs_argv[6] = size_str.c_str(); + mke2fs_args.push_back(std::to_string(size / 4096LL)); } - int status = exec_cmd(mke2fs_argv[0], const_cast(mke2fs_argv)); - if (status != 0) { + if (auto status = exec_cmd(mke2fs_args); status != 0) { LOG(ERROR) << name << ": mke2fs failed (" << status << ") on " << location; return StringValue(""); } - const char* e2fsdroid_argv[] = { "/system/bin/e2fsdroid", "-e", "-a", mount_point.c_str(), - location.c_str(), nullptr }; - status = exec_cmd(e2fsdroid_argv[0], const_cast(e2fsdroid_argv)); - if (status != 0) { + if (auto status = exec_cmd({ "/system/bin/e2fsdroid", "-e", "-a", mount_point, location }); + status != 0) { LOG(ERROR) << name << ": e2fsdroid failed (" << status << ") on " << location; return StringValue(""); } return StringValue(location); - } else if (fs_type == "f2fs") { + } + + if (fs_type == "f2fs") { if (size < 0) { LOG(ERROR) << name << ": fs_size can't be negative for f2fs: " << fs_size; return StringValue(""); } - std::string num_sectors = std::to_string(size / 512); - - const char* f2fs_path = "/sbin/mkfs.f2fs"; - const char* f2fs_argv[] = { "mkfs.f2fs", - "-g", "android", - "-w", "512", - location.c_str(), - (size < 512) ? nullptr : num_sectors.c_str(), - nullptr }; - int status = exec_cmd(f2fs_path, const_cast(f2fs_argv)); - if (status != 0) { + std::vector f2fs_args = { + "/sbin/mkfs.f2fs", "-g", "android", "-w", "512", location + }; + if (size >= 512) { + f2fs_args.push_back(std::to_string(size / 512)); + } + if (auto status = exec_cmd(f2fs_args); status != 0) { LOG(ERROR) << name << ": mkfs.f2fs failed (" << status << ") on " << location; return StringValue(""); } - const char* sload_argv[] = { "/sbin/sload.f2fs", "-t", mount_point.c_str(), location.c_str(), - nullptr }; - status = exec_cmd(sload_argv[0], const_cast(sload_argv)); - if (status != 0) { + if (auto status = exec_cmd({ "/sbin/sload.f2fs", "-t", mount_point, location }); status != 0) { LOG(ERROR) << name << ": sload.f2fs failed (" << status << ") on " << location; return StringValue(""); } return StringValue(location); - } else { - LOG(ERROR) << name << ": unsupported fs_type \"" << fs_type << "\" partition_type \"" - << partition_type << "\""; } + LOG(ERROR) << name << ": unsupported fs_type \"" << fs_type << "\" partition_type \"" + << partition_type << "\""; return nullptr; } @@ -675,17 +668,12 @@ Value* RunProgramFn(const char* name, State* state, const std::vector(name); - if (args2[0] == nullptr) { - return nullptr; - } - for (size_t i = 0; i < argv.size(); ++i) { - args2[i + 1] = &args[i][0]; - } + // tune2fs expects the program name as its first arg. + args.insert(args.begin(), "tune2fs"); + auto tune2fs_args = StringVectorToNullTerminatedArray(args); - // tune2fs changes the file system parameters on an ext2 file system; it - // returns 0 on success. - int result = tune2fs_main(argv.size() + 1, args2); - if (result != 0) { + // tune2fs changes the filesystem parameters on an ext2 filesystem; it returns 0 on success. + if (auto result = tune2fs_main(tune2fs_args.size() - 1, tune2fs_args.data()); result != 0) { return ErrorAbort(state, kTune2FsFailure, "%s() returned error code %d", name, result); } return StringValue("t"); -- cgit v1.2.3