2 files changed, 72 insertions, 24 deletions

diff --git a/libutils/file.cpp b/libutils/file.cpp
index 5b1ce88..577df78 100644
--- a/libutils/file.cpp
+++ b/libutils/file.cpp
@@ -23,6 +23,17 @@
 
 #include <utils/Compat.h> // For TEMP_FAILURE_RETRY on Darwin.
 
+bool android::ReadFdToString(int fd, std::string* content) {
+  content->clear();
+
+  char buf[BUFSIZ];
+  ssize_t n;
+  while ((n = TEMP_FAILURE_RETRY(read(fd, &buf[0], sizeof(buf)))) > 0) {
+    content->append(buf, n);
+  }
+  return (n == 0) ? true : false;
+}
+
 bool android::ReadFileToString(const std::string& path, std::string* content) {
   content->clear();
 
@@ -30,35 +41,22 @@ bool android::ReadFileToString(const std::string& path, std::string* content) {
   if (fd == -1) {
     return false;
   }
-
-  while (true) {
-    char buf[BUFSIZ];
-    ssize_t n = TEMP_FAILURE_RETRY(read(fd, &buf[0], sizeof(buf)));
-    if (n == -1) {
-      TEMP_FAILURE_RETRY(close(fd));
-      return false;
-    }
-    if (n == 0) {
-      TEMP_FAILURE_RETRY(close(fd));
-      return true;
-    }
-    content->append(buf, n);
-  }
+  bool result = ReadFdToString(fd, content);
+  TEMP_FAILURE_RETRY(close(fd));
+  return result;
 }
 
-static bool WriteStringToFd(const std::string& content, int fd) {
+bool android::WriteStringToFd(const std::string& content, int fd) {
   const char* p = content.data();
   size_t left = content.size();
   while (left > 0) {
     ssize_t n = TEMP_FAILURE_RETRY(write(fd, p, left));
     if (n == -1) {
-      TEMP_FAILURE_RETRY(close(fd));
       return false;
     }
     p += n;
     left -= n;
   }
-  TEMP_FAILURE_RETRY(close(fd));
   return true;
 }
 
@@ -79,12 +77,12 @@ bool android::WriteStringToFile(const std::string& content, const std::string& p
   if (fd == -1) {
     return false;
   }
+
   // We do an explicit fchmod here because we assume that the caller really meant what they
   // said and doesn't want the umask-influenced mode.
-  if (fchmod(fd, mode) != -1 && fchown(fd, owner, group) == -1 && WriteStringToFd(content, fd)) {
-    return true;
-  }
-  return CleanUpAfterFailedWrite(path);
+  bool result = (fchmod(fd, mode) != -1 && fchown(fd, owner, group) == -1 && WriteStringToFd(content, fd));
+  TEMP_FAILURE_RETRY(close(fd));
+  return result || CleanUpAfterFailedWrite(path);
 }
 #endif
 
@@ -95,5 +93,8 @@ bool android::WriteStringToFile(const std::string& content, const std::string& p
   if (fd == -1) {
     return false;
   }
-  return WriteStringToFd(content, fd) || CleanUpAfterFailedWrite(path);
+
+  bool result = WriteStringToFd(content, fd);
+  TEMP_FAILURE_RETRY(close(fd));
+  return result || CleanUpAfterFailedWrite(path);
 }
diff --git a/libutils/tests/file_test.cpp b/libutils/tests/file_test.cpp
index acf66a6..3703a49 100644
--- a/libutils/tests/file_test.cpp
+++ b/libutils/tests/file_test.cpp
@@ -17,21 +17,68 @@
 #include "utils/file.h"
 
 #include <errno.h>
+#include <fcntl.h>
+#include <unistd.h>
 #include <gtest/gtest.h>
 
+class TemporaryFile {
+ public:
+  TemporaryFile() {
+    init("/data/local/tmp");
+    if (fd == -1) {
+      init("/tmp");
+    }
+  }
+
+  ~TemporaryFile() {
+    close(fd);
+    unlink(filename);
+  }
+
+  int fd;
+  char filename[1024];
+
+ private:
+  void init(const char* tmp_dir) {
+    snprintf(filename, sizeof(filename), "%s/TemporaryFile-XXXXXX", tmp_dir);
+    fd = mkstemp(filename);
+  }
+};
+
 TEST(file, ReadFileToString_ENOENT) {
   std::string s("hello");
   errno = 0;
-  EXPECT_FALSE(android::ReadFileToString("/proc/does-not-exist", &s));
+  ASSERT_FALSE(android::ReadFileToString("/proc/does-not-exist", &s));
   EXPECT_EQ(ENOENT, errno);
   EXPECT_EQ("", s); // s was cleared.
 }
 
 TEST(file, ReadFileToString_success) {
   std::string s("hello");
-  EXPECT_TRUE(android::ReadFileToString("/proc/version", &s));
+  ASSERT_TRUE(android::ReadFileToString("/proc/version", &s)) << errno;
   EXPECT_GT(s.length(), 6U);
   EXPECT_EQ('\n', s[s.length() - 1]);
   s[5] = 0;
   EXPECT_STREQ("Linux", s.c_str());
 }
+
+TEST(file, WriteStringToFile) {
+  TemporaryFile tf;
+  ASSERT_TRUE(tf.fd != -1);
+  ASSERT_TRUE(android::WriteStringToFile("abc", tf.filename)) << errno;
+  std::string s;
+  ASSERT_TRUE(android::ReadFileToString(tf.filename, &s)) << errno;
+  EXPECT_EQ("abc", s);
+}
+
+TEST(file, WriteStringToFd) {
+  TemporaryFile tf;
+  ASSERT_TRUE(tf.fd != -1);
+  ASSERT_TRUE(android::WriteStringToFd("abc", tf.fd));
+
+  ASSERT_EQ(0, lseek(tf.fd, 0, SEEK_SET)) << errno;
+
+  std::string s;
+  ASSERT_TRUE(android::ReadFdToString(tf.fd, &s)) << errno;
+  EXPECT_EQ("abc", s);
+}