From e5888462f6fbcc9e28e7d6d9eb4b4702242d84ed Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Fri, 9 Nov 2018 11:37:08 +0800
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/24264
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
---
 src/node_process.cc               | 43 ++++++++++---------------------
 test/parallel/test-process-env.js |  6 +++++
 2 files changed, 19 insertions(+), 30 deletions(-)

diff --git a/src/node_process.cc b/src/node_process.cc
index 6c12dcc02b2cd0..74b2b64f3458e1 100644
--- a/src/node_process.cc
+++ b/src/node_process.cc
@@ -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;
@@ -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);
 }
 
diff --git a/test/parallel/test-process-env.js b/test/parallel/test-process-env.js
index 60f4d6f4faf13b..51cc637d06e5c4 100644
--- a/test/parallel/test-process-env.js
+++ b/test/parallel/test-process-env.js
@@ -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) {
@@ -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);
+}