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.

Sign up for GitHub

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

Jump to bottom

src: use a std::vector for preload_modules #12241

Closed
wants to merge 1 commit into from
Closed

src: use a std::vector for preload_modules #12241

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 6 additions & 22 deletions src/node.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,7 @@ static bool throw_deprecation = false;
static bool trace_sync_io = false;
static bool track_heap_objects = false;
static const char* eval_string = nullptr;
static unsigned int preload_module_count = 0;
static const char** preload_modules = nullptr;
static std::vector<std::string> preload_modules;
static const int v8_default_thread_pool_size = 4;
static int v8_thread_pool_size = v8_default_thread_pool_size;
static bool prof_process = false;
Expand Down Expand Up @@ -3279,21 +3278,18 @@ void SetupProcessObject(Environment* env,
READONLY_PROPERTY(process, "_forceRepl", True(env->isolate()));
}

if (preload_module_count) {
CHECK(preload_modules);
if (!preload_modules.empty()) {
Local<Array> array = Array::New(env->isolate());
for (unsigned int i = 0; i < preload_module_count; ++i) {
for (unsigned int i = 0; i < preload_modules.size(); ++i) {
Copy link

Choose a reason for hiding this comment

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

A do-while loop would avoid one unnecessary bounds check. There is at least one element in the vector.

Local<String> module = String::NewFromUtf8(env->isolate(),
preload_modules[i]);
preload_modules[i].c_str());
array->Set(i, module);
}
READONLY_PROPERTY(process,
"_preload_modules",
array);

delete[] preload_modules;
preload_modules = nullptr;
preload_module_count = 0;
preload_modules.clear();
}

// --no-deprecation
Expand Down Expand Up @@ -3651,15 +3647,13 @@ static void ParseArgs(int* argc,
const char** new_exec_argv = new const char*[nargs];
const char** new_v8_argv = new const char*[nargs];
const char** new_argv = new const char*[nargs];
const char** local_preload_modules = new const char*[nargs];
bool use_bundled_ca = false;
bool use_openssl_ca = false;

for (unsigned int i = 0; i < nargs; ++i) {
new_exec_argv[i] = nullptr;
new_v8_argv[i] = nullptr;
new_argv[i] = nullptr;
local_preload_modules[i] = nullptr;
}

// exec_argv starts with the first option, the other two start with argv[0].
Expand Down Expand Up @@ -3717,7 +3711,7 @@ static void ParseArgs(int* argc,
exit(9);
}
args_consumed += 1;
local_preload_modules[preload_module_count++] = module;
preload_modules.push_back(module);
Copy link

Choose a reason for hiding this comment

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

It should be possible to reserve space in the vector before inserting elements.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How? This is only inserting one element, I don't know how many -r options were passed.

} else if (strcmp(arg, "--check") == 0 || strcmp(arg, "-c") == 0) {
syntax_check_only = true;
} else if (strcmp(arg, "--interactive") == 0 || strcmp(arg, "-i") == 0) {
Expand Down Expand Up @@ -3837,16 +3831,6 @@ static void ParseArgs(int* argc,
memcpy(argv, new_argv, new_argc * sizeof(*argv));
delete[] new_argv;
*argc = static_cast<int>(new_argc);

// Copy the preload_modules from the local array to an appropriately sized
// global array.
if (preload_module_count > 0) {
CHECK(!preload_modules);
preload_modules = new const char*[preload_module_count];
memcpy(preload_modules, local_preload_modules,
preload_module_count * sizeof(*preload_modules));
}
delete[] local_preload_modules;
}


Expand Down