From 0ff1eb83d9c3416f9619c1ce2c3a7327efd9bee4 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 31 Dec 2018 01:03:16 +0100 Subject: [PATCH] src: simplify JS Array creation Use `ToV8Value()` where appropriate to reduce duplication and avoid extra `Array::Set()` calls. PR-URL: https://github.com/nodejs/node/pull/25288 Reviewed-By: Gus Caplan Reviewed-By: Joyee Cheung Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- src/node.cc | 30 +++++------------------------- src/node_credentials.cc | 35 ++++++++++------------------------- src/node_crypto.cc | 16 +++------------- src/node_url.cc | 11 +---------- 4 files changed, 19 insertions(+), 73 deletions(-) diff --git a/src/node.cc b/src/node.cc index edd1ca3cd19d34..3fb31134b9ba85 100644 --- a/src/node.cc +++ b/src/node.cc @@ -889,28 +889,15 @@ void SetupProcessObject(Environment* env, #endif // process.argv - Local 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 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 @@ -960,17 +947,10 @@ void SetupProcessObject(Environment* env, const std::vector& preload_modules = env->options()->preload_modules; if (!preload_modules.empty()) { - Local array = Array::New(env->isolate()); - for (unsigned int i = 0; i < preload_modules.size(); ++i) { - Local 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 index 0f202dfb753bf7..82f6ef0dd8c0f3 100644 --- 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& 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 groups(ngroups); - if (ngroups == -1) { - delete[] groups; + ngroups = getgroups(ngroups, groups.data()); + if (ngroups == -1) return env->ThrowErrnoException(errno, "getgroups"); - } - Local 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 array = ToV8Value(env->context(), groups); + if (!array.IsEmpty()) + args.GetReturnValue().Set(array.ToLocalChecked()); } static void SetGroups(const FunctionCallbackInfo& args) { diff --git a/src/node_crypto.cc b/src/node_crypto.cc index c18432b6205105..ab355688888ee3 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -250,22 +250,12 @@ struct CryptoErrorVector : public std::vector { CHECK(!exception_v.IsEmpty()); if (!empty()) { - Local array = Array::New(env->isolate(), size()); - CHECK(!array.IsEmpty()); - - for (const std::string& string : *this) { - const size_t index = &string - &front(); - Local value = - String::NewFromUtf8(env->isolate(), string.data(), - NewStringType::kNormal, string.size()) - .ToLocalChecked(); - array->Set(env->context(), index, value).FromJust(); - } - CHECK(exception_v->IsObject()); Local exception = exception_v.As(); 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 index d028a66dbea542..1841f32aad787c 100644 --- a/src/node_url.cc +++ b/src/node_url.cc @@ -1190,15 +1190,6 @@ inline std::vector FromJSStringArray(Environment* env, return vec; } -inline Local ToJSStringArray(Environment* env, - const std::vector& vec) { - Isolate* isolate = env->isolate(); - Local 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 base_obj) { url_data base; Local 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,