4 files changed, 119 insertions, 177 deletions

diff --git a/otautil/DirUtil.cpp b/otautil/DirUtil.cpp
index ad344dedf..fffc82219 100644
--- a/otautil/DirUtil.cpp
+++ b/otautil/DirUtil.cpp
@@ -16,147 +16,101 @@
 
 #include "DirUtil.h"
 
+#include <dirent.h>
+#include <errno.h>
 #include <stdlib.h>
-#include <string.h>
-#include <stdio.h>
-#include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/types.h>
 #include <unistd.h>
-#include <errno.h>
-#include <dirent.h>
-#include <limits.h>
 
 #include <string>
 
 #include <selinux/label.h>
 #include <selinux/selinux.h>
 
-typedef enum { DMISSING, DDIR, DILLEGAL } DirStatus;
-
-static DirStatus
-getPathDirStatus(const char *path)
-{
-    struct stat st;
-    int err;
+enum class DirStatus { DMISSING, DDIR, DILLEGAL };
 
-    err = stat(path, &st);
-    if (err == 0) {
-        /* Something's there; make sure it's a directory.
-         */
-        if (S_ISDIR(st.st_mode)) {
-            return DDIR;
-        }
-        errno = ENOTDIR;
-        return DILLEGAL;
-    } else if (errno != ENOENT) {
-        /* Something went wrong, or something in the path
-         * is bad.  Can't do anything in this situation.
-         */
-        return DILLEGAL;
+static DirStatus dir_status(const std::string& path) {
+  struct stat sb;
+  if (stat(path.c_str(), &sb) == 0) {
+    // Something's there; make sure it's a directory.
+    if (S_ISDIR(sb.st_mode)) {
+      return DirStatus::DDIR;
     }
-    return DMISSING;
+    errno = ENOTDIR;
+    return DirStatus::DILLEGAL;
+  } else if (errno != ENOENT) {
+    // Something went wrong, or something in the path is bad. Can't do anything in this situation.
+    return DirStatus::DILLEGAL;
+  }
+  return DirStatus::DMISSING;
 }
 
-int
-dirCreateHierarchy(const char *path, int mode,
-        const struct utimbuf *timestamp, bool stripFileName,
-        struct selabel_handle *sehnd)
-{
-    DirStatus ds;
-
-    /* Check for an empty string before we bother
-     * making any syscalls.
-     */
-    if (path[0] == '\0') {
-        errno = ENOENT;
-        return -1;
+int mkdir_recursively(const std::string& input_path, mode_t mode, bool strip_filename,
+                      const selabel_handle* sehnd) {
+  // Check for an empty string before we bother making any syscalls.
+  if (input_path.empty()) {
+    errno = ENOENT;
+    return -1;
+  }
+
+  // Allocate a path that we can modify; stick a slash on the end to make things easier.
+  std::string path = input_path;
+  if (strip_filename) {
+    // Strip everything after the last slash.
+    size_t pos = path.rfind('/');
+    if (pos == std::string::npos) {
+      errno = ENOENT;
+      return -1;
     }
-    // Allocate a path that we can modify; stick a slash on
-    // the end to make things easier.
-    std::string cpath = path;
-    if (stripFileName) {
-        // Strip everything after the last slash.
-        size_t pos = cpath.rfind('/');
-        if (pos == std::string::npos) {
-            errno = ENOENT;
-            return -1;
-        }
-        cpath.resize(pos + 1);
-    } else {
-        // Make sure that the path ends in a slash.
-        cpath.push_back('/');
+    path.resize(pos + 1);
+  } else {
+    // Make sure that the path ends in a slash.
+    path.push_back('/');
+  }
+
+  // See if it already exists.
+  DirStatus ds = dir_status(path);
+  if (ds == DirStatus::DDIR) {
+    return 0;
+  } else if (ds == DirStatus::DILLEGAL) {
+    return -1;
+  }
+
+  // Walk up the path from the root and make each level.
+  size_t prev_end = 0;
+  while (prev_end < path.size()) {
+    size_t next_end = path.find('/', prev_end + 1);
+    if (next_end == std::string::npos) {
+      break;
     }
-
-    /* See if it already exists.
-     */
-    ds = getPathDirStatus(cpath.c_str());
-    if (ds == DDIR) {
-        return 0;
-    } else if (ds == DILLEGAL) {
+    std::string dir_path = path.substr(0, next_end);
+    // Check this part of the path and make a new directory if necessary.
+    switch (dir_status(dir_path)) {
+      case DirStatus::DILLEGAL:
+        // Could happen if some other process/thread is messing with the filesystem.
         return -1;
-    }
-
-    /* Walk up the path from the root and make each level.
-     * If a directory already exists, no big deal.
-     */
-    const char *path_start = &cpath[0];
-    char *p = &cpath[0];
-    while (*p != '\0') {
-        /* Skip any slashes, watching out for the end of the string.
-         */
-        while (*p != '\0' && *p == '/') {
-            p++;
-        }
-        if (*p == '\0') {
-            break;
+      case DirStatus::DMISSING: {
+        char* secontext = nullptr;
+        if (sehnd) {
+          selabel_lookup(const_cast<selabel_handle*>(sehnd), &secontext, dir_path.c_str(), mode);
+          setfscreatecon(secontext);
         }
-
-        /* Find the end of the next path component.
-         * We know that we'll see a slash before the NUL,
-         * because we added it, above.
-         */
-        while (*p != '/') {
-            p++;
+        int err = mkdir(dir_path.c_str(), mode);
+        if (secontext) {
+          freecon(secontext);
+          setfscreatecon(nullptr);
         }
-        *p = '\0';
-
-        /* Check this part of the path and make a new directory
-         * if necessary.
-         */
-        ds = getPathDirStatus(path_start);
-        if (ds == DILLEGAL) {
-            /* Could happen if some other process/thread is
-             * messing with the filesystem.
-             */
-            return -1;
-        } else if (ds == DMISSING) {
-            int err;
-
-            char *secontext = NULL;
-
-            if (sehnd) {
-                selabel_lookup(sehnd, &secontext, path_start, mode);
-                setfscreatecon(secontext);
-            }
-
-            err = mkdir(path_start, mode);
-
-            if (secontext) {
-                freecon(secontext);
-                setfscreatecon(NULL);
-            }
-
-            if (err != 0) {
-                return -1;
-            }
-            if (timestamp != NULL && utime(path_start, timestamp)) {
-                return -1;
-            }
+        if (err != 0) {
+          return -1;
         }
-        // else, this directory already exists.
-
-        // Repair the path and continue.
-        *p = '/';
+        break;
+      }
+      default:
+        // Already exists.
+        break;
     }
-    return 0;
+    prev_end = next_end;
+  }
+  return 0;
 }
diff --git a/otautil/DirUtil.h b/otautil/DirUtil.h
index beecc1081..85d6c16d1 100644
--- a/otautil/DirUtil.h
+++ b/otautil/DirUtil.h
@@ -14,28 +14,26 @@
  * limitations under the License.
  */
 
-#ifndef MINZIP_DIRUTIL_H_
-#define MINZIP_DIRUTIL_H_
+#ifndef OTAUTIL_DIRUTIL_H_
+#define OTAUTIL_DIRUTIL_H_
 
-#include <utime.h>
+#include <sys/stat.h>  // mode_t
+
+#include <string>
 
 struct selabel_handle;
 
-/* Like "mkdir -p", try to guarantee that all directories
- * specified in path are present, creating as many directories
- * as necessary.  The specified mode is passed to all mkdir
- * calls;  no modifications are made to umask.
- *
- * If stripFileName is set, everything after the final '/'
- * is stripped before creating the directory hierarchy.
- *
- * If timestamp is non-NULL, new directories will be timestamped accordingly.
- *
- * Returns 0 on success; returns -1 (and sets errno) on failure
- * (usually if some element of path is not a directory).
- */
-int dirCreateHierarchy(const char *path, int mode,
-        const struct utimbuf *timestamp, bool stripFileName,
-        struct selabel_handle* sehnd);
+// Like "mkdir -p", try to guarantee that all directories specified in path are present, creating as
+// many directories as necessary. The specified mode is passed to all mkdir calls; no modifications
+// are made to umask.
+//
+// If strip_filename is set, everything after the final '/' is stripped before creating the
+// directory
+// hierarchy.
+//
+// Returns 0 on success; returns -1 (and sets errno) on failure (usually if some element of path is
+// not a directory).
+int mkdir_recursively(const std::string& path, mode_t mode, bool strip_filename,
+                      const struct selabel_handle* sehnd);
 
-#endif  // MINZIP_DIRUTIL_H_
+#endif  // OTAUTIL_DIRUTIL_H_
diff --git a/recovery.cpp b/recovery.cpp
index 8f3e9bdea..233e56246 100644
--- a/recovery.cpp
+++ b/recovery.cpp
@@ -178,19 +178,19 @@ struct selabel_handle* sehandle;
  *    7b. the user reboots (pulling the battery, etc) into the main system
  */
 
-// open a given path, mounting partitions as necessary
-FILE* fopen_path(const char *path, const char *mode) {
-    if (ensure_path_mounted(path) != 0) {
-        LOG(ERROR) << "Can't mount " << path;
-        return NULL;
-    }
-
-    // When writing, try to create the containing directory, if necessary.
-    // Use generous permissions, the system (init.rc) will reset them.
-    if (strchr("wa", mode[0])) dirCreateHierarchy(path, 0777, NULL, 1, sehandle);
+// Open a given path, mounting partitions as necessary.
+FILE* fopen_path(const char* path, const char* mode) {
+  if (ensure_path_mounted(path) != 0) {
+    LOG(ERROR) << "Can't mount " << path;
+    return nullptr;
+  }
 
-    FILE *fp = fopen(path, mode);
-    return fp;
+  // When writing, try to create the containing directory, if necessary. Use generous permissions,
+  // the system (init.rc) will reset them.
+  if (strchr("wa", mode[0])) {
+    mkdir_recursively(path, 0777, true, sehandle);
+  }
+  return fopen(path, mode);
 }
 
 // close a file, log an error if the error indicator is set
@@ -593,7 +593,7 @@ static bool erase_volume(const char* volume) {
   if (is_cache) {
     // Re-create the log dir and write back the log entries.
     if (ensure_path_mounted(CACHE_LOG_DIR) == 0 &&
-        dirCreateHierarchy(CACHE_LOG_DIR, 0777, nullptr, false, sehandle) == 0) {
+        mkdir_recursively(CACHE_LOG_DIR, 0777, false, sehandle) == 0) {
       for (const auto& log : log_files) {
         if (!android::base::WriteStringToFile(log.data, log.name, log.sb.st_mode, log.sb.st_uid,
                                               log.sb.st_gid)) {
diff --git a/tests/unit/dirutil_test.cpp b/tests/unit/dirutil_test.cpp
index e62032c68..7f85d13ea 100644
--- a/tests/unit/dirutil_test.cpp
+++ b/tests/unit/dirutil_test.cpp
@@ -26,23 +26,23 @@
 
 TEST(DirUtilTest, create_invalid) {
   // Requesting to create an empty dir is invalid.
-  ASSERT_EQ(-1, dirCreateHierarchy("", 0755, nullptr, false, nullptr));
+  ASSERT_EQ(-1, mkdir_recursively("", 0755, false, nullptr));
   ASSERT_EQ(ENOENT, errno);
 
   // Requesting to strip the name with no slash present.
-  ASSERT_EQ(-1, dirCreateHierarchy("abc", 0755, nullptr, true, nullptr));
+  ASSERT_EQ(-1, mkdir_recursively("abc", 0755, true, nullptr));
   ASSERT_EQ(ENOENT, errno);
 
   // Creating a dir that already exists.
   TemporaryDir td;
-  ASSERT_EQ(0, dirCreateHierarchy(td.path, 0755, nullptr, false, nullptr));
+  ASSERT_EQ(0, mkdir_recursively(td.path, 0755, false, nullptr));
 
   // "///" is a valid dir.
-  ASSERT_EQ(0, dirCreateHierarchy("///", 0755, nullptr, false, nullptr));
+  ASSERT_EQ(0, mkdir_recursively("///", 0755, false, nullptr));
 
   // Request to create a dir, but a file with the same name already exists.
   TemporaryFile tf;
-  ASSERT_EQ(-1, dirCreateHierarchy(tf.path, 0755, nullptr, false, nullptr));
+  ASSERT_EQ(-1, mkdir_recursively(tf.path, 0755, false, nullptr));
   ASSERT_EQ(ENOTDIR, errno);
 }
 
@@ -51,7 +51,7 @@ TEST(DirUtilTest, create_smoke) {
   std::string prefix(td.path);
   std::string path = prefix + "/a/b";
   constexpr mode_t mode = 0755;
-  ASSERT_EQ(0, dirCreateHierarchy(path.c_str(), mode, nullptr, false, nullptr));
+  ASSERT_EQ(0, mkdir_recursively(path, mode, false, nullptr));
 
   // Verify.
   struct stat sb;
@@ -69,7 +69,7 @@ TEST(DirUtilTest, create_strip_filename) {
   TemporaryDir td;
   std::string prefix(td.path);
   std::string path = prefix + "/a/b";
-  ASSERT_EQ(0, dirCreateHierarchy(path.c_str(), 0755, nullptr, true, nullptr));
+  ASSERT_EQ(0, mkdir_recursively(path, 0755, true, nullptr));
 
   // Verify that "../a" exists but not "../a/b".
   struct stat sb;
@@ -83,31 +83,21 @@ TEST(DirUtilTest, create_strip_filename) {
   ASSERT_EQ(0, rmdir((prefix + "/a").c_str()));
 }
 
-TEST(DirUtilTest, create_mode_and_timestamp) {
+TEST(DirUtilTest, create_mode) {
   TemporaryDir td;
   std::string prefix(td.path);
   std::string path = prefix + "/a/b";
-  // Set the timestamp to 8/1/2008.
-  constexpr struct utimbuf timestamp = { 1217592000, 1217592000 };
   constexpr mode_t mode = 0751;
-  ASSERT_EQ(0, dirCreateHierarchy(path.c_str(), mode, &timestamp, false, nullptr));
+  ASSERT_EQ(0, mkdir_recursively(path, mode, false, nullptr));
 
-  // Verify the mode and timestamp for "../a/b".
+  // Verify the mode for "../a/b".
   struct stat sb;
   ASSERT_EQ(0, stat(path.c_str(), &sb)) << strerror(errno);
   ASSERT_TRUE(S_ISDIR(sb.st_mode));
   constexpr mode_t mask = S_IRWXU | S_IRWXG | S_IRWXO;
   ASSERT_EQ(mode, sb.st_mode & mask);
 
-  timespec time;
-  time.tv_sec = 1217592000;
-  time.tv_nsec = 0;
-
-  ASSERT_EQ(time.tv_sec, static_cast<long>(sb.st_atime));
-  ASSERT_EQ(time.tv_sec, static_cast<long>(sb.st_mtime));
-
-  // Verify the mode for "../a". Note that the timestamp for intermediate directories (e.g. "../a")
-  // may not be 'timestamp' according to the current implementation.
+  // Verify the mode for "../a".
   ASSERT_EQ(0, stat((prefix + "/a").c_str(), &sb)) << strerror(errno);
   ASSERT_TRUE(S_ISDIR(sb.st_mode));
   ASSERT_EQ(mode, sb.st_mode & mask);