Skip to content

Commit

Permalink
process: remove pushValueToArray in EnvEnumerator
Browse files Browse the repository at this point in the history
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: nodejs#24264
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
  • Loading branch information
joyeecheung authored and kiyomizumia committed Nov 15, 2018
1 parent 524bdb3 commit 41189d5
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 30 deletions.
43 changes: 13 additions & 30 deletions src/node_process.cc
Original file line number Diff line number Diff line change
Expand Up @@ -730,40 +730,29 @@ void EnvDeleter(Local<Name> property,
void EnvEnumerator(const PropertyCallbackInfo<Array>& info) {
Environment* env = Environment::GetCurrent(info);
Isolate* isolate = env->isolate();
Local<Context> ctx = env->context();
Local<Function> fn = env->push_values_to_array_function();
Local<Value> argv[NODE_PUSH_VAL_TO_ARRAY_MAX];
size_t idx = 0;

Mutex::ScopedLock lock(environ_mutex);
Local<Array> envarr;
int env_size = 0;
#ifdef __POSIX__
int size = 0;
while (environ[size])
size++;

Local<Array> envarr = Array::New(isolate);
while (environ[env_size]) {
env_size++;
}
std::vector<Local<Value>> env_v(env_size);

for (int i = 0; i < size; ++i) {
for (int i = 0; i < env_size; ++i) {
const char* var = environ[i];
const char* s = strchr(var, '=');
const int length = s ? s - var : strlen(var);
argv[idx] = String::NewFromUtf8(isolate,
var,
v8::NewStringType::kNormal,
length).ToLocalChecked();
if (++idx >= arraysize(argv)) {
fn->Call(ctx, envarr, idx, argv).ToLocalChecked();
idx = 0;
}
}
if (idx > 0) {
fn->Call(ctx, envarr, idx, argv).ToLocalChecked();
env_v[i] =
String::NewFromUtf8(isolate, var, v8::NewStringType::kNormal, length)
.ToLocalChecked();
}
#else // _WIN32
std::vector<Local<Value>> env_v;
WCHAR* environment = GetEnvironmentStringsW();
if (environment == nullptr)
return; // This should not happen.
Local<Array> envarr = Array::New(isolate);
WCHAR* p = environment;
while (*p) {
WCHAR* s;
Expand All @@ -789,19 +778,13 @@ void EnvEnumerator(const PropertyCallbackInfo<Array>& info) {
FreeEnvironmentStringsW(environment);
return;
}
argv[idx] = rc.ToLocalChecked();
if (++idx >= arraysize(argv)) {
fn->Call(ctx, envarr, idx, argv).ToLocalChecked();
idx = 0;
}
env_v.push_back(rc.ToLocalChecked());
p = s + wcslen(s) + 1;
}
if (idx > 0) {
fn->Call(ctx, envarr, idx, argv).ToLocalChecked();
}
FreeEnvironmentStringsW(environment);
#endif

envarr = Array::New(isolate, env_v.data(), env_v.size());
info.GetReturnValue().Set(envarr);
}

Expand Down
6 changes: 6 additions & 0 deletions test/parallel/test-process-env.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ assert.strictEqual(5, date.getHours());
// case-sensitive on other platforms.
process.env.TEST = 'test';
assert.strictEqual(process.env.TEST, 'test');

// Check both mixed case and lower case, to avoid any regressions that might
// simply convert input to lower case.
if (common.isWindows) {
Expand All @@ -100,3 +101,8 @@ if (common.isWindows) {
assert.strictEqual(process.env.test, undefined);
assert.strictEqual(process.env.teST, undefined);
}

{
const keys = Object.keys(process.env);
assert.ok(keys.length > 0);
}

0 comments on commit 41189d5

Please sign in to comment.