Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: replace kPathSeparator with std::filesystem #53063

Merged
merged 1 commit into from
Jun 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/compile_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down
3 changes: 2 additions & 1 deletion src/compile_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

#include <cinttypes>
#include <filesystem>
anonrig marked this conversation as resolved.
Show resolved Hide resolved
#include <memory>
#include <string>
#include <unordered_map>
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 4 additions & 2 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <atomic>
#include <cinttypes>
#include <cstdio>
#include <filesystem>
#include <iostream>
#include <limits>
#include <memory>
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);

Expand Down
5 changes: 3 additions & 2 deletions src/inspector_profiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "v8-inspector.h"

#include <cinttypes>
#include <filesystem>
#include <limits>
#include <sstream>
#include "simdutf.h"
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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()) {
Expand Down
18 changes: 6 additions & 12 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -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()) {
Expand Down
9 changes: 4 additions & 5 deletions src/node_report.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions src/path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion src/path.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
18 changes: 5 additions & 13 deletions src/permission/fs_permission.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 /= "*";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I previously mentioned ::operator/ might cause some unexpected results.

}
uv_fs_req_cleanup(&req);
return res;
return path.string();
}

void FreeRecursivelyNode(
Expand Down
3 changes: 2 additions & 1 deletion src/permission/fs_permission.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "v8.h"

#include <filesystem>
#include <unordered_map>
#include "permission/permission_base.h"
#include "util.h"
Expand Down Expand Up @@ -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;
}

Expand Down
3 changes: 0 additions & 3 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading