From 53089aa25ca9707e22e45e862f794bfc958d302a Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Fri, 31 Mar 2017 15:47:33 -0700 Subject: [PATCH] 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 Test: Boot bullhead, run unit tests Change-Id: Idad60e4228ee2de741ab3ab6a4917065b5e63cd8 --- init/builtins.cpp | 8 +++----- init/init_parser.cpp | 2 +- init/util.cpp | 9 +++++---- init/util.h | 4 ++-- init/util_test.cpp | 18 ++++++++++++++++++ 5 files changed, 29 insertions(+), 12 deletions(-) diff --git a/init/builtins.cpp b/init/builtins.cpp index dc2bda644..4ad36e078 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -155,7 +155,7 @@ static int do_class_restart(const std::vector& args) { } static int do_domainname(const std::vector& 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& args) { @@ -179,7 +179,7 @@ static int do_export(const std::vector& args) { } static int do_hostname(const std::vector& 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& args) { @@ -696,9 +696,7 @@ static int do_verity_update_state(const std::vector& args) { } static int do_write(const std::vector& 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& args) { diff --git a/init/init_parser.cpp b/init/init_parser.cpp index 326ebf230..a192862df 100644 --- a/init/init_parser.cpp +++ b/init/init_parser.cpp @@ -96,7 +96,7 @@ bool Parser::ParseConfigFile(const std::string& path) { 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 3f8f244cc..d1e0b850d 100644 --- a/init/util.cpp +++ b/init/util.cpp @@ -163,10 +163,11 @@ out_unlink: 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; } @@ -186,9 +187,9 @@ bool read_file(const char* path, std::string* content) { 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 23509d3fe..1034c9b31 100644 --- a/init/util.h +++ b/init/util.h @@ -35,8 +35,8 @@ using namespace std::chrono_literals; 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); class Timer { public: diff --git a/init/util_test.cpp b/init/util_test.cpp index 24c75c42d..613499813 100644 --- a/init/util_test.cpp +++ b/init/util_test.cpp @@ -18,6 +18,7 @@ #include +#include #include TEST(util, read_file_ENOENT) { @@ -37,6 +38,23 @@ TEST(util, read_file_success) { 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"));