From d44c8f5e7eb4d96d6c513d879156cfeb237fe6a1 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Fri, 29 Nov 2024 14:14:39 -0500 Subject: [PATCH] src: converts args to std::span --- src/api/embed_helpers.cc | 4 ++-- src/api/environment.cc | 4 ++-- src/node.cc | 34 +++++++++++++++++++--------------- src/node.h | 12 ++++++------ src/node_dotenv.cc | 6 +++--- src/node_dotenv.h | 4 ++-- src/node_internals.h | 10 +++++----- src/node_options.h | 7 ++++--- src/node_sea.cc | 2 +- src/node_snapshot_builder.h | 8 ++++---- src/node_snapshotable.cc | 18 +++++++++--------- src/node_task_runner.cc | 4 ++-- src/node_task_runner.h | 2 +- 13 files changed, 60 insertions(+), 55 deletions(-) diff --git a/src/api/embed_helpers.cc b/src/api/embed_helpers.cc index 34de89a8dc0398..d1ba03c42703a5 100644 --- a/src/api/embed_helpers.cc +++ b/src/api/embed_helpers.cc @@ -188,8 +188,8 @@ std::unique_ptr CommonEnvironmentSetup::CreateForSnapshotting( MultiIsolatePlatform* platform, std::vector* errors, - const std::vector& args, - const std::vector& exec_args, + const std::vector& args, + const std::vector& exec_args, const SnapshotConfig& snapshot_config) { // It's not guaranteed that a context that goes through // v8_inspector::V8Inspector::contextCreated() is runtime-independent, diff --git a/src/api/environment.cc b/src/api/environment.cc index 2641ea7d7f84f4..0e541731f04c95 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -396,8 +396,8 @@ struct InspectorParentHandleImpl : public InspectorParentHandle { Environment* CreateEnvironment( IsolateData* isolate_data, Local context, - const std::vector& args, - const std::vector& exec_args, + const std::vector& args, + const std::vector& exec_args, EnvironmentFlags::Flags flags, ThreadId thread_id, std::unique_ptr inspector_parent_handle) { diff --git a/src/node.cc b/src/node.cc index dd52fbffac0dee..89be7ff6760978 100644 --- a/src/node.cc +++ b/src/node.cc @@ -710,8 +710,8 @@ void ResetStdio() { #endif // __POSIX__ } -static ExitCode ProcessGlobalArgsInternal(std::vector* args, - std::vector* exec_args, +static ExitCode ProcessGlobalArgsInternal(std::vector* args, + std::vector* exec_args, std::vector* errors, OptionEnvvarSettings settings) { // Parse a few arguments which are specific to Node. @@ -803,8 +803,8 @@ static std::atomic_bool init_called{false}; // TODO(addaleax): Turn this into a wrapper around InitializeOncePerProcess() // (with the corresponding additional flags set), then eventually remove this. static ExitCode InitializeNodeWithArgsInternal( - std::vector* argv, - std::vector* exec_argv, + std::vector* argv, + std::vector* exec_argv, std::vector* errors, ProcessInitializationFlags::Flags flags) { // Make sure InitializeNodeWithArgs() is called only once. @@ -861,16 +861,16 @@ static ExitCode InitializeNodeWithArgsInternal( case Dotenv::ParseResult::Valid: break; case Dotenv::ParseResult::InvalidContent: - errors->push_back(file_data.path + ": invalid format"); + errors->push_back(std::string(file_data.path) + ": invalid format"); break; case Dotenv::ParseResult::FileError: if (file_data.is_optional) { fprintf(stderr, "%s not found. Continuing without it.\n", - file_data.path.c_str()); + file_data.path.data()); continue; } - errors->push_back(file_data.path + ": not found"); + errors->push_back(std::string(file_data.path) + ": not found"); break; default: UNREACHABLE(); @@ -885,7 +885,7 @@ static ExitCode InitializeNodeWithArgsInternal( // NODE_OPTIONS environment variable is preferred over the file one. if (credentials::SafeGetenv("NODE_OPTIONS", &node_options) || !node_options.empty()) { - std::vector env_argv = + std::vector env_argv = ParseNodeOptionsEnvVar(node_options, errors); if (!errors->empty()) return ExitCode::kInvalidCommandLineArgument; @@ -975,8 +975,8 @@ static ExitCode InitializeNodeWithArgsInternal( return ExitCode::kNoFailure; } -int InitializeNodeWithArgs(std::vector* argv, - std::vector* exec_argv, +int InitializeNodeWithArgs(std::vector* argv, + std::vector* exec_argv, std::vector* errors, ProcessInitializationFlags::Flags flags) { return static_cast( @@ -984,11 +984,11 @@ int InitializeNodeWithArgs(std::vector* argv, } static std::shared_ptr -InitializeOncePerProcessInternal(const std::vector& args, +InitializeOncePerProcessInternal(const std::vector& args, ProcessInitializationFlags::Flags flags = ProcessInitializationFlags::kNoFlags) { auto result = std::make_shared(); - result->args_ = args; + result->args_ = std::move(args); if (!(flags & ProcessInitializationFlags::kNoParseGlobalDebugVariables)) { // Initialized the enabled list for Debug() calls with system @@ -1216,7 +1216,7 @@ InitializeOncePerProcessInternal(const std::vector& args, } std::shared_ptr InitializeOncePerProcess( - const std::vector& args, + const std::vector& args, ProcessInitializationFlags::Flags flags) { return InitializeOncePerProcessInternal(args, flags); } @@ -1286,7 +1286,11 @@ ExitCode GenerateAndWriteSnapshotData(const SnapshotData** snapshot_data_ptr, } } else { snapshot_config.builder_script_path = result->args()[1]; - args_maybe_patched = result->args(); + args_maybe_patched.reserve(result->args().size()); + // TODO(anonrig): Avoid copying in here. + for (const auto& arg : result->args()) { + args_maybe_patched.push_back(std::string(arg)); + }; } DCHECK(snapshot_config.builder_script_path.has_value()); const std::string& builder_script = @@ -1429,7 +1433,7 @@ static ExitCode StartInternal(int argc, char** argv) { std::shared_ptr result = InitializeOncePerProcessInternal( - std::vector(argv, argv + argc)); + std::vector(argv, argv + argc)); for (const std::string& error : result->errors()) { FPrintF(stderr, "%s: %s\n", result->args().at(0), error); } diff --git a/src/node.h b/src/node.h index 643f964115b0ae..ee3974d3c1e491 100644 --- a/src/node.h +++ b/src/node.h @@ -350,7 +350,7 @@ NODE_DEPRECATED("Use InitializeOncePerProcess() instead", // errors encountered during initialization, and a potential suggested // exit code. NODE_EXTERN std::shared_ptr InitializeOncePerProcess( - const std::vector& args, + const std::vector& args, ProcessInitializationFlags::Flags flags = ProcessInitializationFlags::kNoFlags); // Undoes the initialization performed by InitializeOncePerProcess(), @@ -359,7 +359,7 @@ NODE_EXTERN void TearDownOncePerProcess(); // Convenience overload for specifying multiple flags without having // to worry about casts. inline std::shared_ptr InitializeOncePerProcess( - const std::vector& args, + const std::vector& args, std::initializer_list list) { uint64_t flags_accum = ProcessInitializationFlags::kNoFlags; for (const auto flag : list) flags_accum |= static_cast(flag); @@ -704,8 +704,8 @@ struct InspectorParentHandle { NODE_EXTERN Environment* CreateEnvironment( IsolateData* isolate_data, v8::Local context, - const std::vector& args, - const std::vector& exec_args, + const std::vector& args, + const std::vector& exec_args, EnvironmentFlags::Flags flags = EnvironmentFlags::kDefaultFlags, ThreadId thread_id = {} /* allocates a thread id automatically */, std::unique_ptr inspector_parent_handle = {}); @@ -923,8 +923,8 @@ class NODE_EXTERN CommonEnvironmentSetup { static std::unique_ptr CreateForSnapshotting( MultiIsolatePlatform* platform, std::vector* errors, - const std::vector& args = {}, - const std::vector& exec_args = {}, + const std::vector& args = {}, + const std::vector& exec_args = {}, const SnapshotConfig& snapshot_config = {}); EmbedderSnapshotData::Pointer CreateSnapshot(); diff --git a/src/node_dotenv.cc b/src/node_dotenv.cc index 049f5cfcb77b9c..d7a75a9d34efe8 100644 --- a/src/node_dotenv.cc +++ b/src/node_dotenv.cc @@ -12,8 +12,8 @@ using v8::Object; using v8::String; std::vector Dotenv::GetDataFromArgs( - const std::vector& args) { - const std::string_view optional_env_file_flag = "--env-file-if-exists"; + const std::vector& args) { + constexpr std::string_view optional_env_file_flag = "--env-file-if-exists"; const auto find_match = [](const std::string& arg) { return arg == "--" || arg == "--env-file" || @@ -37,7 +37,7 @@ std::vector Dotenv::GetDataFromArgs( auto flag = matched_arg->substr(0, equal_char_index); auto file_path = matched_arg->substr(equal_char_index + 1); - struct env_file_data env_file_data = { + env_file_data env_file_data = { file_path, flag.starts_with(optional_env_file_flag)}; env_files.push_back(env_file_data); } else { diff --git a/src/node_dotenv.h b/src/node_dotenv.h index d508b13fc5db74..2bc251a01d3d59 100644 --- a/src/node_dotenv.h +++ b/src/node_dotenv.h @@ -14,7 +14,7 @@ class Dotenv { public: enum ParseResult { Valid, FileError, InvalidContent }; struct env_file_data { - std::string path; + std::string_view path; bool is_optional; }; @@ -32,7 +32,7 @@ class Dotenv { v8::Local ToObject(Environment* env) const; static std::vector GetDataFromArgs( - const std::vector& args); + const std::vector& args); private: std::map store_; diff --git a/src/node_internals.h b/src/node_internals.h index 000ba16303740d..3e6970945d5695 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -346,14 +346,14 @@ class InitializationResultImpl final : public InitializationResult { int exit_code() const { return static_cast(exit_code_enum()); } ExitCode exit_code_enum() const { return exit_code_; } bool early_return() const { return early_return_; } - const std::vector& args() const { return args_; } - const std::vector& exec_args() const { return exec_args_; } - const std::vector& errors() const { return errors_; } + const std::vector& args() const { return args_; } + const std::vector& exec_args() const { return exec_args_; } + const std::vector& errors() const { return errors_; } MultiIsolatePlatform* platform() const { return platform_; } ExitCode exit_code_ = ExitCode::kNoFailure; - std::vector args_; - std::vector exec_args_; + std::vector args_; + std::vector exec_args_; std::vector errors_; bool early_return_ = false; MultiIsolatePlatform* platform_ = nullptr; diff --git a/src/node_options.h b/src/node_options.h index 8cce0ec019254d..d407409bd88e3c 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -358,7 +358,7 @@ class PerProcessOptions : public Options { // TODO(addaleax): Some of these could probably be per-Environment. std::string use_largepages = "off"; bool trace_sigint = false; - std::vector cmdline; + std::vector cmdline; inline PerIsolateOptions* get_per_isolate_options(); void CheckOptions(std::vector* errors, @@ -567,9 +567,10 @@ class OptionsParser { }; using StringVector = std::vector; +using StringViewVector = std::vector; template void Parse( - StringVector* const args, StringVector* const exec_args, + StringViewVector* const args, StringViewVector* const exec_args, StringVector* const v8_args, OptionsType* const options, OptionEnvvarSettings required_env_settings, StringVector* const errors); @@ -586,7 +587,7 @@ void HandleEnvOptions(std::shared_ptr env_options); void HandleEnvOptions(std::shared_ptr env_options, std::function opt_getter); -std::vector ParseNodeOptionsEnvVar( +std::vector ParseNodeOptionsEnvVar( const std::string& node_options, std::vector* errors); } // namespace node diff --git a/src/node_sea.cc b/src/node_sea.cc index fb9f933a19fa70..19a9fb767e293f 100644 --- a/src/node_sea.cc +++ b/src/node_sea.cc @@ -388,7 +388,7 @@ ExitCode GenerateSnapshotForSEA(const SeaConfig& config, SnapshotData snapshot; // TODO(joyeecheung): make the arguments configurable through the JSON // config or a programmatic API. - std::vector patched_args = {args[0], config.main_path}; + std::vector patched_args = {args[0], config.main_path}; ExitCode exit_code = SnapshotBuilder::Generate(&snapshot, patched_args, exec_args, diff --git a/src/node_snapshot_builder.h b/src/node_snapshot_builder.h index e2302946d1f8cb..8be8d4a0e9a9a3 100644 --- a/src/node_snapshot_builder.h +++ b/src/node_snapshot_builder.h @@ -21,8 +21,8 @@ std::optional ReadSnapshotConfig(const char* path); class NODE_EXTERN_PRIVATE SnapshotBuilder { public: static ExitCode GenerateAsSource(const char* out_path, - const std::vector& args, - const std::vector& exec_args, + const std::vector& args, + const std::vector& exec_args, const SnapshotConfig& config, bool use_array_literals = false); @@ -31,8 +31,8 @@ class NODE_EXTERN_PRIVATE SnapshotBuilder { // in case the script is already read for other purposes. static ExitCode Generate( SnapshotData* out, - const std::vector& args, - const std::vector& exec_args, + const std::vector& args, + const std::vector& exec_args, std::optional builder_script_content, const SnapshotConfig& config); diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index fe04a8ee8d708b..855fb774d0f98e 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -936,8 +936,8 @@ std::optional ReadSnapshotConfig(const char* config_path) { ExitCode BuildSnapshotWithoutCodeCache( SnapshotData* out, - const std::vector& args, - const std::vector& exec_args, + const std::vector& args, + const std::vector& exec_args, std::optional builder_script_content, const SnapshotConfig& config) { DCHECK(builder_script_content.has_value() == @@ -957,7 +957,7 @@ ExitCode BuildSnapshotWithoutCodeCache( per_process::v8_platform.Platform(), &errors, args, exec_args, config); if (!setup) { for (const std::string& err : errors) - fprintf(stderr, "%s: %s\n", args[0].c_str(), err.c_str()); + fprintf(stderr, "%s: %s\n", args[0].data(), err.c_str()); return ExitCode::kBootstrapFailure; } @@ -999,8 +999,8 @@ ExitCode BuildSnapshotWithoutCodeCache( } ExitCode BuildCodeCacheFromSnapshot(SnapshotData* out, - const std::vector& args, - const std::vector& exec_args) { + const std::vector& args, + const std::vector& exec_args) { RAIIIsolate raii_isolate(out); Isolate* isolate = raii_isolate.get(); v8::Locker locker(isolate); @@ -1039,8 +1039,8 @@ ExitCode BuildCodeCacheFromSnapshot(SnapshotData* out, ExitCode SnapshotBuilder::Generate( SnapshotData* out, - const std::vector& args, - const std::vector& exec_args, + const std::vector& args, + const std::vector& exec_args, std::optional builder_script_content, const SnapshotConfig& snapshot_config) { ExitCode code = BuildSnapshotWithoutCodeCache( @@ -1177,8 +1177,8 @@ ExitCode SnapshotBuilder::CreateSnapshot(SnapshotData* out, ExitCode SnapshotBuilder::GenerateAsSource( const char* out_path, - const std::vector& args, - const std::vector& exec_args, + const std::vector& args, + const std::vector& exec_args, const SnapshotConfig& config, bool use_array_literals) { std::string builder_script_content; diff --git a/src/node_task_runner.cc b/src/node_task_runner.cc index efaf6f01b3fbf9..4de7a39dfe16dd 100644 --- a/src/node_task_runner.cc +++ b/src/node_task_runner.cc @@ -348,7 +348,7 @@ void RunTask(const std::shared_ptr& result, // If the "--" flag is not found, it returns an empty optional. // Otherwise, it returns the positional arguments as a single string. // Example: "node -- script.js arg1 arg2" returns "arg1 arg2". -PositionalArgs GetPositionalArgs(const std::vector& args) { +PositionalArgs GetPositionalArgs(const std::vector& args) { // If the "--" flag is not found, return an empty optional // Otherwise, return the positional arguments as a single string if (auto dash_dash = std::find(args.begin(), args.end(), "--"); @@ -358,7 +358,7 @@ PositionalArgs GetPositionalArgs(const std::vector& args) { for (auto it = dash_dash + 1; it != args.end(); ++it) { // SAFETY: The following code is safe because the lifetime of the // arguments is guaranteed to be valid until the end of the task runner. - positional_args.emplace_back(it->c_str(), it->size()); + positional_args.push_back(*it); } return positional_args; } diff --git a/src/node_task_runner.h b/src/node_task_runner.h index ec8f2adf45aa4a..4636b59f9a0d10 100644 --- a/src/node_task_runner.h +++ b/src/node_task_runner.h @@ -82,7 +82,7 @@ FindPackageJson(const std::filesystem::path& cwd); void RunTask(const std::shared_ptr& result, std::string_view command_id, const PositionalArgs& positional_args); -PositionalArgs GetPositionalArgs(const std::vector& args); +PositionalArgs GetPositionalArgs(const std::vector& args); std::string EscapeShell(std::string_view command); } // namespace node::task_runner