Skip to content

Commit

Permalink
src: prefer 3-argument Array::New()
Browse files Browse the repository at this point in the history 
This is nicer, because:

1. It reduces overall code size,
2. It’s faster, because `Object::Set()` calls are relatively slow, and
3. It helps avoid invalid `.Check()`/`.FromJust()` calls.

PR-URL: #31775
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: David Carlier <[email protected]>
  • Loading branch information
@addaleax
addaleax committed Feb 15, 2020
1 parent 79296dc commit a7c523e
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 103 deletions.
30 changes: 14 additions & 16 deletions src/cares_wrap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -749,16 +749,12 @@ Local<Array> AddrTTLToArray(Environment* env,
const T* addrttls,
size_t naddrttls) {
auto isolate = env->isolate();
EscapableHandleScope escapable_handle_scope(isolate);
auto context = env->context();

Local<Array> ttls = Array::New(isolate, naddrttls);
for (size_t i = 0; i < naddrttls; i++) {
auto value = Integer::NewFromUnsigned(isolate, addrttls[i].ttl);
ttls->Set(context, i, value).Check();
}
MaybeStackBuffer<Local<Value>, 8> ttls(naddrttls);
for (size_t i = 0; i < naddrttls; i++)
ttls[i] = Integer::NewFromUnsigned(isolate, addrttls[i].ttl);

return escapable_handle_scope.Escape(ttls);
return Array::New(isolate, ttls.out(), naddrttls);
}


Expand Down Expand Up @@ -2039,6 +2035,7 @@ void GetServers(const FunctionCallbackInfo<Value>& args) {

int r = ares_get_servers_ports(channel->cares_channel(), &servers);
CHECK_EQ(r, ARES_SUCCESS);
auto cleanup = OnScopeLeave([&]() { ares_free_data(servers); });

ares_addr_port_node* cur = servers;

Expand All @@ -2049,17 +2046,18 @@ void GetServers(const FunctionCallbackInfo<Value>& args) {
int err = uv_inet_ntop(cur->family, caddr, ip, sizeof(ip));
CHECK_EQ(err, 0);

Local<Array> ret = Array::New(env->isolate(), 2);
ret->Set(env->context(), 0, OneByteString(env->isolate(), ip)).Check();
ret->Set(env->context(),
1,
Integer::New(env->isolate(), cur->udp_port)).Check();
Local<Value> ret[] = {
OneByteString(env->isolate(), ip),
Integer::New(env->isolate(), cur->udp_port)
};

server_array->Set(env->context(), i, ret).Check();
if (server_array->Set(env->context(), i,
Array::New(env->isolate(), ret, arraysize(ret)))
.IsNothing()) {
return;
}
}

ares_free_data(servers);

args.GetReturnValue().Set(server_array);
}

Expand Down
9 changes: 4 additions & 5 deletions src/js_stream.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,16 +116,15 @@ int JSStream::DoWrite(WriteWrap* w,
HandleScope scope(env()->isolate());
Context::Scope context_scope(env()->context());

Local<Array> bufs_arr = Array::New(env()->isolate(), count);
Local<Object> buf;
MaybeStackBuffer<Local<Value>, 16> bufs_arr(count);
for (size_t i = 0; i < count; i++) {
buf = Buffer::Copy(env(), bufs[i].base, bufs[i].len).ToLocalChecked();
bufs_arr->Set(env()->context(), i, buf).Check();
bufs_arr[i] =
Buffer::Copy(env(), bufs[i].base, bufs[i].len).ToLocalChecked();
}

Local<Value> argv[] = {
w->object(),
bufs_arr
Array::New(env()->isolate(), bufs_arr.out(), count)
};

TryCatchScope try_catch(env());
Expand Down
18 changes: 10 additions & 8 deletions src/module_wrap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,11 +264,11 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
Local<Context> mod_context = obj->context_.Get(isolate);
Local<Module> module = obj->module_.Get(isolate);

Local<Array> promises = Array::New(isolate,
module->GetModuleRequestsLength());
const int module_requests_length = module->GetModuleRequestsLength();
MaybeStackBuffer<Local<Value>, 16> promises(module_requests_length);

// call the dependency resolve callbacks
for (int i = 0; i < module->GetModuleRequestsLength(); i++) {
for (int i = 0; i < module_requests_length; i++) {
Local<String> specifier = module->GetModuleRequest(i);
Utf8Value specifier_utf8(env->isolate(), specifier);
std::string specifier_std(*specifier_utf8, specifier_utf8.length());
Expand All @@ -290,10 +290,11 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
Local<Promise> resolve_promise = resolve_return_value.As<Promise>();
obj->resolve_cache_[specifier_std].Reset(env->isolate(), resolve_promise);

promises->Set(mod_context, i, resolve_promise).Check();
promises[i] = resolve_promise;
}

args.GetReturnValue().Set(promises);
args.GetReturnValue().Set(
Array::New(isolate, promises.out(), promises.length()));
}

void ModuleWrap::Instantiate(const FunctionCallbackInfo<Value>& args) {
Expand Down Expand Up @@ -426,12 +427,13 @@ void ModuleWrap::GetStaticDependencySpecifiers(

int count = module->GetModuleRequestsLength();

Local<Array> specifiers = Array::New(env->isolate(), count);
MaybeStackBuffer<Local<Value>, 16> specifiers(count);

for (int i = 0; i < count; i++)
specifiers->Set(env->context(), i, module->GetModuleRequest(i)).Check();
specifiers[i] = module->GetModuleRequest(i);

args.GetReturnValue().Set(specifiers);
args.GetReturnValue().Set(
Array::New(env->isolate(), specifiers.out(), count));
}

void ModuleWrap::GetError(const FunctionCallbackInfo<Value>& args) {
Expand Down
78 changes: 34 additions & 44 deletions src/node_crypto.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1011,19 +1011,19 @@ static X509_STORE* NewRootCertStore() {

void GetRootCertificates(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Local<Array> result = Array::New(env->isolate(), arraysize(root_certs));
Local<Value> result[arraysize(root_certs)];

for (size_t i = 0; i < arraysize(root_certs); i++) {
Local<Value> value;
if (!String::NewFromOneByte(env->isolate(),
reinterpret_cast<const uint8_t*>(root_certs[i]),
NewStringType::kNormal).ToLocal(&value) ||
!result->Set(env->context(), i, value).FromMaybe(false)) {
if (!String::NewFromOneByte(
env->isolate(),
reinterpret_cast<const uint8_t*>(root_certs[i]),
NewStringType::kNormal).ToLocal(&result[i])) {
return;
}
}

args.GetReturnValue().Set(result);
args.GetReturnValue().Set(
Array::New(env->isolate(), result, arraysize(root_certs)));
}


Expand Down Expand Up @@ -2138,22 +2138,22 @@ static Local<Object> X509ToObject(Environment* env, X509* cert) {
StackOfASN1 eku(static_cast<STACK_OF(ASN1_OBJECT)*>(
X509_get_ext_d2i(cert, NID_ext_key_usage, nullptr, nullptr)));
if (eku) {
Local<Array> ext_key_usage = Array::New(env->isolate());
const int count = sk_ASN1_OBJECT_num(eku.get());
MaybeStackBuffer<Local<Value>, 16> ext_key_usage(count);
char buf[256];

int j = 0;
for (int i = 0; i < sk_ASN1_OBJECT_num(eku.get()); i++) {
for (int i = 0; i < count; i++) {
if (OBJ_obj2txt(buf,
sizeof(buf),
sk_ASN1_OBJECT_value(eku.get(), i), 1) >= 0) {
ext_key_usage->Set(context,
j++,
OneByteString(env->isolate(), buf)).Check();
ext_key_usage[j++] = OneByteString(env->isolate(), buf);
}
}

eku.reset();
info->Set(context, env->ext_key_usage_string(), ext_key_usage).Check();
info->Set(context, env->ext_key_usage_string(),
Array::New(env->isolate(), ext_key_usage.out(), count)).Check();
}

if (ASN1_INTEGER* serial_number = X509_get_serialNumber(cert)) {
Expand Down Expand Up @@ -6799,15 +6799,8 @@ void GenerateKeyPair(const FunctionCallbackInfo<Value>& args,
Local<Value> err, pubkey, privkey;
job->ToResult(&err, &pubkey, &privkey);

bool (*IsNotTrue)(Maybe<bool>) = [](Maybe<bool> maybe) {
return maybe.IsNothing() || !maybe.ToChecked();
};
Local<Array> ret = Array::New(env->isolate(), 3);
if (IsNotTrue(ret->Set(env->context(), 0, err)) ||
IsNotTrue(ret->Set(env->context(), 1, pubkey)) ||
IsNotTrue(ret->Set(env->context(), 2, privkey)))
return;
args.GetReturnValue().Set(ret);
Local<Value> ret[] = { err, pubkey, privkey };
args.GetReturnValue().Set(Array::New(env->isolate(), ret, arraysize(ret)));
}

void GenerateKeyPairRSA(const FunctionCallbackInfo<Value>& args) {
Expand Down Expand Up @@ -6940,17 +6933,6 @@ void GetSSLCiphers(const FunctionCallbackInfo<Value>& args) {
CHECK(ssl);

STACK_OF(SSL_CIPHER)* ciphers = SSL_get_ciphers(ssl.get());
int n = sk_SSL_CIPHER_num(ciphers);
Local<Array> arr = Array::New(env->isolate(), n);

for (int i = 0; i < n; ++i) {
const SSL_CIPHER* cipher = sk_SSL_CIPHER_value(ciphers, i);
arr->Set(env->context(),
i,
OneByteString(args.GetIsolate(),
SSL_CIPHER_get_name(cipher))).Check();
}

// TLSv1.3 ciphers aren't listed by EVP. There are only 5, we could just
// document them, but since there are only 5, easier to just add them manually
// and not have to explain their absence in the API docs. They are lower-cased
Expand All @@ -6963,13 +6945,20 @@ void GetSSLCiphers(const FunctionCallbackInfo<Value>& args) {
"tls_aes_128_ccm_sha256"
};

const int n = sk_SSL_CIPHER_num(ciphers);
std::vector<Local<Value>> arr(n + arraysize(TLS13_CIPHERS));

for (int i = 0; i < n; ++i) {
const SSL_CIPHER* cipher = sk_SSL_CIPHER_value(ciphers, i);
arr[i] = OneByteString(env->isolate(), SSL_CIPHER_get_name(cipher));
}

for (unsigned i = 0; i < arraysize(TLS13_CIPHERS); ++i) {
const char* name = TLS13_CIPHERS[i];
arr->Set(env->context(),
arr->Length(), OneByteString(args.GetIsolate(), name)).Check();
arr[n + i] = OneByteString(env->isolate(), name);
}

args.GetReturnValue().Set(arr);
args.GetReturnValue().Set(Array::New(env->isolate(), arr.data(), arr.size()));
}


Expand Down Expand Up @@ -7020,22 +7009,23 @@ void GetHashes(const FunctionCallbackInfo<Value>& args) {
void GetCurves(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
const size_t num_curves = EC_get_builtin_curves(nullptr, 0);
Local<Array> arr = Array::New(env->isolate(), num_curves);

if (num_curves) {
std::vector<EC_builtin_curve> curves(num_curves);

if (EC_get_builtin_curves(curves.data(), num_curves)) {
for (size_t i = 0; i < num_curves; i++) {
arr->Set(env->context(),
i,
OneByteString(env->isolate(),
OBJ_nid2sn(curves[i].nid))).Check();
}
std::vector<Local<Value>> arr(num_curves);

for (size_t i = 0; i < num_curves; i++)
arr[i] = OneByteString(env->isolate(), OBJ_nid2sn(curves[i].nid));

args.GetReturnValue().Set(
Array::New(env->isolate(), arr.data(), arr.size()));
return;
}
}

args.GetReturnValue().Set(arr);
args.GetReturnValue().Set(Array::New(env->isolate()));
}


Expand Down
27 changes: 10 additions & 17 deletions src/node_file.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -748,16 +748,11 @@ void AfterScanDirWithTypes(uv_fs_t* req) {
type_v.emplace_back(Integer::New(isolate, ent.type));
}

Local<Array> result = Array::New(isolate, 2);
result->Set(env->context(),
0,
Array::New(isolate, name_v.data(),
name_v.size())).Check();
result->Set(env->context(),
1,
Array::New(isolate, type_v.data(),
type_v.size())).Check();
req_wrap->Resolve(result);
Local<Value> result[] = {
Array::New(isolate, name_v.data(), name_v.size()),
Array::New(isolate, type_v.data(), type_v.size())
};
req_wrap->Resolve(Array::New(isolate, result, arraysize(result)));
}

void Access(const FunctionCallbackInfo<Value>& args) {
Expand Down Expand Up @@ -1611,13 +1606,11 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {

Local<Array> names = Array::New(isolate, name_v.data(), name_v.size());
if (with_types) {
Local<Array> result = Array::New(isolate, 2);
result->Set(env->context(), 0, names).Check();
result->Set(env->context(),
1,
Array::New(isolate, type_v.data(),
type_v.size())).Check();
args.GetReturnValue().Set(result);
Local<Value> result[] = {
names,
Array::New(isolate, type_v.data(), type_v.size())
};
args.GetReturnValue().Set(Array::New(isolate, result, arraysize(result)));
} else {
args.GetReturnValue().Set(names);
}
Expand Down
16 changes: 8 additions & 8 deletions src/node_v8.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,19 +218,19 @@ void Initialize(Local<Object> target,
// Heap space names are extracted once and exposed to JavaScript to
// avoid excessive creation of heap space name Strings.
HeapSpaceStatistics s;
const Local<Array> heap_spaces = Array::New(env->isolate(),
number_of_heap_spaces);
MaybeStackBuffer<Local<Value>, 16> heap_spaces(number_of_heap_spaces);
for (size_t i = 0; i < number_of_heap_spaces; i++) {
env->isolate()->GetHeapSpaceStatistics(&s, i);
Local<String> heap_space_name = String::NewFromUtf8(env->isolate(),
s.space_name(),
NewStringType::kNormal)
.ToLocalChecked();
heap_spaces->Set(env->context(), i, heap_space_name).Check();
heap_spaces[i] = String::NewFromUtf8(env->isolate(),
s.space_name(),
NewStringType::kNormal)
.ToLocalChecked();
}
target->Set(env->context(),
FIXED_ONE_BYTE_STRING(env->isolate(), "kHeapSpaces"),
heap_spaces).Check();
Array::New(env->isolate(),
heap_spaces.out(),
number_of_heap_spaces)).Check();

env->SetMethod(target,
"updateHeapSpaceStatisticsArrayBuffer",
Expand Down
10 changes: 5 additions & 5 deletions src/spawn_sync.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -721,18 +721,18 @@ Local<Array> SyncProcessRunner::BuildOutputArray() {
CHECK(!stdio_pipes_.empty());

EscapableHandleScope scope(env()->isolate());
Local<Context> context = env()->context();
Local<Array> js_output = Array::New(env()->isolate(), stdio_count_);
MaybeStackBuffer<Local<Value>, 8> js_output(stdio_pipes_.size());

for (uint32_t i = 0; i < stdio_pipes_.size(); i++) {
SyncProcessStdioPipe* h = stdio_pipes_[i].get();
if (h != nullptr && h->writable())
js_output->Set(context, i, h->GetOutputAsBuffer(env())).Check();
js_output[i] = h->GetOutputAsBuffer(env());
else
js_output->Set(context, i, Null(env()->isolate())).Check();
js_output[i] = Null(env()->isolate());
}

return scope.Escape(js_output);
return scope.Escape(
Array::New(env()->isolate(), js_output.out(), js_output.length()));
}

Maybe<int> SyncProcessRunner::ParseOptions(Local<Value> js_value) {
Expand Down

0 comments on commit a7c523e

Please sign in to comment.