From b53ae22f5c493ccdc4ff40bb139b5a06bd0cb740 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Sun, 18 Jun 2023 12:23:42 -0400 Subject: [PATCH 1/2] src: refactor `SplitString` in util --- src/node_options.cc | 8 +++++--- src/node_v8_platform-inl.h | 5 +++-- src/permission/fs_permission.cc | 5 +++-- src/util.cc | 26 ++++++++++++++------------ src/util.h | 5 ++--- 5 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/node_options.cc b/src/node_options.cc index 4e3633e5b5b8a6..5cfb7208747f4b 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -50,11 +50,13 @@ void DebugOptions::CheckOptions(std::vector* errors, "`node --inspect-brk` instead."); } - std::vector destinations = - SplitString(inspect_publish_uid_string, ','); + std::vector destinations = + SplitString(std::string_view(inspect_publish_uid_string.c_str(), + inspect_publish_uid_string.size()), + ","); inspect_publish_uid.console = false; inspect_publish_uid.http = false; - for (const std::string& destination : destinations) { + for (const std::string_view destination : destinations) { if (destination == "stderr") { inspect_publish_uid.console = true; } else if (destination == "http") { diff --git a/src/node_v8_platform-inl.h b/src/node_v8_platform-inl.h index 9f428d53315713..7af74ec47ebc82 100644 --- a/src/node_v8_platform-inl.h +++ b/src/node_v8_platform-inl.h @@ -129,8 +129,9 @@ struct V8Platform { // Attach a new NodeTraceWriter only if this function hasn't been called // before. if (tracing_file_writer_.IsDefaultHandle()) { - std::vector categories = - SplitString(per_process::cli_options->trace_event_categories, ','); + auto out = per_process::cli_options->trace_event_categories; + std::vector categories = + SplitString(std::string_view(out.c_str(), out.size()), ","); tracing_file_writer_ = tracing_agent_->AddClient( std::set(std::make_move_iterator(categories.begin()), diff --git a/src/permission/fs_permission.cc b/src/permission/fs_permission.cc index 7a8a0ba2511d4b..936975df48b89c 100644 --- a/src/permission/fs_permission.cc +++ b/src/permission/fs_permission.cc @@ -74,7 +74,8 @@ namespace permission { // allow = '*' // allow = '/tmp/,/home/example.js' void FSPermission::Apply(const std::string& allow, PermissionScope scope) { - for (const auto& res : SplitString(allow, ',')) { + for (const auto res : + SplitString(std::string_view(allow.c_str(), allow.size()), ",")) { if (res == "*") { if (scope == PermissionScope::kFileSystemRead) { deny_all_in_ = false; @@ -85,7 +86,7 @@ void FSPermission::Apply(const std::string& allow, PermissionScope scope) { } return; } - GrantAccess(scope, res); + GrantAccess(scope, std::string(res.data(), res.size())); } } diff --git a/src/util.cc b/src/util.cc index 7aaa5e2be5b880..91c69a98c49c41 100644 --- a/src/util.cc +++ b/src/util.cc @@ -169,19 +169,21 @@ std::string GetHumanReadableProcessName() { return SPrintF("%s[%d]", GetProcessTitle("Node.js"), uv_os_getpid()); } -std::vector SplitString(const std::string& in, - char delim, - bool skipEmpty) { - std::vector out; - if (in.empty()) - return out; - std::istringstream in_stream(in); - while (in_stream.good()) { - std::string item; - std::getline(in_stream, item, delim); - if (item.empty() && skipEmpty) continue; - out.emplace_back(std::move(item)); +std::vector SplitString(const std::string_view in, + const std::string_view delim) { + std::vector out; + + for (auto first = in.data(), second = in.data(), last = first + in.size(); + second != last && first != last; + first = second + 1) { + second = + std::find_first_of(first, last, std::cbegin(delim), std::cend(delim)); + + if (first != second) { + out.emplace_back(first, second - first); + } } + return out; } diff --git a/src/util.h b/src/util.h index 52dd334d07d8b1..8b8ecf41d7d201 100644 --- a/src/util.h +++ b/src/util.h @@ -685,9 +685,8 @@ struct FunctionDeleter { template using DeleteFnPtr = typename FunctionDeleter::Pointer; -std::vector SplitString(const std::string& in, - char delim, - bool skipEmpty = true); +std::vector SplitString(const std::string_view in, + const std::string_view delim); inline v8::MaybeLocal ToV8Value(v8::Local context, std::string_view str, From 63f8f96397cb992a08edde8b407a169f0ee3bc57 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Thu, 22 Jun 2023 15:23:19 -0400 Subject: [PATCH 2/2] fixup! src: refactor `SplitString` in util --- src/node_options.cc | 16 ++++++++-------- src/node_v8_platform-inl.h | 18 +++++++++++++----- src/permission/fs_permission.cc | 7 ++++--- 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/node_options.cc b/src/node_options.cc index 5cfb7208747f4b..fc6050877eb8b4 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -11,10 +11,11 @@ #endif #include -#include -#include #include #include // strtoul, errno +#include +#include +#include using v8::Boolean; using v8::Context; @@ -50,16 +51,15 @@ void DebugOptions::CheckOptions(std::vector* errors, "`node --inspect-brk` instead."); } - std::vector destinations = - SplitString(std::string_view(inspect_publish_uid_string.c_str(), - inspect_publish_uid_string.size()), - ","); + using std::string_view_literals::operator""sv; + const std::vector destinations = + SplitString(inspect_publish_uid_string, ","sv); inspect_publish_uid.console = false; inspect_publish_uid.http = false; for (const std::string_view destination : destinations) { - if (destination == "stderr") { + if (destination == "stderr"sv) { inspect_publish_uid.console = true; - } else if (destination == "http") { + } else if (destination == "http"sv) { inspect_publish_uid.http = true; } else { errors->push_back("--inspect-publish-uid destination can be " diff --git a/src/node_v8_platform-inl.h b/src/node_v8_platform-inl.h index 7af74ec47ebc82..4e3d6cfe31d9bd 100644 --- a/src/node_v8_platform-inl.h +++ b/src/node_v8_platform-inl.h @@ -4,6 +4,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #include +#include #include "env-inl.h" #include "node.h" @@ -126,16 +127,23 @@ struct V8Platform { } inline void StartTracingAgent() { + constexpr auto convert_to_set = + [](std::vector categories) -> std::set { + std::set out; + for (const auto s : categories) { + out.emplace(s); + } + return out; + }; // Attach a new NodeTraceWriter only if this function hasn't been called // before. if (tracing_file_writer_.IsDefaultHandle()) { - auto out = per_process::cli_options->trace_event_categories; - std::vector categories = - SplitString(std::string_view(out.c_str(), out.size()), ","); + using std::string_view_literals::operator""sv; + const std::vector categories = + SplitString(per_process::cli_options->trace_event_categories, ","sv); tracing_file_writer_ = tracing_agent_->AddClient( - std::set(std::make_move_iterator(categories.begin()), - std::make_move_iterator(categories.end())), + convert_to_set(categories), std::unique_ptr( new tracing::NodeTraceWriter( per_process::cli_options->trace_event_file_pattern)), diff --git a/src/permission/fs_permission.cc b/src/permission/fs_permission.cc index 936975df48b89c..0c844703b5a7ae 100644 --- a/src/permission/fs_permission.cc +++ b/src/permission/fs_permission.cc @@ -9,6 +9,7 @@ #include #include #include +#include #include namespace { @@ -74,9 +75,9 @@ namespace permission { // allow = '*' // allow = '/tmp/,/home/example.js' void FSPermission::Apply(const std::string& allow, PermissionScope scope) { - for (const auto res : - SplitString(std::string_view(allow.c_str(), allow.size()), ",")) { - if (res == "*") { + using std::string_view_literals::operator""sv; + for (const std::string_view res : SplitString(allow, ","sv)) { + if (res == "*"sv) { if (scope == PermissionScope::kFileSystemRead) { deny_all_in_ = false; allow_all_in_ = true;