Skip to content

Commit e588846

Browse files
joyeecheungBridgeAR
authored andcommitted
process: remove pushValueToArray in EnvEnumerator
Instead of calling into JS from C++ to push values into an array, use the new Array::New API that takes a pointer and a length directly. PR-URL: #24264 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
1 parent 86aa27f commit e588846

File tree

2 files changed

+19
-30
lines changed

2 files changed

+19
-30
lines changed

src/node_process.cc

+13-30
Original file line numberDiff line numberDiff line change
@@ -730,40 +730,29 @@ void EnvDeleter(Local<Name> property,
730730
void EnvEnumerator(const PropertyCallbackInfo<Array>& info) {
731731
Environment* env = Environment::GetCurrent(info);
732732
Isolate* isolate = env->isolate();
733-
Local<Context> ctx = env->context();
734-
Local<Function> fn = env->push_values_to_array_function();
735-
Local<Value> argv[NODE_PUSH_VAL_TO_ARRAY_MAX];
736-
size_t idx = 0;
737733

738734
Mutex::ScopedLock lock(environ_mutex);
735+
Local<Array> envarr;
736+
int env_size = 0;
739737
#ifdef __POSIX__
740-
int size = 0;
741-
while (environ[size])
742-
size++;
743-
744-
Local<Array> envarr = Array::New(isolate);
738+
while (environ[env_size]) {
739+
env_size++;
740+
}
741+
std::vector<Local<Value>> env_v(env_size);
745742

746-
for (int i = 0; i < size; ++i) {
743+
for (int i = 0; i < env_size; ++i) {
747744
const char* var = environ[i];
748745
const char* s = strchr(var, '=');
749746
const int length = s ? s - var : strlen(var);
750-
argv[idx] = String::NewFromUtf8(isolate,
751-
var,
752-
v8::NewStringType::kNormal,
753-
length).ToLocalChecked();
754-
if (++idx >= arraysize(argv)) {
755-
fn->Call(ctx, envarr, idx, argv).ToLocalChecked();
756-
idx = 0;
757-
}
758-
}
759-
if (idx > 0) {
760-
fn->Call(ctx, envarr, idx, argv).ToLocalChecked();
747+
env_v[i] =
748+
String::NewFromUtf8(isolate, var, v8::NewStringType::kNormal, length)
749+
.ToLocalChecked();
761750
}
762751
#else // _WIN32
752+
std::vector<Local<Value>> env_v;
763753
WCHAR* environment = GetEnvironmentStringsW();
764754
if (environment == nullptr)
765755
return; // This should not happen.
766-
Local<Array> envarr = Array::New(isolate);
767756
WCHAR* p = environment;
768757
while (*p) {
769758
WCHAR* s;
@@ -789,19 +778,13 @@ void EnvEnumerator(const PropertyCallbackInfo<Array>& info) {
789778
FreeEnvironmentStringsW(environment);
790779
return;
791780
}
792-
argv[idx] = rc.ToLocalChecked();
793-
if (++idx >= arraysize(argv)) {
794-
fn->Call(ctx, envarr, idx, argv).ToLocalChecked();
795-
idx = 0;
796-
}
781+
env_v.push_back(rc.ToLocalChecked());
797782
p = s + wcslen(s) + 1;
798783
}
799-
if (idx > 0) {
800-
fn->Call(ctx, envarr, idx, argv).ToLocalChecked();
801-
}
802784
FreeEnvironmentStringsW(environment);
803785
#endif
804786

787+
envarr = Array::New(isolate, env_v.data(), env_v.size());
805788
info.GetReturnValue().Set(envarr);
806789
}
807790

test/parallel/test-process-env.js

+6
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ assert.strictEqual(5, date.getHours());
9191
// case-sensitive on other platforms.
9292
process.env.TEST = 'test';
9393
assert.strictEqual(process.env.TEST, 'test');
94+
9495
// Check both mixed case and lower case, to avoid any regressions that might
9596
// simply convert input to lower case.
9697
if (common.isWindows) {
@@ -100,3 +101,8 @@ if (common.isWindows) {
100101
assert.strictEqual(process.env.test, undefined);
101102
assert.strictEqual(process.env.teST, undefined);
102103
}
104+
105+
{
106+
const keys = Object.keys(process.env);
107+
assert.ok(keys.length > 0);
108+
}

0 commit comments

Comments
 (0)