From 640a791831d9df769b4688f7ee6915dd115a7821 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Thu, 22 Jun 2023 18:23:28 -0400 Subject: [PATCH] src: refactor `SplitString` in util PR-URL: https://github.com/nodejs/node/pull/48491 Reviewed-By: James M Snell Reviewed-By: Stephen Belanger Reviewed-By: Rafael Gonzaga --- src/node_options.cc | 16 +++++++++------- src/node_v8_platform-inl.h | 17 +++++++++++++---- src/permission/fs_permission.cc | 8 +++++--- src/util.cc | 26 ++++++++++++++------------ src/util.h | 5 ++--- 5 files changed, 43 insertions(+), 29 deletions(-) diff --git a/src/node_options.cc b/src/node_options.cc index 4e3633e5b5b8a6..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,14 +51,15 @@ void DebugOptions::CheckOptions(std::vector* errors, "`node --inspect-brk` instead."); } - std::vector destinations = - SplitString(inspect_publish_uid_string, ','); + 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& destination : destinations) { - if (destination == "stderr") { + for (const std::string_view destination : destinations) { + 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 9f428d53315713..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,15 +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()) { - std::vector categories = - SplitString(per_process::cli_options->trace_event_categories, ','); + 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 7a8a0ba2511d4b..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,8 +75,9 @@ namespace permission { // allow = '*' // allow = '/tmp/,/home/example.js' void FSPermission::Apply(const std::string& allow, PermissionScope scope) { - for (const auto& res : SplitString(allow, ',')) { - 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; @@ -85,7 +87,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,