init: Use std::string for write_file()
The content parameter of write_file() previously took a char* that was
then converted to a std::string in WriteStringToFd(). One unfortunate
effect of this, is that it is impossible to write data that contains
'\0' within it, as the new string will only contain characters up
until the '\0'.
This changes write_file() to take an std::string, such that
std::string::size() is used to determine the length of the string,
allowing it to contain null characters.
Also change the path parameter of read_file() and write_file() for
consistency.
Lastly, add a test for handling strings with '\0' in them.
Bug: 36726045
Bug: 36576280
Test: Boot bullhead, run unit tests
Change-Id: Idad60e4228ee2de741ab3ab6a4917065b5e63cd8
(cherry picked from commit 53089aa25ca9707e22e45e862f794bfc958d302a)
diff --git a/init/builtins.cpp b/init/builtins.cpp
index 08a715c..2260358 100644
--- a/init/builtins.cpp
+++ b/init/builtins.cpp
@@ -155,7 +155,7 @@
}
static int do_domainname(const std::vector<std::string>& args) {
- return write_file("/proc/sys/kernel/domainname", args[1].c_str()) ? 0 : 1;
+ return write_file("/proc/sys/kernel/domainname", args[1]) ? 0 : 1;
}
static int do_enable(const std::vector<std::string>& args) {
@@ -179,7 +179,7 @@
}
static int do_hostname(const std::vector<std::string>& args) {
- return write_file("/proc/sys/kernel/hostname", args[1].c_str()) ? 0 : 1;
+ return write_file("/proc/sys/kernel/hostname", args[1]) ? 0 : 1;
}
static int do_ifup(const std::vector<std::string>& args) {
@@ -648,9 +648,7 @@
}
static int do_write(const std::vector<std::string>& args) {
- const char* path = args[1].c_str();
- const char* value = args[2].c_str();
- return write_file(path, value) ? 0 : 1;
+ return write_file(args[1], args[2]) ? 0 : 1;
}
static int do_copy(const std::vector<std::string>& args) {
diff --git a/init/init_parser.cpp b/init/init_parser.cpp
index 326ebf2..a192862 100644
--- a/init/init_parser.cpp
+++ b/init/init_parser.cpp
@@ -96,7 +96,7 @@
LOG(INFO) << "Parsing file " << path << "...";
Timer t;
std::string data;
- if (!read_file(path.c_str(), &data)) {
+ if (!read_file(path, &data)) {
return false;
}
diff --git a/init/util.cpp b/init/util.cpp
index 73d97ed..c06d51b 100644
--- a/init/util.cpp
+++ b/init/util.cpp
@@ -161,10 +161,11 @@
return -1;
}
-bool read_file(const char* path, std::string* content) {
+bool read_file(const std::string& path, std::string* content) {
content->clear();
- android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(path, O_RDONLY | O_NOFOLLOW | O_CLOEXEC)));
+ android::base::unique_fd fd(
+ TEMP_FAILURE_RETRY(open(path.c_str(), O_RDONLY | O_NOFOLLOW | O_CLOEXEC)));
if (fd == -1) {
return false;
}
@@ -184,9 +185,9 @@
return android::base::ReadFdToString(fd, content);
}
-bool write_file(const char* path, const char* content) {
+bool write_file(const std::string& path, const std::string& content) {
android::base::unique_fd fd(
- TEMP_FAILURE_RETRY(open(path, O_WRONLY | O_CREAT | O_NOFOLLOW | O_CLOEXEC, 0600)));
+ TEMP_FAILURE_RETRY(open(path.c_str(), O_WRONLY | O_CREAT | O_NOFOLLOW | O_CLOEXEC, 0600)));
if (fd == -1) {
PLOG(ERROR) << "write_file: Unable to open '" << path << "'";
return false;
diff --git a/init/util.h b/init/util.h
index 81c64d7..90ec6bb 100644
--- a/init/util.h
+++ b/init/util.h
@@ -32,8 +32,8 @@
int create_socket(const char *name, int type, mode_t perm,
uid_t uid, gid_t gid, const char *socketcon);
-bool read_file(const char* path, std::string* content);
-bool write_file(const char* path, const char* content);
+bool read_file(const std::string& path, std::string* content);
+bool write_file(const std::string& path, const std::string& content);
// A std::chrono clock based on CLOCK_BOOTTIME.
class boot_clock {
diff --git a/init/util_test.cpp b/init/util_test.cpp
index 24c75c4..6134998 100644
--- a/init/util_test.cpp
+++ b/init/util_test.cpp
@@ -18,6 +18,7 @@
#include <errno.h>
+#include <android-base/test_utils.h>
#include <gtest/gtest.h>
TEST(util, read_file_ENOENT) {
@@ -37,6 +38,23 @@
EXPECT_STREQ("Linux", s.c_str());
}
+TEST(util, write_file_binary) {
+ std::string contents("abcd");
+ contents.push_back('\0');
+ contents.push_back('\0');
+ contents.append("dcba");
+ ASSERT_EQ(10u, contents.size());
+
+ TemporaryFile tf;
+ ASSERT_TRUE(tf.fd != -1);
+ EXPECT_TRUE(write_file(tf.path, contents)) << strerror(errno);
+
+ std::string read_back_contents;
+ EXPECT_TRUE(read_file(tf.path, &read_back_contents)) << strerror(errno);
+ EXPECT_EQ(contents, read_back_contents);
+ EXPECT_EQ(10u, read_back_contents.size());
+}
+
TEST(util, decode_uid) {
EXPECT_EQ(0U, decode_uid("root"));
EXPECT_EQ(UINT_MAX, decode_uid("toot"));