src: replace kPathSeparator with std::filesystem

PR-URL: #53063 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>
nodejs · Jul 16, 2024 · 664bf6c · 664bf6c
1 parent 0e25fae
commit 664bf6c
Show file tree

Hide file tree

Showing 11 changed files with 34 additions and 46 deletions.
diff --git a/src/compile_cache.cc b/src/compile_cache.cc
@@ -206,7 +206,7 @@ CompileCacheEntry* CompileCacheHandler::GetOrInsert(
   result->code_size = code_utf8.length();
   result->cache_key = key;
   result->cache_filename =
-      compile_cache_dir_ + kPathSeparator + Uint32ToHex(result->cache_key);
+      (compile_cache_dir_ / Uint32ToHex(result->cache_key)).string();
   result->source_filename = filename_utf8.ToString();
   result->cache = nullptr;
   result->type = type;
@@ -380,7 +380,7 @@ bool CompileCacheHandler::InitializeDirectory(Environment* env,
     return false;
   }
 
-  compile_cache_dir_ = cache_dir;
+  compile_cache_dir_ = std::filesystem::path(cache_dir);
   return true;
 }
 

diff --git a/src/compile_cache.h b/src/compile_cache.h
@@ -4,6 +4,7 @@
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
 #include <cinttypes>
+#include <filesystem>
 #include <memory>
 #include <string>
 #include <unordered_map>
@@ -70,7 +71,7 @@ class CompileCacheHandler {
   v8::Isolate* isolate_ = nullptr;
   bool is_debug_ = false;
 
-  std::string compile_cache_dir_;
+  std::filesystem::path compile_cache_dir_;
   // The compile cache is stored in a directory whose name is the hex string of
   // compiler_cache_key_.
   uint32_t compiler_cache_key_ = 0;

diff --git a/src/env.cc b/src/env.cc
@@ -28,6 +28,7 @@
 #include <atomic>
 #include <cinttypes>
 #include <cstdio>
+#include <filesystem>
 #include <iostream>
 #include <limits>
 #include <memory>
@@ -725,7 +726,8 @@ std::string Environment::GetCwd(const std::string& exec_path) {
 
   // This can fail if the cwd is deleted. In that case, fall back to
   // exec_path.
-  return exec_path.substr(0, exec_path.find_last_of(kPathSeparator));
+  return exec_path.substr(
+      0, exec_path.find_last_of(std::filesystem::path::preferred_separator));
 }
 
 void Environment::add_refs(int64_t diff) {
@@ -2079,7 +2081,7 @@ size_t Environment::NearHeapLimitCallback(void* data,
     dir = Environment::GetCwd(env->exec_path_);
   }
   DiagnosticFilename name(env, "Heap", "heapsnapshot");
-  std::string filename = dir + kPathSeparator + (*name);
+  std::string filename = (std::filesystem::path(dir) / (*name)).string();
 
   Debug(env, DebugCategory::DIAGNOSTICS, "Start generating %s...\n", *name);
 

diff --git a/src/inspector_profiler.cc b/src/inspector_profiler.cc
@@ -11,6 +11,7 @@
 #include "v8-inspector.h"
 
 #include <cinttypes>
+#include <filesystem>
 #include <limits>
 #include <sstream>
 #include "simdutf.h"
@@ -248,7 +249,7 @@ void V8ProfilerConnection::WriteProfile(simdjson::ondemand::object* result) {
 
   std::string filename = GetFilename();
   DCHECK(!filename.empty());
-  std::string path = directory + kPathSeparator + filename;
+  std::string path = (std::filesystem::path(directory) / filename).string();
 
   WriteResult(env_, path.c_str(), profile);
 }
@@ -304,7 +305,7 @@ void V8CoverageConnection::WriteProfile(simdjson::ondemand::object* result) {
 
   std::string filename = GetFilename();
   DCHECK(!filename.empty());
-  std::string path = directory + kPathSeparator + filename;
+  std::string path = (std::filesystem::path(directory) / filename).string();
 
   // Only insert source map cache when there's source map data at all.
   if (!source_map_cache_v->IsUndefined()) {

diff --git a/src/node_file.cc b/src/node_file.cc
@@ -82,12 +82,6 @@ using v8::Value;
 # define S_ISDIR(mode)  (((mode) & S_IFMT) == S_IFDIR)
 #endif
 
-#ifdef __POSIX__
-constexpr char kPathSeparator = '/';
-#else
-const char* const kPathSeparator = "\\/";
-#endif
-
 inline int64_t GetOffset(Local<Value> value) {
   return IsSafeJsInt(value) ? value.As<Integer>()->Value() : -1;
 }
@@ -1639,9 +1633,9 @@ int MKDirpSync(uv_loop_t* loop,
           return err;
         }
         case UV_ENOENT: {
-          std::string dirname = next_path.substr(0,
-                                        next_path.find_last_of(kPathSeparator));
-          if (dirname != next_path) {
+          auto filesystem_path = std::filesystem::path(next_path);
+          if (filesystem_path.has_parent_path()) {
+            std::string dirname = filesystem_path.parent_path().string();
             req_wrap->continuation_data()->PushPath(std::move(next_path));
             req_wrap->continuation_data()->PushPath(std::move(dirname));
           } else if (req_wrap->continuation_data()->paths().empty()) {
@@ -1719,9 +1713,9 @@ int MKDirpAsync(uv_loop_t* loop,
           break;
         }
         case UV_ENOENT: {
-          std::string dirname = path.substr(0,
-                                            path.find_last_of(kPathSeparator));
-          if (dirname != path) {
+          auto filesystem_path = std::filesystem::path(path);
+          if (filesystem_path.has_parent_path()) {
+            std::string dirname = filesystem_path.parent_path().string();
             req_wrap->continuation_data()->PushPath(path);
             req_wrap->continuation_data()->PushPath(std::move(dirname));
           } else if (req_wrap->continuation_data()->paths().empty()) {

diff --git a/src/node_report.cc b/src/node_report.cc
@@ -18,10 +18,10 @@
 #include <dlfcn.h>
 #endif
 
-#include <iostream>
 #include <cstring>
 #include <ctime>
 #include <cwctype>
+#include <filesystem>
 #include <fstream>
 
 constexpr int NODE_REPORT_VERSION = 3;
@@ -887,10 +887,9 @@ std::string TriggerNodeReport(Isolate* isolate,
       report_directory = per_process::cli_options->report_directory;
     }
     // Regular file. Append filename to directory path if one was specified
-    if (report_directory.length() > 0) {
-      std::string pathname = report_directory;
-      pathname += kPathSeparator;
-      pathname += filename;
+    if (!report_directory.empty()) {
+      std::string pathname =
+          (std::filesystem::path(report_directory) / filename).string();
       outfile.open(pathname, std::ios::out | std::ios::binary);
     } else {
       outfile.open(filename, std::ios::out | std::ios::binary);

diff --git a/src/path.cc b/src/path.cc
@@ -7,12 +7,12 @@
 namespace node {
 
 #ifdef _WIN32
-bool IsPathSeparator(const char c) noexcept {
-  return c == kPathSeparator || c == '/';
+constexpr bool IsPathSeparator(char c) noexcept {
+  return c == '\\' || c == '/';
 }
 #else   // POSIX
-bool IsPathSeparator(const char c) noexcept {
-  return c == kPathSeparator;
+constexpr bool IsPathSeparator(char c) noexcept {
+  return c == '/';
 }
 #endif  // _WIN32
 

diff --git a/src/path.h b/src/path.h
@@ -10,8 +10,9 @@
 
 namespace node {
 
-bool IsPathSeparator(const char c) noexcept;
+class Environment;
 
+constexpr bool IsPathSeparator(char c) noexcept;
 std::string NormalizeString(const std::string_view path,
                             bool allowAboveRoot,
                             const std::string_view separator);

diff --git a/src/permission/fs_permission.cc b/src/permission/fs_permission.cc
@@ -17,20 +17,12 @@
 namespace {
 
 std::string WildcardIfDir(const std::string& res) noexcept {
-  uv_fs_t req;
-  int rc = uv_fs_stat(nullptr, &req, res.c_str(), nullptr);
-  if (rc == 0) {
-    const uv_stat_t* const s = static_cast<const uv_stat_t*>(req.ptr);
-    if ((s->st_mode & S_IFMT) == S_IFDIR) {
-      // add wildcard when directory
-      if (res.back() == node::kPathSeparator) {
-        return res + "*";
-      }
-      return res + node::kPathSeparator + "*";
-    }
+  auto path = std::filesystem::path(res);
+  auto file_status = std::filesystem::status(path);
+  if (file_status.type() == std::filesystem::file_type::directory) {
+    path /= "*";
   }
-  uv_fs_req_cleanup(&req);
-  return res;
+  return path.string();
 }
 
 void FreeRecursivelyNode(

diff --git a/src/permission/fs_permission.h b/src/permission/fs_permission.h
@@ -5,6 +5,7 @@
 
 #include "v8.h"
 
+#include <filesystem>
 #include <unordered_map>
 #include "permission/permission_base.h"
 #include "util.h"
@@ -106,7 +107,7 @@ class FSPermission final : public PermissionBase {
           // path = /home/subdirectory
           // child = subdirectory/*
           if (idx >= path.length() &&
-              child->prefix[i] == node::kPathSeparator) {
+              child->prefix[i] == std::filesystem::path::preferred_separator) {
             continue;
           }
 

diff --git a/src/util.h b/src/util.h
@@ -56,13 +56,10 @@
 
 namespace node {
 
-// Maybe remove kPathSeparator when cpp17 is ready
 #ifdef _WIN32
-    constexpr char kPathSeparator = '\\';
 /* MAX_PATH is in characters, not bytes. Make sure we have enough headroom. */
 #define PATH_MAX_BYTES (MAX_PATH * 4)
 #else
-    constexpr char kPathSeparator = '/';
 #define PATH_MAX_BYTES (PATH_MAX)
 #endif