util: add new function create_directories with unit tests (#21871)

* util::create_directories

* check bool ret

* don't assume mask

* rename with_umask to no_umask

* remove umask

* rebase master

* rebase master

* 0755

selfdrive/common/params.cc

@@ -41,27 +41,10 @@ int fsync_dir(const char* path) {
   return result;
 }
 
-int mkdir_p(std::string path) {
-  char * _path = (char *)path.c_str();
-
-  for (char *p = _path + 1; *p; p++) {
-    if (*p == '/') {
-      *p = '\0'; // Temporarily truncate
-      if (mkdir(_path, 0775) != 0) {
-        if (errno != EEXIST) return -1;
-      }
-      *p = '/';
-    }
-  }
-  if (mkdir(_path, 0775) != 0) {
-    if (errno != EEXIST) return -1;
-  }
-  return 0;
-}
 
 bool create_params_path(const std::string &param_path, const std::string &key_path) {
   // Make sure params path exists
-  if (!util::file_exists(param_path) && mkdir_p(param_path) != 0) {
+  if (!util::file_exists(param_path) && !util::create_directories(param_path, 0775)) {
     return false;
   }
 

selfdrive/common/tests/test_util.cc

@@ -92,3 +92,36 @@ TEST_CASE("util::read_files_in_dir") {
     REQUIRE(k == v);
   }
 }
+
+TEST_CASE("util::create_directories") {
+  system("rm /tmp/test_create_directories -rf");
+  std::string dir = "/tmp/test_create_directories/a/b/c/d/e/f";
+
+  auto check_dir_permissions = [](const std::string &dir, mode_t mode) -> bool {
+    struct stat st = {};
+    return stat(dir.c_str(), &st) == 0 && (st.st_mode & S_IFMT) == S_IFDIR && (st.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) == mode;
+  };
+
+  SECTION("create_directories") {
+    REQUIRE(util::create_directories(dir, 0755));
+    REQUIRE(check_dir_permissions(dir, 0755));
+  }
+  SECTION("dir already exists") {
+    REQUIRE(util::create_directories(dir, 0755));
+    REQUIRE(util::create_directories(dir, 0755));
+  }
+  SECTION("a file exists with the same name") {
+    REQUIRE(util::create_directories(dir, 0755));
+    int f = open((dir + "/file").c_str(), O_RDWR | O_CREAT);
+    REQUIRE(f != -1);
+    close(f);
+    REQUIRE(util::create_directories(dir + "/file", 0755) == false);
+    REQUIRE(util::create_directories(dir + "/file/1/2/3", 0755) == false);
+  }
+  SECTION("end with slashs") {
+    REQUIRE(util::create_directories(dir + "/", 0755));
+  }
+  SECTION("empty") {
+    REQUIRE(util::create_directories("", 0755) == false);
+  }
+}

selfdrive/common/util.cc

@@ -1,14 +1,15 @@
 #include "selfdrive/common/util.h"
 
 #include <sys/stat.h>
+#include <dirent.h>
 
 #include <cassert>
 #include <cerrno>
 #include <cstring>
 #include <dirent.h>
 #include <fstream>
-#include <sstream>
 #include <iomanip>
+#include <sstream>
 
 #ifdef __linux__
 #include <sys/prctl.h>
@@ -121,6 +122,37 @@ bool file_exists(const std::string& fn) {
   return stat(fn.c_str(), &st) != -1;
 }
 
+static bool createDirectory(std::string dir, mode_t mode) {
+  auto verify_dir = [](const std::string& dir) -> bool {
+    struct stat st = {};
+    return (stat(dir.c_str(), &st) == 0 && (st.st_mode & S_IFMT) == S_IFDIR);
+  };
+  // remove trailing /'s
+  while (dir.size() > 1 && dir.back() == '/') {
+    dir.pop_back();
+  }
+  // try to mkdir this directory
+  if (mkdir(dir.c_str(), mode) == 0) return true;
+  if (errno == EEXIST) return verify_dir(dir);
+  if (errno != ENOENT) return false;
+
+  // mkdir failed because the parent dir doesn't exist, so try to create it
+  size_t slash = dir.rfind('/');
+  if ((slash == std::string::npos || slash < 1) ||
+      !createDirectory(dir.substr(0, slash), mode)) {
+    return false;
+  }
+
+  // try again
+  if (mkdir(dir.c_str(), mode) == 0) return true;
+  return errno == EEXIST && verify_dir(dir);
+}
+
+bool create_directories(const std::string& dir, mode_t mode) {
+  if (dir.empty()) return false;
+  return createDirectory(dir, mode);
+}
+
 std::string getenv(const char* key, const char* default_val) {
   const char* val = ::getenv(key);
   return val ? val : default_val;

selfdrive/common/util.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <fcntl.h>
+#include <sys/stat.h>
 #include <unistd.h>
 
 #include <algorithm>
@@ -76,6 +77,7 @@ std::map<std::string, std::string> read_files_in_dir(const std::string& path);
 int write_file(const char* path, const void* data, size_t size, int flags = O_WRONLY, mode_t mode = 0664);
 std::string readlink(const std::string& path);
 bool file_exists(const std::string& fn);
+bool create_directories(const std::string &dir, mode_t mode);
 
 inline void sleep_for(const int milliseconds) {
   std::this_thread::sleep_for(std::chrono::milliseconds(milliseconds));

selfdrive/loggerd/bootlog.cc

@@ -40,8 +40,8 @@ int main(int argc, char** argv) {
   LOGW("bootlog to %s", path.c_str());
 
   // Open bootlog
-  int r = logger_mkpath((char*)path.c_str());
-  assert(r == 0);
+  bool r = util::create_directories(LOG_ROOT + "/boot/", 0775);
+  assert(r);
 
   BZFile bz_file(path.c_str());
 

selfdrive/loggerd/logger.cc

@@ -30,22 +30,6 @@ void append_property(const char* key, const char* value, void *cookie) {
   properties->push_back(std::make_pair(std::string(key), std::string(value)));
 }
 
-int logger_mkpath(char* file_path) {
-  assert(file_path && *file_path);
-  char* p;
-  for (p=strchr(file_path+1, '/'); p; p=strchr(p+1, '/')) {
-    *p = '\0';
-    if (mkdir(file_path, 0775)==-1) {
-      if (errno != EEXIST) {
-        *p = '/';
-        return -1;
-      }
-    }
-    *p = '/';
-  }
-  return 0;
-}
-
 // ***** log metadata *****
 kj::Array<capnp::word> logger_build_init_data() {
   MessageBuilder msg;
@@ -154,8 +138,6 @@ void logger_init(LoggerState *s, const char* log_name, bool has_qlog) {
 }
 
 static LoggerHandle* logger_open(LoggerState *s, const char* root_path) {
-  int err;
-
   LoggerHandle *h = NULL;
   for (int i=0; i<LOGGER_MAX_HANDLES; i++) {
     if (s->handles[i].refcnt == 0) {
@@ -174,8 +156,7 @@ static LoggerHandle* logger_open(LoggerState *s, const char* root_path) {
   h->end_sentinel_type = SentinelType::END_OF_SEGMENT;
   h->exit_signal = 0;
 
-  err = logger_mkpath(h->log_path);
-  if (err) return NULL;
+  if (!util::create_directories(h->segment_path, 0775)) return nullptr;
 
   FILE* lock_file = fopen(h->lock_path, "wb");
   if (lock_file == NULL) return NULL;

selfdrive/loggerd/logger.h

@@ -83,7 +83,6 @@ typedef struct LoggerState {
   LoggerHandle* cur_handle;
 } LoggerState;
 
-int logger_mkpath(char* file_path);
 kj::Array<capnp::word> logger_build_init_data();
 std::string logger_get_route_name();
 void logger_init(LoggerState *s, const char* log_name, bool has_qlog);