Skip to content

Commit

Permalink
src: simplify JS Array creation
Browse files Browse the repository at this point in the history
Use `ToV8Value()` where appropriate to reduce duplication
and avoid extra `Array::Set()` calls.

PR-URL: #25288
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
addaleax committed Jan 2, 2019
1 parent 112ee2a commit 0ff1eb8
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 73 deletions.
30 changes: 5 additions & 25 deletions src/node.cc
Expand Up @@ -889,28 +889,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
Expand Down Expand Up @@ -960,17 +947,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
Expand Down
35 changes: 10 additions & 25 deletions src/node_credentials.cc
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
16 changes: 3 additions & 13 deletions src/node_crypto.cc
Expand Up @@ -250,22 +250,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;
Expand Down
11 changes: 1 addition & 10 deletions src/node_url.cc
Expand Up @@ -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();
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 0ff1eb8

Please sign in to comment.