src: simplify JS Array creation

Use `ToV8Value()` where appropriate to reduce duplication and avoid extra `Array::Set()` calls. PR-URL: #25288 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Jan 14, 2019 · 43535f5 · 43535f5
1 parent 6b5c962
commit 43535f5
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 73 deletions.
diff --git a/src/node.cc b/src/node.cc
@@ -938,28 +938,15 @@ void SetupProcessObject(Environment* env,
 #endif
 
   // process.argv
-  Local<Array> arguments = Array::New(env->isolate(), args.size());
-  for (size_t i = 0; i < args.size(); ++i) {
-    arguments->Set(env->context(), i,
-        String::NewFromUtf8(env->isolate(), args[i].c_str(),
-                            NewStringType::kNormal).ToLocalChecked())
-        .FromJust();
-  }
   process->Set(env->context(),
                FIXED_ONE_BYTE_STRING(env->isolate(), "argv"),
-               arguments).FromJust();
+               ToV8Value(env->context(), args).ToLocalChecked()).FromJust();
 
   // process.execArgv
-  Local<Array> exec_arguments = Array::New(env->isolate(), exec_args.size());
-  for (size_t i = 0; i < exec_args.size(); ++i) {
-    exec_arguments->Set(env->context(), i,
-        String::NewFromUtf8(env->isolate(), exec_args[i].c_str(),
-                            NewStringType::kNormal).ToLocalChecked())
-        .FromJust();
-  }
   process->Set(env->context(),
                FIXED_ONE_BYTE_STRING(env->isolate(), "execArgv"),
-               exec_arguments).FromJust();
+               ToV8Value(env->context(), exec_args)
+                   .ToLocalChecked()).FromJust();
 
   // create process.env
   process
@@ -1010,17 +997,10 @@ void SetupProcessObject(Environment* env,
   const std::vector<std::string>& preload_modules =
       env->options()->preload_modules;
   if (!preload_modules.empty()) {
-    Local<Array> array = Array::New(env->isolate());
-    for (unsigned int i = 0; i < preload_modules.size(); ++i) {
-      Local<String> module = String::NewFromUtf8(env->isolate(),
-                                                 preload_modules[i].c_str(),
-                                                 NewStringType::kNormal)
-                                 .ToLocalChecked();
-      array->Set(env->context(), i, module).FromJust();
-    }
     READONLY_PROPERTY(process,
                       "_preload_modules",
-                      array);
+                      ToV8Value(env->context(), preload_modules)
+                          .ToLocalChecked());
   }
 
   // --no-deprecation

diff --git a/src/node_credentials.cc b/src/node_credentials.cc
@@ -15,9 +15,9 @@ using v8::Array;
 using v8::Context;
 using v8::Function;
 using v8::FunctionCallbackInfo;
-using v8::Integer;
 using v8::Isolate;
 using v8::Local;
+using v8::MaybeLocal;
 using v8::Object;
 using v8::String;
 using v8::Uint32;
@@ -253,36 +253,21 @@ static void GetGroups(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
   int ngroups = getgroups(0, nullptr);
-
   if (ngroups == -1) return env->ThrowErrnoException(errno, "getgroups");
 
-  gid_t* groups = new gid_t[ngroups];
-
-  ngroups = getgroups(ngroups, groups);
+  std::vector<gid_t> groups(ngroups);
 
-  if (ngroups == -1) {
-    delete[] groups;
+  ngroups = getgroups(ngroups, groups.data());
+  if (ngroups == -1)
     return env->ThrowErrnoException(errno, "getgroups");
-  }
 
-  Local<Array> groups_list = Array::New(env->isolate(), ngroups);
-  bool seen_egid = false;
+  groups.resize(ngroups);
   gid_t egid = getegid();
-
-  for (int i = 0; i < ngroups; i++) {
-    groups_list->Set(env->context(), i, Integer::New(env->isolate(), groups[i]))
-        .FromJust();
-    if (groups[i] == egid) seen_egid = true;
-  }
-
-  delete[] groups;
-
-  if (seen_egid == false)
-    groups_list
-        ->Set(env->context(), ngroups, Integer::New(env->isolate(), egid))
-        .FromJust();
-
-  args.GetReturnValue().Set(groups_list);
+  if (std::find(groups.begin(), groups.end(), egid) == groups.end())
+    groups.push_back(egid);
+  MaybeLocal<Value> array = ToV8Value(env->context(), groups);
+  if (!array.IsEmpty())
+    args.GetReturnValue().Set(array.ToLocalChecked());
 }
 
 static void SetGroups(const FunctionCallbackInfo<Value>& args) {

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -248,22 +248,12 @@ struct CryptoErrorVector : public std::vector<std::string> {
     CHECK(!exception_v.IsEmpty());
 
     if (!empty()) {
-      Local<Array> array = Array::New(env->isolate(), size());
-      CHECK(!array.IsEmpty());
-
-      for (const std::string& string : *this) {
-        const size_t index = &string - &front();
-        Local<String> value =
-            String::NewFromUtf8(env->isolate(), string.data(),
-                                NewStringType::kNormal, string.size())
-            .ToLocalChecked();
-        array->Set(env->context(), index, value).FromJust();
-      }
-
       CHECK(exception_v->IsObject());
       Local<Object> exception = exception_v.As<Object>();
       exception->Set(env->context(),
-                     env->openssl_error_stack(), array).FromJust();
+                     env->openssl_error_stack(),
+                     ToV8Value(env->context(), *this).ToLocalChecked())
+          .FromJust();
     }
 
     return exception_v;

diff --git a/src/node_url.cc b/src/node_url.cc
@@ -1190,15 +1190,6 @@ inline std::vector<std::string> FromJSStringArray(Environment* env,
   return vec;
 }
 
-inline Local<Array> ToJSStringArray(Environment* env,
-                                    const std::vector<std::string>& vec) {
-  Isolate* isolate = env->isolate();
-  Local<Array> array = Array::New(isolate, vec.size());
-  for (size_t n = 0; n < vec.size(); n++)
-    array->Set(env->context(), n, Utf8String(isolate, vec[n])).FromJust();
-  return array;
-}
-
 inline url_data HarvestBase(Environment* env, Local<Object> base_obj) {
   url_data base;
   Local<Context> context = env->context();
@@ -2119,7 +2110,7 @@ static inline void SetArgs(Environment* env,
   if (url.port > -1)
     argv[ARG_PORT] = Integer::New(isolate, url.port);
   if (url.flags & URL_FLAGS_HAS_PATH)
-    argv[ARG_PATH] = ToJSStringArray(env, url.path);
+    argv[ARG_PATH] = ToV8Value(env->context(), url.path).ToLocalChecked();
 }
 
 static void Parse(Environment* env,