From 517e1f17cfec2143d4d10a64b1496a550acf3ea2 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Thu, 4 May 2017 17:40:33 -0700 Subject: [PATCH 1/2] init: Check DecodeUid() result and use error string Check the result of DecodeUid() and return failure when uids/gids are unable to be decoded. Also, use an error string instead of logging directly such that more context can be added when decoding fails. Bug: 38038887 Test: Boot bullhead Test: Init unit tests Change-Id: I84c11aa5a8041bf5d2f754ee9af748344b789b37 --- init/builtins.cpp | 39 +++++++++++++++++++-------- init/init_parser_test.cpp | 18 ++++++++++--- init/service.cpp | 57 +++++++++++++++++++++++++++++++++------ init/util.cpp | 48 +++++++++++++++------------------ init/util.h | 2 +- init/util_test.cpp | 19 ++++++++++--- 6 files changed, 128 insertions(+), 55 deletions(-) diff --git a/init/builtins.cpp b/init/builtins.cpp index 3dadfd7dc..848dfdbf9 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -215,11 +215,19 @@ static int do_mkdir(const std::vector& args) { } if (args.size() >= 4) { - uid_t uid = decode_uid(args[3].c_str()); + 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; + } gid_t gid = -1; if (args.size() == 5) { - gid = decode_uid(args[4].c_str()); + if (!DecodeUid(args[4], &gid, &decode_uid_err)) { + LOG(ERROR) << "Unable to find GID for '" << args[3] << "': " << decode_uid_err; + return -1; + } } if (lchown(args[1].c_str(), uid, gid) == -1) { @@ -655,17 +663,26 @@ static int do_copy(const std::vector& args) { } static int do_chown(const std::vector& args) { - /* GID is optional. */ - if (args.size() == 3) { - if (lchown(args[2].c_str(), decode_uid(args[1].c_str()), -1) == -1) - return -errno; - } else if (args.size() == 4) { - if (lchown(args[3].c_str(), decode_uid(args[1].c_str()), - decode_uid(args[2].c_str())) == -1) - return -errno; - } else { + 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; } + + // 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; + + 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; + } + } + + if (lchown(path.c_str(), uid, gid) == -1) return -errno; + return 0; } diff --git a/init/init_parser_test.cpp b/init/init_parser_test.cpp index d8fd2bab9..86d60d024 100644 --- a/init/init_parser_test.cpp +++ b/init/init_parser_test.cpp @@ -86,19 +86,29 @@ static void Test_make_exec_oneshot_service(bool dash_dash, bool seclabel, bool u ASSERT_EQ("", svc->seclabel()); } if (uid) { - ASSERT_EQ(decode_uid("log"), svc->uid()); + uid_t decoded_uid; + std::string err; + ASSERT_TRUE(DecodeUid("log", &decoded_uid, &err)); + ASSERT_EQ(decoded_uid, svc->uid()); } else { ASSERT_EQ(0U, svc->uid()); } if (gid) { - ASSERT_EQ(decode_uid("shell"), svc->gid()); + uid_t decoded_uid; + std::string err; + ASSERT_TRUE(DecodeUid("shell", &decoded_uid, &err)); + ASSERT_EQ(decoded_uid, svc->gid()); } else { ASSERT_EQ(0U, svc->gid()); } if (supplementary_gids) { ASSERT_EQ(2U, svc->supp_gids().size()); - ASSERT_EQ(decode_uid("system"), svc->supp_gids()[0]); - ASSERT_EQ(decode_uid("adb"), svc->supp_gids()[1]); + 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]); } else { ASSERT_EQ(0U, svc->supp_gids().size()); } diff --git a/init/service.cpp b/init/service.cpp index 3a9f62228..4d9edc445 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -380,9 +380,18 @@ bool Service::ParseDisabled(const std::vector& args, std::string* e } bool Service::ParseGroup(const std::vector& args, std::string* err) { - gid_ = decode_uid(args[1].c_str()); + 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; + } for (std::size_t n = 2; n < args.size(); n++) { - supp_gids_.emplace_back(decode_uid(args[n].c_str())); + gid_t gid; + if (!DecodeUid(args[n], &gid, &decode_uid_err)) { + *err = "Unable to find GID for '" + args[n] + "': " + decode_uid_err; + return false; + } + supp_gids_.emplace_back(gid); } return true; } @@ -480,10 +489,25 @@ bool Service::ParseSetenv(const std::vector& args, std::string* err template bool Service::AddDescriptor(const std::vector& args, std::string* err) { int perm = args.size() > 3 ? std::strtoul(args[3].c_str(), 0, 8) : -1; - uid_t uid = args.size() > 4 ? decode_uid(args[4].c_str()) : 0; - gid_t gid = args.size() > 5 ? decode_uid(args[5].c_str()) : 0; + uid_t uid = 0; + gid_t 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; + } + } + + 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; + } + } + auto descriptor = std::make_unique(args[1], args[2], uid, gid, perm, context); auto old = @@ -522,7 +546,11 @@ bool Service::ParseFile(const std::vector& args, std::string* err) } bool Service::ParseUser(const std::vector& args, std::string* err) { - uid_ = decode_uid(args[1].c_str()); + 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; + } return true; } @@ -936,15 +964,28 @@ Service* ServiceManager::MakeExecOneshotService(const std::vector& } uid_t uid = 0; if (command_arg > 3) { - uid = decode_uid(args[2].c_str()); + 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; + return nullptr; + } } gid_t gid = 0; std::vector supp_gids; if (command_arg > 4) { - gid = decode_uid(args[3].c_str()); + 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; + 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) { - supp_gids.push_back(decode_uid(args[4 + i].c_str())); + 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; + return nullptr; + } + supp_gids.push_back(supp_gid); } } diff --git a/init/util.cpp b/init/util.cpp index 87b8d9d81..4df0c25c9 100644 --- a/init/util.cpp +++ b/init/util.cpp @@ -48,39 +48,33 @@ #endif using android::base::boot_clock; +using namespace std::literals::string_literals; -static unsigned int do_decode_uid(const char *s) -{ - unsigned int v; +// 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 = ""; - if (!s || *s == '\0') - return UINT_MAX; - - if (isalpha(s[0])) { - struct passwd* pwd = getpwnam(s); - if (!pwd) - return UINT_MAX; - return pwd->pw_uid; + 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; } errno = 0; - v = (unsigned int) strtoul(s, 0, 0); - if (errno) - return UINT_MAX; - return v; -} - -/* - * decode_uid - 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. - */ -unsigned int decode_uid(const char *s) { - unsigned int v = do_decode_uid(s); - if (v == UINT_MAX) { - LOG(ERROR) << "decode_uid: Unable to find UID for '" << s << "'; returning UINT_MAX"; + uid_t result = static_cast(strtoul(name.c_str(), 0, 0)); + if (errno) { + *err = "strtoul failed: "s + strerror(errno); + return false; } - return v; + *uid = result; + return true; } /* diff --git a/init/util.h b/init/util.h index 55ebded57..4957f1550 100644 --- a/init/util.h +++ b/init/util.h @@ -61,7 +61,7 @@ class Timer { std::ostream& operator<<(std::ostream& os, const Timer& t); -unsigned int decode_uid(const char *s); +bool DecodeUid(const std::string& name, uid_t* uid, std::string* err); int mkdir_recursive(const std::string& pathname, mode_t mode, selabel_handle* sehandle); int wait_for_file(const char *filename, std::chrono::nanoseconds timeout); diff --git a/init/util_test.cpp b/init/util_test.cpp index ac23a326e..a24185b00 100644 --- a/init/util_test.cpp +++ b/init/util_test.cpp @@ -115,10 +115,21 @@ TEST(util, write_file_exist) { EXPECT_STREQ("2ll2", s2.c_str()); } -TEST(util, decode_uid) { - EXPECT_EQ(0U, decode_uid("root")); - EXPECT_EQ(UINT_MAX, decode_uid("toot")); - EXPECT_EQ(123U, decode_uid("123")); +TEST(util, DecodeUid) { + uid_t decoded_uid; + std::string err; + + EXPECT_TRUE(DecodeUid("root", &decoded_uid, &err)); + EXPECT_EQ("", err); + EXPECT_EQ(0U, decoded_uid); + + 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); } TEST(util, is_dir) { From 2cbbe9f7a35efdc94e8e34ef92eb6f70a85887fe Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Thu, 4 May 2017 18:17:33 -0700 Subject: [PATCH 2/2] init: do not log directly from read_file() and write_file() Their callers may be able to add more context, so use an error string to record the error. Bug: 38038887 Test: boot bullhead Test: Init unit tests Change-Id: I46690d1c66e00a4b15cadc6fd0d6b50e990388c3 --- init/builtins.cpp | 33 ++++++++++++--- init/init.cpp | 4 +- init/init_parser.cpp | 4 +- init/init_test.cpp | 7 +-- init/property_service.cpp | 5 ++- init/util.cpp | 28 +++++++----- init/util.h | 4 +- init/util_test.cpp | 89 +++++++++++++++++++++++++-------------- 8 files changed, 117 insertions(+), 57 deletions(-) diff --git a/init/builtins.cpp b/init/builtins.cpp index 848dfdbf9..27b72f96c 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -150,7 +150,12 @@ 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]) ? 0 : 1; + std::string err; + if (!WriteFile("/proc/sys/kernel/domainname", args[1], &err)) { + LOG(ERROR) << err; + return -1; + } + return 0; } static int do_enable(const std::vector& args) { @@ -174,7 +179,12 @@ 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]) ? 0 : 1; + std::string err; + if (!WriteFile("/proc/sys/kernel/hostname", args[1], &err)) { + LOG(ERROR) << err; + return -1; + } + return 0; } static int do_ifup(const std::vector& args) { @@ -651,15 +661,26 @@ static int do_verity_update_state(const std::vector& args) { } static int do_write(const std::vector& args) { - return write_file(args[1], args[2]) ? 0 : 1; + std::string err; + if (!WriteFile(args[1], args[2], &err)) { + LOG(ERROR) << err; + return -1; + } + return 0; } static int do_copy(const std::vector& args) { std::string data; - if (read_file(args[1], &data)) { - return write_file(args[2], data) ? 0 : 1; + std::string err; + if (!ReadFile(args[1], &data, &err)) { + LOG(ERROR) << err; + return -1; } - return 1; + if (!WriteFile(args[2], data, &err)) { + LOG(ERROR) << err; + return -1; + } + return 0; } static int do_chown(const std::vector& args) { diff --git a/init/init.cpp b/init/init.cpp index 52f6b0cec..b5f97e5c8 100644 --- a/init/init.cpp +++ b/init/init.cpp @@ -853,7 +853,9 @@ static void selinux_initialize(bool in_kernel_domain) { } } - if (!write_file("/sys/fs/selinux/checkreqprot", "0")) { + std::string err; + if (!WriteFile("/sys/fs/selinux/checkreqprot", "0", &err)) { + LOG(ERROR) << err; security_failure(); } diff --git a/init/init_parser.cpp b/init/init_parser.cpp index 620367a62..1b31cf223 100644 --- a/init/init_parser.cpp +++ b/init/init_parser.cpp @@ -110,7 +110,9 @@ bool Parser::ParseConfigFile(const std::string& path) { LOG(INFO) << "Parsing file " << path << "..."; Timer t; std::string data; - if (!read_file(path, &data)) { + std::string err; + if (!ReadFile(path, &data, &err)) { + LOG(ERROR) << err; return false; } diff --git a/init/init_test.cpp b/init/init_test.cpp index 3da14b5ab..7093ba980 100644 --- a/init/init_test.cpp +++ b/init/init_test.cpp @@ -152,10 +152,11 @@ TEST(init, EventTriggerOrderMultipleFiles) { "on boot\n" "execute 3"; // clang-format on - // write_file() ensures the right mode is set - ASSERT_TRUE(write_file(std::string(dir.path) + "/a.rc", dir_a_script)); + // 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(write_file(std::string(dir.path) + "/b.rc", "on boot\nexecute 5")); + ASSERT_TRUE(WriteFile(std::string(dir.path) + "/b.rc", "on boot\nexecute 5", &err)); // clang-format off std::string start_script = "import " + std::string(first_import.path) + "\n" diff --git a/init/property_service.cpp b/init/property_service.cpp index 6f40d1ce0..e01cd0b4b 100644 --- a/init/property_service.cpp +++ b/init/property_service.cpp @@ -510,8 +510,9 @@ static void load_properties(char *data, const char *filter) static void load_properties_from_file(const char* filename, const char* filter) { Timer t; std::string data; - if (!read_file(filename, &data)) { - PLOG(WARNING) << "Couldn't load properties from " << filename; + std::string err; + if (!ReadFile(filename, &data, &err)) { + PLOG(WARNING) << "Couldn't load property file: " << err; return; } data.push_back('\n'); diff --git a/init/util.cpp b/init/util.cpp index 4df0c25c9..e7f724b8f 100644 --- a/init/util.cpp +++ b/init/util.cpp @@ -151,12 +151,14 @@ out_unlink: return -1; } -bool read_file(const std::string& path, std::string* content) { +bool ReadFile(const std::string& path, std::string* content, std::string* err) { content->clear(); + *err = ""; 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; } @@ -164,29 +166,35 @@ bool read_file(const std::string& path, std::string* content) { // or group-writable files. struct stat sb; if (fstat(fd, &sb) == -1) { - PLOG(ERROR) << "fstat failed for '" << path << "'"; + *err = "fstat failed for '" + path + "': " + strerror(errno); return false; } if ((sb.st_mode & (S_IWGRP | S_IWOTH)) != 0) { - LOG(ERROR) << "skipping insecure file '" << path << "'"; + *err = "Skipping insecure file '" + path + "'"; return false; } - return android::base::ReadFdToString(fd, content); + if (!android::base::ReadFdToString(fd, content)) { + *err = "Unable to read '" + path + "': " + strerror(errno); + return false; + } + return true; } -bool write_file(const std::string& path, const std::string& content) { +bool WriteFile(const std::string& path, const std::string& content, std::string* err) { + *err = ""; + 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) { - PLOG(ERROR) << "write_file: Unable to open '" << path << "'"; + *err = "Unable to open '" + path + "': " + strerror(errno); return false; } - bool success = android::base::WriteStringToFd(content, fd); - if (!success) { - PLOG(ERROR) << "write_file: Unable to write to '" << path << "'"; + if (!android::base::WriteStringToFd(content, fd)) { + *err = "Unable to write to '" + path + "': " + strerror(errno); + return false; } - return success; + return true; } int mkdir_recursive(const std::string& path, mode_t mode, selabel_handle* sehandle) { diff --git a/init/util.h b/init/util.h index 4957f1550..4c33909f0 100644 --- a/init/util.h +++ b/init/util.h @@ -38,8 +38,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, selabel_handle* sehandle); -bool read_file(const std::string& path, std::string* content); -bool write_file(const std::string& path, const std::string& content); +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); class Timer { public: diff --git a/init/util_test.cpp b/init/util_test.cpp index a24185b00..4bb8a8378 100644 --- a/init/util_test.cpp +++ b/init/util_test.cpp @@ -24,53 +24,67 @@ #include #include -TEST(util, read_file_ENOENT) { - std::string s("hello"); - errno = 0; - EXPECT_FALSE(read_file("/proc/does-not-exist", &s)); - EXPECT_EQ(ENOENT, errno); - EXPECT_EQ("", s); // s was cleared. +using namespace std::literals::string_literals; + +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); + EXPECT_EQ(ENOENT, errno); + EXPECT_EQ("", s); // s was cleared. } -TEST(util, read_file_group_writeable) { +TEST(util, ReadFileGroupWriteable) { std::string s("hello"); TemporaryFile tf; + std::string err; ASSERT_TRUE(tf.fd != -1); - EXPECT_TRUE(write_file(tf.path, s)) << strerror(errno); + EXPECT_TRUE(WriteFile(tf.path, s, &err)) << strerror(errno); + EXPECT_EQ("", err); EXPECT_NE(-1, fchmodat(AT_FDCWD, tf.path, 0620, AT_SYMLINK_NOFOLLOW)) << strerror(errno); - EXPECT_FALSE(read_file(tf.path, &s)) << 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. } -TEST(util, read_file_world_writeable) { +TEST(util, ReadFileWorldWiteable) { std::string s("hello"); TemporaryFile tf; + std::string err; ASSERT_TRUE(tf.fd != -1); - EXPECT_TRUE(write_file(tf.path, s.c_str())) << strerror(errno); + EXPECT_TRUE(WriteFile(tf.path, s, &err)) << strerror(errno); + EXPECT_EQ("", err); EXPECT_NE(-1, fchmodat(AT_FDCWD, tf.path, 0602, AT_SYMLINK_NOFOLLOW)) << strerror(errno); - EXPECT_FALSE(read_file(tf.path, &s)) << 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. } -TEST(util, read_file_symbolic_link) { +TEST(util, ReadFileSymbolicLink) { std::string s("hello"); errno = 0; // lrwxrwxrwx 1 root root 13 1970-01-01 00:00 charger -> /sbin/healthd - EXPECT_FALSE(read_file("/charger", &s)); + std::string err; + EXPECT_FALSE(ReadFile("/charger", &s, &err)); + EXPECT_EQ("Unable to open '/charger': Too many symbolic links encountered", err); EXPECT_EQ(ELOOP, errno); EXPECT_EQ("", s); // s was cleared. } -TEST(util, read_file_success) { - std::string s("hello"); - EXPECT_TRUE(read_file("/proc/version", &s)); - EXPECT_GT(s.length(), 6U); - EXPECT_EQ('\n', s[s.length() - 1]); - s[5] = 0; - EXPECT_STREQ("Linux", s.c_str()); +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()); } -TEST(util, write_file_binary) { +TEST(util, WriteFileBinary) { std::string contents("abcd"); contents.push_back('\0'); contents.push_back('\0'); @@ -78,22 +92,28 @@ TEST(util, write_file_binary) { ASSERT_EQ(10u, contents.size()); TemporaryFile tf; + std::string err; ASSERT_TRUE(tf.fd != -1); - EXPECT_TRUE(write_file(tf.path, contents)) << strerror(errno); + EXPECT_TRUE(WriteFile(tf.path, contents, &err)) << strerror(errno); + EXPECT_EQ("", err); std::string read_back_contents; - EXPECT_TRUE(read_file(tf.path, &read_back_contents)) << strerror(errno); + 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()); } -TEST(util, write_file_not_exist) { +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); - EXPECT_TRUE(write_file(path, s)); - EXPECT_TRUE(read_file(path, &s2)); + 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); struct stat sb; int fd = open(path.c_str(), O_RDONLY | O_NOFOLLOW | O_CLOEXEC); @@ -103,15 +123,20 @@ TEST(util, write_file_not_exist) { EXPECT_EQ(0, unlink(path.c_str())); } -TEST(util, write_file_exist) { +TEST(util, WriteFileExist) { std::string s2(""); TemporaryFile tf; ASSERT_TRUE(tf.fd != -1); - EXPECT_TRUE(write_file(tf.path, "1hello1")) << strerror(errno); - EXPECT_TRUE(read_file(tf.path, &s2)); + 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(write_file(tf.path, "2ll2")); - EXPECT_TRUE(read_file(tf.path, &s2)); + 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()); }