Merge "Clean up the arg setup for exec(3)." am: f0c03e62a2 am: 1b94d3a35b
am: 96e1f45c5d
Change-Id: I757b93896adb730e5b4c44c976142cdd6de29c42
diff --git a/install.cpp b/install.cpp
index ce3d4f6..9d8943f 100644
--- a/install.cpp
+++ b/install.cpp
@@ -395,12 +395,8 @@
// 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 @@
if (pid == 0) {
umask(022);
close(pipefd[0]);
- execv(chr_args[0], const_cast<char**>(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 e988da0..6db0ca5 100644
--- a/roots.cpp
+++ b/roots.cpp
@@ -28,7 +28,6 @@
#include <sys/wait.h>
#include <unistd.h>
-#include <algorithm>
#include <iostream>
#include <string>
#include <vector>
@@ -44,6 +43,7 @@
#include <fs_mgr_dm_linear.h>
#include "otautil/mounts.h"
+#include "otautil/sysutil.h"
static Fstab fstab;
@@ -90,12 +90,8 @@
}
static int exec_cmd(const std::vector<std::string>& args) {
- CHECK_NE(static_cast<size_t>(0), args.size());
-
- std::vector<char*> argv(args.size());
- std::transform(args.cbegin(), args.cend(), argv.begin(),
- [](const std::string& arg) { return const_cast<char*>(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 ccde409..0e1028b 100644
--- a/updater/install.cpp
+++ b/updater/install.cpp
@@ -393,17 +393,20 @@
return StringValue(mount_point);
}
-static int exec_cmd(const char* path, char* const argv[]) {
+static int exec_cmd(const std::vector<std::string>& 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 @@
}
if (fs_type == "ext4") {
- const char* mke2fs_argv[] = { "/system/bin/mke2fs", "-t", "ext4", "-b", "4096",
- location.c_str(), nullptr, nullptr };
- std::string size_str;
+ std::vector<std::string> 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<char**>(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<char**>(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<char**>(f2fs_argv));
- if (status != 0) {
+ std::vector<std::string> 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<char**>(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 @@
return ErrorAbort(state, kArgsParsingFailure, "%s() Failed to parse the argument(s)", name);
}
- char* args2[argv.size() + 1];
- for (size_t i = 0; i < argv.size(); i++) {
- args2[i] = &args[i][0];
- }
- args2[argv.size()] = nullptr;
-
- LOG(INFO) << "about to run program [" << args2[0] << "] with " << argv.size() << " args";
+ auto exec_args = StringVectorToNullTerminatedArray(args);
+ LOG(INFO) << "about to run program [" << exec_args[0] << "] with " << argv.size() << " args";
pid_t child = fork();
if (child == 0) {
- execv(args2[0], args2);
+ execv(exec_args[0], exec_args.data());
PLOG(ERROR) << "run_program: execv failed";
_exit(EXIT_FAILURE);
}
@@ -909,20 +897,12 @@
return ErrorAbort(state, kArgsParsingFailure, "%s() could not read args", name);
}
- char* args2[argv.size() + 1];
- // Tune2fs expects the program name as its args[0]
- args2[0] = const_cast<char*>(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");