Skip to content

Commit 5c8e9eb

Browse files
Anna Henningsenrefack
Anna Henningsen
authored andcommitted
src: simplify JS Array creation
Use `ToV8Value()` where appropriate to reduce duplication and avoid extra `Array::Set()` calls. PR-URL: nodejs#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>
1 parent 98a9544 commit 5c8e9eb

File tree

4 files changed

+19
-73
lines changed

4 files changed

+19
-73
lines changed

src/node.cc

+5-25
Original file line numberDiff line numberDiff line change
@@ -889,28 +889,15 @@ void SetupProcessObject(Environment* env,
889889
#endif
890890

891891
// process.argv
892-
Local<Array> arguments = Array::New(env->isolate(), args.size());
893-
for (size_t i = 0; i < args.size(); ++i) {
894-
arguments->Set(env->context(), i,
895-
String::NewFromUtf8(env->isolate(), args[i].c_str(),
896-
NewStringType::kNormal).ToLocalChecked())
897-
.FromJust();
898-
}
899892
process->Set(env->context(),
900893
FIXED_ONE_BYTE_STRING(env->isolate(), "argv"),
901-
arguments).FromJust();
894+
ToV8Value(env->context(), args).ToLocalChecked()).FromJust();
902895

903896
// process.execArgv
904-
Local<Array> exec_arguments = Array::New(env->isolate(), exec_args.size());
905-
for (size_t i = 0; i < exec_args.size(); ++i) {
906-
exec_arguments->Set(env->context(), i,
907-
String::NewFromUtf8(env->isolate(), exec_args[i].c_str(),
908-
NewStringType::kNormal).ToLocalChecked())
909-
.FromJust();
910-
}
911897
process->Set(env->context(),
912898
FIXED_ONE_BYTE_STRING(env->isolate(), "execArgv"),
913-
exec_arguments).FromJust();
899+
ToV8Value(env->context(), exec_args)
900+
.ToLocalChecked()).FromJust();
914901

915902
// create process.env
916903
process
@@ -960,17 +947,10 @@ void SetupProcessObject(Environment* env,
960947
const std::vector<std::string>& preload_modules =
961948
env->options()->preload_modules;
962949
if (!preload_modules.empty()) {
963-
Local<Array> array = Array::New(env->isolate());
964-
for (unsigned int i = 0; i < preload_modules.size(); ++i) {
965-
Local<String> module = String::NewFromUtf8(env->isolate(),
966-
preload_modules[i].c_str(),
967-
NewStringType::kNormal)
968-
.ToLocalChecked();
969-
array->Set(env->context(), i, module).FromJust();
970-
}
971950
READONLY_PROPERTY(process,
972951
"_preload_modules",
973-
array);
952+
ToV8Value(env->context(), preload_modules)
953+
.ToLocalChecked());
974954
}
975955

976956
// --no-deprecation

src/node_credentials.cc

+10-25
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@ using v8::Array;
1515
using v8::Context;
1616
using v8::Function;
1717
using v8::FunctionCallbackInfo;
18-
using v8::Integer;
1918
using v8::Isolate;
2019
using v8::Local;
20+
using v8::MaybeLocal;
2121
using v8::Object;
2222
using v8::String;
2323
using v8::Uint32;
@@ -253,36 +253,21 @@ static void GetGroups(const FunctionCallbackInfo<Value>& args) {
253253
Environment* env = Environment::GetCurrent(args);
254254

255255
int ngroups = getgroups(0, nullptr);
256-
257256
if (ngroups == -1) return env->ThrowErrnoException(errno, "getgroups");
258257

259-
gid_t* groups = new gid_t[ngroups];
260-
261-
ngroups = getgroups(ngroups, groups);
258+
std::vector<gid_t> groups(ngroups);
262259

263-
if (ngroups == -1) {
264-
delete[] groups;
260+
ngroups = getgroups(ngroups, groups.data());
261+
if (ngroups == -1)
265262
return env->ThrowErrnoException(errno, "getgroups");
266-
}
267263

268-
Local<Array> groups_list = Array::New(env->isolate(), ngroups);
269-
bool seen_egid = false;
264+
groups.resize(ngroups);
270265
gid_t egid = getegid();
271-
272-
for (int i = 0; i < ngroups; i++) {
273-
groups_list->Set(env->context(), i, Integer::New(env->isolate(), groups[i]))
274-
.FromJust();
275-
if (groups[i] == egid) seen_egid = true;
276-
}
277-
278-
delete[] groups;
279-
280-
if (seen_egid == false)
281-
groups_list
282-
->Set(env->context(), ngroups, Integer::New(env->isolate(), egid))
283-
.FromJust();
284-
285-
args.GetReturnValue().Set(groups_list);
266+
if (std::find(groups.begin(), groups.end(), egid) == groups.end())
267+
groups.push_back(egid);
268+
MaybeLocal<Value> array = ToV8Value(env->context(), groups);
269+
if (!array.IsEmpty())
270+
args.GetReturnValue().Set(array.ToLocalChecked());
286271
}
287272

288273
static void SetGroups(const FunctionCallbackInfo<Value>& args) {

src/node_crypto.cc

+3-13
Original file line numberDiff line numberDiff line change
@@ -250,22 +250,12 @@ struct CryptoErrorVector : public std::vector<std::string> {
250250
CHECK(!exception_v.IsEmpty());
251251

252252
if (!empty()) {
253-
Local<Array> array = Array::New(env->isolate(), size());
254-
CHECK(!array.IsEmpty());
255-
256-
for (const std::string& string : *this) {
257-
const size_t index = &string - &front();
258-
Local<String> value =
259-
String::NewFromUtf8(env->isolate(), string.data(),
260-
NewStringType::kNormal, string.size())
261-
.ToLocalChecked();
262-
array->Set(env->context(), index, value).FromJust();
263-
}
264-
265253
CHECK(exception_v->IsObject());
266254
Local<Object> exception = exception_v.As<Object>();
267255
exception->Set(env->context(),
268-
env->openssl_error_stack(), array).FromJust();
256+
env->openssl_error_stack(),
257+
ToV8Value(env->context(), *this).ToLocalChecked())
258+
.FromJust();
269259
}
270260

271261
return exception_v;

src/node_url.cc

+1-10
Original file line numberDiff line numberDiff line change
@@ -1190,15 +1190,6 @@ inline std::vector<std::string> FromJSStringArray(Environment* env,
11901190
return vec;
11911191
}
11921192

1193-
inline Local<Array> ToJSStringArray(Environment* env,
1194-
const std::vector<std::string>& vec) {
1195-
Isolate* isolate = env->isolate();
1196-
Local<Array> array = Array::New(isolate, vec.size());
1197-
for (size_t n = 0; n < vec.size(); n++)
1198-
array->Set(env->context(), n, Utf8String(isolate, vec[n])).FromJust();
1199-
return array;
1200-
}
1201-
12021193
inline url_data HarvestBase(Environment* env, Local<Object> base_obj) {
12031194
url_data base;
12041195
Local<Context> context = env->context();
@@ -2119,7 +2110,7 @@ static inline void SetArgs(Environment* env,
21192110
if (url.port > -1)
21202111
argv[ARG_PORT] = Integer::New(isolate, url.port);
21212112
if (url.flags & URL_FLAGS_HAS_PATH)
2122-
argv[ARG_PATH] = ToJSStringArray(env, url.path);
2113+
argv[ARG_PATH] = ToV8Value(env->context(), url.path).ToLocalChecked();
21232114
}
21242115

21252116
static void Parse(Environment* env,

0 commit comments

Comments
 (0)