Skip to content

Commit db29a1d

Browse files
indutnyindutny
committed
address feedback
1 parent ec1fc03 commit db29a1d

File tree

4 files changed

+57
-54
lines changed

4 files changed

+57
-54
lines changed

‎src/node_file.cc‎

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3169,19 +3169,6 @@ static void GetFormatOfExtensionlessFile(
31693169
return args.GetReturnValue().Set(EXTENSIONLESS_FORMAT_JAVASCRIPT);
31703170
}
31713171

3172-
#ifdef _WIN32
3173-
#define BufferValueToPath(str) \
3174-
std::filesystem::path(ConvertToWideString(str.ToString(), CP_UTF8))
3175-
3176-
#define PathToString(path) ConvertWideToUTF8(path.wstring());
3177-
3178-
#else // _WIN32
3179-
3180-
#define BufferValueToPath(str) std::filesystem::path(str.ToStringView());
3181-
#define PathToString(path) path.native();
3182-
3183-
#endif // _WIN32
3184-
31853172
static void CpSyncCheckPaths(const FunctionCallbackInfo<Value>& args) {
31863173
Environment* env = Environment::GetCurrent(args);
31873174
Isolate* isolate = env->isolate();
@@ -3194,15 +3181,15 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo<Value>& args) {
31943181
THROW_IF_INSUFFICIENT_PERMISSIONS(
31953182
env, permission::PermissionScope::kFileSystemRead, src.ToStringView());
31963183

3197-
auto src_path = BufferValueToPath(src);
3184+
auto src_path = src.ToPath();
31983185

31993186
BufferValue dest(isolate, args[1]);
32003187
CHECK_NOT_NULL(*dest);
32013188
ToNamespacedPath(env, &dest);
32023189
THROW_IF_INSUFFICIENT_PERMISSIONS(
32033190
env, permission::PermissionScope::kFileSystemWrite, dest.ToStringView());
32043191

3205-
auto dest_path = BufferValueToPath(dest);
3192+
auto dest_path = dest.ToPath();
32063193
bool dereference = args[2]->IsTrue();
32073194
bool recursive = args[3]->IsTrue();
32083195

@@ -3231,8 +3218,8 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo<Value>& args) {
32313218
(src_status.type() == std::filesystem::file_type::directory) ||
32323219
(dereference && src_status.type() == std::filesystem::file_type::symlink);
32333220

3234-
auto src_path_str = PathToString(src_path);
3235-
auto dest_path_str = PathToString(dest_path);
3221+
auto src_path_str = ConvertPathToUTF8(src_path);
3222+
auto dest_path_str = ConvertPathToUTF8(dest_path);
32363223

32373224
if (!error_code) {
32383225
// Check if src and dest are identical.
@@ -3327,7 +3314,7 @@ static bool CopyUtimes(const std::filesystem::path& src,
33273314
uv_fs_t req;
33283315
auto cleanup = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); });
33293316

3330-
auto src_path_str = PathToString(src);
3317+
auto src_path_str = ConvertPathToUTF8(src);
33313318
int result = uv_fs_stat(nullptr, &req, src_path_str.c_str(), nullptr);
33323319
if (is_uv_error(result)) {
33333320
env->ThrowUVException(result, "stat", nullptr, src_path_str.c_str());
@@ -3338,7 +3325,7 @@ static bool CopyUtimes(const std::filesystem::path& src,
33383325
const double source_atime = s->st_atim.tv_sec + s->st_atim.tv_nsec / 1e9;
33393326
const double source_mtime = s->st_mtim.tv_sec + s->st_mtim.tv_nsec / 1e9;
33403327

3341-
auto dest_file_path_str = PathToString(dest);
3328+
auto dest_file_path_str = ConvertPathToUTF8(dest);
33423329
int utime_result = uv_fs_utime(nullptr,
33433330
&req,
33443331
dest_file_path_str.c_str(),
@@ -3473,7 +3460,7 @@ static void CpSyncCopyDir(const FunctionCallbackInfo<Value>& args) {
34733460
std::error_code error;
34743461
for (auto dir_entry : std::filesystem::directory_iterator(src)) {
34753462
auto dest_file_path = dest / dir_entry.path().filename();
3476-
auto dest_str = PathToString(dest);
3463+
auto dest_str = ConvertPathToUTF8(dest);
34773464

34783465
if (dir_entry.is_symlink()) {
34793466
if (verbatim_symlinks) {
@@ -3536,7 +3523,7 @@ static void CpSyncCopyDir(const FunctionCallbackInfo<Value>& args) {
35363523
}
35373524
} else if (std::filesystem::is_regular_file(dest_file_path)) {
35383525
if (!dereference || (!force && error_on_exist)) {
3539-
auto dest_file_path_str = PathToString(dest_file_path);
3526+
auto dest_file_path_str = ConvertPathToUTF8(dest_file_path);
35403527
env->ThrowStdErrException(
35413528
std::make_error_code(std::errc::file_exists),
35423529
"cp",

‎src/node_modules.cc‎

Lines changed: 8 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -297,16 +297,10 @@ const BindingData::PackageConfig* BindingData::TraverseParent(
297297
// Stop the search when the process doesn't have permissions
298298
// to walk upwards
299299
if (is_permissions_enabled) {
300-
std::string generic_path;
301-
#ifdef _WIN32
302-
generic_path = ConvertWideToUTF8(current_path.generic_wstring());
303-
#else // _WIN32
304-
generic_path = current_path.generic_string();
305-
#endif // _WIN32
306-
//
307300
if (!env->permission()->is_granted(
308-
env, permission::PermissionScope::kFileSystemRead, generic_path))
309-
[[unlikely]] {
301+
env,
302+
permission::PermissionScope::kFileSystemRead,
303+
ConvertGenericPathToUTF8(current_path))) [[unlikely]] {
310304
return nullptr;
311305
}
312306
}
@@ -318,13 +312,8 @@ const BindingData::PackageConfig* BindingData::TraverseParent(
318312

319313
auto package_json_path = current_path / "package.json";
320314

321-
std::string package_json_path_str;
322-
#ifdef _WIN32
323-
package_json_path_str = ConvertWideToUTF8(package_json_path.wstring());
324-
#else // _WIN32
325-
package_json_path_str = package_json_path.string();
326-
#endif // _WIN32
327-
auto package_json = GetPackageJSON(realm, package_json_path_str, nullptr);
315+
auto package_json =
316+
GetPackageJSON(realm, ConvertPathToUTF8(package_json_path), nullptr);
328317
if (package_json != nullptr) {
329318
return package_json;
330319
}
@@ -346,20 +335,12 @@ void BindingData::GetNearestParentPackageJSONType(
346335

347336
ToNamespacedPath(realm->env(), &path_value);
348337

349-
std::string path_value_str = path_value.ToString();
338+
auto path = path_value.ToPath();
339+
350340
if (slashCheck) {
351-
path_value_str.push_back(kPathSeparator);
341+
path /= "";
352342
}
353343

354-
std::filesystem::path path;
355-
356-
#ifdef _WIN32
357-
std::wstring wide_path = ConvertToWideString(path_value_str, CP_UTF8);
358-
path = std::filesystem::path(wide_path);
359-
#else
360-
path = std::filesystem::path(path_value_str);
361-
#endif
362-
363344
auto package_json = TraverseParent(realm, path);
364345

365346
if (package_json == nullptr) {

‎src/util-inl.h‎

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -718,12 +718,11 @@ inline bool IsWindowsBatchFile(const char* filename) {
718718
return !extension.empty() && (extension == "cmd" || extension == "bat");
719719
}
720720

721-
inline std::wstring ConvertToWideString(const std::string& str,
722-
UINT code_page) {
721+
inline std::wstring ConvertUTF8ToWideString(const std::string& str) {
723722
int size_needed = MultiByteToWideChar(
724-
code_page, 0, &str[0], static_cast<int>(str.size()), nullptr, 0);
723+
CP_UTF8, 0, &str[0], static_cast<int>(str.size()), nullptr, 0);
725724
std::wstring wstrTo(size_needed, 0);
726-
MultiByteToWideChar(code_page,
725+
MultiByteToWideChar(CP_UTF8,
727726
0,
728727
&str[0],
729728
static_cast<int>(str.size()),
@@ -754,6 +753,36 @@ std::string ConvertWideToUTF8(const std::wstring& wstr) {
754753
nullptr);
755754
return strTo;
756755
}
756+
757+
template <typename T, size_t kStackStorageSize>
758+
std::filesystem::path MaybeStackBuffer<T, kStackStorageSize>::ToPath() const {
759+
std::wstring wide_path = ConvertUTF8ToWideString(ToString());
760+
return std::filesystem::path(wide_path);
761+
}
762+
763+
std::string ConvertPathToUTF8(const std::filesystem::path& path) {
764+
return ConvertWideStringToUTF8(path.wstring());
765+
}
766+
767+
std::string ConvertGenericPathToUTF8(const std::filesystem::path& path) {
768+
return ConvertWideStringToUTF8(path.generic_wstring());
769+
}
770+
771+
#else // _WIN32
772+
773+
template <typename T, size_t kStackStorageSize>
774+
std::filesystem::path MaybeStackBuffer<T, kStackStorageSize>::ToPath() const {
775+
return std::filesystem::path(ToStringView());
776+
}
777+
778+
std::string ConvertPathToUTF8(const std::filesystem::path& path) {
779+
return path.native();
780+
}
781+
782+
std::string ConvertGenericPathToUTF8(const std::filesystem::path& path) {
783+
return path.generic_string();
784+
}
785+
757786
#endif // _WIN32
758787

759788
inline v8::MaybeLocal<v8::Object> NewDictionaryInstance(

‎src/util.h‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,7 @@ class MaybeStackBuffer {
507507
inline std::basic_string_view<T> ToStringView() const {
508508
return {out(), length()};
509509
}
510+
inline std::filesystem::path ToPath() const;
510511

511512
private:
512513
size_t length_;
@@ -1038,10 +1039,15 @@ class JSONOutputStream final : public v8::OutputStream {
10381039
// Returns true if OS==Windows and filename ends in .bat or .cmd,
10391040
// case insensitive.
10401041
inline bool IsWindowsBatchFile(const char* filename);
1041-
inline std::wstring ConvertToWideString(const std::string& str, UINT code_page);
1042-
inline std::string ConvertWideToUTF8(const std::wstring& wstr);
1042+
inline std::wstring ConvertUTF8ToWideString(const std::string& str);
1043+
inline std::string ConvertWideStringToUTF8(const std::wstring& wstr);
1044+
10431045
#endif // _WIN32
10441046

1047+
inline std::filesystem::path ConvertUTF8ToPath(const std::string& str);
1048+
inline std::string ConvertPathToUTF8(const std::filesystem::path& path);
1049+
inline std::string ConvertGenericPathToUTF8(const std::filesystem::path& path);
1050+
10451051
// A helper to create a new instance of the dictionary template.
10461052
// Unlike v8::DictionaryTemplate::NewInstance, this method will
10471053
// check that all properties have been set (are not empty MaybeLocals)

0 commit comments

Comments
 (0)