Skip to content

Commit 17d59ef

Browse files
jasnelladuh95
jasnell
authored and
aduh95
committed
src: minor cleanups on OneByteString usage
* Provide a OneByteString variant that accepts std::string_view * Use FIXED_ONE_BYTE_STRING in appropriate places Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #56482 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent 3e6e010 commit 17d59ef

18 files changed

+92
-74
lines changed

‎src/crypto/crypto_ec.cc

+4-4
Original file line numberDiff line numberDiff line change
@@ -767,16 +767,16 @@ Maybe<void> ExportJWKEcKey(Environment* env,
767767
const int nid = EC_GROUP_get_curve_name(group);
768768
switch (nid) {
769769
case NID_X9_62_prime256v1:
770-
crv_name = OneByteString(env->isolate(), "P-256");
770+
crv_name = FIXED_ONE_BYTE_STRING(env->isolate(), "P-256");
771771
break;
772772
case NID_secp256k1:
773-
crv_name = OneByteString(env->isolate(), "secp256k1");
773+
crv_name = FIXED_ONE_BYTE_STRING(env->isolate(), "secp256k1");
774774
break;
775775
case NID_secp384r1:
776-
crv_name = OneByteString(env->isolate(), "P-384");
776+
crv_name = FIXED_ONE_BYTE_STRING(env->isolate(), "P-384");
777777
break;
778778
case NID_secp521r1:
779-
crv_name = OneByteString(env->isolate(), "P-521");
779+
crv_name = FIXED_ONE_BYTE_STRING(env->isolate(), "P-521");
780780
break;
781781
default: {
782782
THROW_ERR_CRYPTO_JWK_UNSUPPORTED_CURVE(

‎src/crypto/crypto_hash.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ void Hash::GetCachedAliases(const FunctionCallbackInfo<Value>& args) {
152152
names.reserve(size);
153153
values.reserve(size);
154154
for (auto& [alias, id] : env->alias_to_md_id_map) {
155-
names.push_back(OneByteString(isolate, alias.c_str(), alias.size()));
155+
names.push_back(OneByteString(isolate, alias));
156156
values.push_back(v8::Uint32::New(isolate, id));
157157
}
158158
#else

‎src/crypto/crypto_tls.cc

+2-3
Original file line numberDiff line numberDiff line change
@@ -859,8 +859,7 @@ void TLSWrap::ClearOut() {
859859
if (context.IsEmpty()) [[unlikely]]
860860
return;
861861
const std::string error_str = GetBIOError();
862-
Local<String> message = OneByteString(
863-
env()->isolate(), error_str.c_str(), error_str.size());
862+
Local<String> message = OneByteString(env()->isolate(), error_str);
864863
if (message.IsEmpty()) [[unlikely]]
865864
return;
866865
error = Exception::Error(message);
@@ -1974,7 +1973,7 @@ void TLSWrap::GetSharedSigalgs(const FunctionCallbackInfo<Value>& args) {
19741973
} else {
19751974
sig_with_md += "UNDEF";
19761975
}
1977-
ret_arr[i] = OneByteString(env->isolate(), sig_with_md.c_str());
1976+
ret_arr[i] = OneByteString(env->isolate(), sig_with_md);
19781977
}
19791978

19801979
args.GetReturnValue().Set(

‎src/histogram.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ Local<FunctionTemplate> IntervalHistogram::GetConstructorTemplate(
341341
Isolate* isolate = env->isolate();
342342
tmpl = NewFunctionTemplate(isolate, nullptr);
343343
tmpl->Inherit(HandleWrap::GetConstructorTemplate(env));
344-
tmpl->SetClassName(OneByteString(isolate, "Histogram"));
344+
tmpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "Histogram"));
345345
auto instance = tmpl->InstanceTemplate();
346346
instance->SetInternalFieldCount(HistogramImpl::kInternalFieldCount);
347347
HistogramImpl::AddMethods(isolate, tmpl);

‎src/inspector_js_api.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ void Url(const FunctionCallbackInfo<Value>& args) {
333333
if (url.empty()) {
334334
return;
335335
}
336-
args.GetReturnValue().Set(OneByteString(env->isolate(), url.c_str()));
336+
args.GetReturnValue().Set(OneByteString(env->isolate(), url));
337337
}
338338

339339
void Initialize(Local<Object> target, Local<Value> unused,

‎src/node_builtins.cc

+8-9
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ void BuiltinLoader::GetNatives(Local<Name> property,
7878
Local<Object> out = Object::New(isolate);
7979
auto source = env->builtin_loader()->source_.read();
8080
for (auto const& x : *source) {
81-
Local<String> key = OneByteString(isolate, x.first.c_str(), x.first.size());
81+
Local<String> key = OneByteString(isolate, x.first);
8282
out->Set(context, key, x.second.ToStringChecked(isolate)).FromJust();
8383
}
8484
info.GetReturnValue().Set(out);
@@ -206,7 +206,7 @@ MaybeLocal<String> BuiltinLoader::LoadBuiltinSource(Isolate* isolate,
206206
uv_err_name(r),
207207
uv_strerror(r),
208208
filename);
209-
Local<String> message = OneByteString(isolate, buf.c_str());
209+
Local<String> message = OneByteString(isolate, buf);
210210
isolate->ThrowException(v8::Exception::Error(message));
211211
return MaybeLocal<String>();
212212
}
@@ -276,8 +276,7 @@ MaybeLocal<Function> BuiltinLoader::LookupAndCompileInternal(
276276
}
277277

278278
std::string filename_s = std::string("node:") + id;
279-
Local<String> filename =
280-
OneByteString(isolate, filename_s.c_str(), filename_s.size());
279+
Local<String> filename = OneByteString(isolate, filename_s);
281280
ScriptOrigin origin(filename, 0, 0, true);
282281

283282
BuiltinCodeCacheData cached_data{};
@@ -594,7 +593,7 @@ void BuiltinLoader::GetBuiltinCategories(
594593
return;
595594
if (result
596595
->Set(context,
597-
OneByteString(isolate, "cannotBeRequired"),
596+
FIXED_ONE_BYTE_STRING(isolate, "cannotBeRequired"),
598597
cannot_be_required_js)
599598
.IsNothing())
600599
return;
@@ -603,7 +602,7 @@ void BuiltinLoader::GetBuiltinCategories(
603602
return;
604603
if (result
605604
->Set(context,
606-
OneByteString(isolate, "canBeRequired"),
605+
FIXED_ONE_BYTE_STRING(isolate, "canBeRequired"),
607606
can_be_required_js)
608607
.IsNothing()) {
609608
return;
@@ -626,7 +625,7 @@ void BuiltinLoader::GetCacheUsage(const FunctionCallbackInfo<Value>& args) {
626625
}
627626
if (result
628627
->Set(context,
629-
OneByteString(isolate, "compiledWithCache"),
628+
FIXED_ONE_BYTE_STRING(isolate, "compiledWithCache"),
630629
builtins_with_cache_js)
631630
.IsNothing()) {
632631
return;
@@ -638,7 +637,7 @@ void BuiltinLoader::GetCacheUsage(const FunctionCallbackInfo<Value>& args) {
638637
}
639638
if (result
640639
->Set(context,
641-
OneByteString(isolate, "compiledWithoutCache"),
640+
FIXED_ONE_BYTE_STRING(isolate, "compiledWithoutCache"),
642641
builtins_without_cache_js)
643642
.IsNothing()) {
644643
return;
@@ -650,7 +649,7 @@ void BuiltinLoader::GetCacheUsage(const FunctionCallbackInfo<Value>& args) {
650649
}
651650
if (result
652651
->Set(context,
653-
OneByteString(isolate, "compiledInSnapshot"),
652+
FIXED_ONE_BYTE_STRING(isolate, "compiledInSnapshot"),
654653
builtins_in_snapshot_js)
655654
.IsNothing()) {
656655
return;

‎src/node_constants.cc

+41-27
Original file line numberDiff line numberDiff line change
@@ -1337,33 +1337,47 @@ void CreatePerContextProperties(Local<Object> target,
13371337
// Define libuv constants.
13381338
NODE_DEFINE_CONSTANT(os_constants, UV_UDP_REUSEADDR);
13391339

1340-
os_constants->Set(env->context(),
1341-
OneByteString(isolate, "dlopen"),
1342-
dlopen_constants).Check();
1343-
os_constants->Set(env->context(),
1344-
OneByteString(isolate, "errno"),
1345-
err_constants).Check();
1346-
os_constants->Set(env->context(),
1347-
OneByteString(isolate, "signals"),
1348-
sig_constants).Check();
1349-
os_constants->Set(env->context(),
1350-
OneByteString(isolate, "priority"),
1351-
priority_constants).Check();
1352-
target->Set(env->context(),
1353-
OneByteString(isolate, "os"),
1354-
os_constants).Check();
1355-
target->Set(env->context(),
1356-
OneByteString(isolate, "fs"),
1357-
fs_constants).Check();
1358-
target->Set(env->context(),
1359-
OneByteString(isolate, "crypto"),
1360-
crypto_constants).Check();
1361-
target->Set(env->context(),
1362-
OneByteString(isolate, "zlib"),
1363-
zlib_constants).Check();
1364-
target->Set(env->context(),
1365-
OneByteString(isolate, "trace"),
1366-
trace_constants).Check();
1340+
os_constants
1341+
->Set(env->context(),
1342+
FIXED_ONE_BYTE_STRING(isolate, "dlopen"),
1343+
dlopen_constants)
1344+
.Check();
1345+
os_constants
1346+
->Set(env->context(),
1347+
FIXED_ONE_BYTE_STRING(isolate, "errno"),
1348+
err_constants)
1349+
.Check();
1350+
os_constants
1351+
->Set(env->context(),
1352+
FIXED_ONE_BYTE_STRING(isolate, "signals"),
1353+
sig_constants)
1354+
.Check();
1355+
os_constants
1356+
->Set(env->context(),
1357+
FIXED_ONE_BYTE_STRING(isolate, "priority"),
1358+
priority_constants)
1359+
.Check();
1360+
target
1361+
->Set(env->context(), FIXED_ONE_BYTE_STRING(isolate, "os"), os_constants)
1362+
.Check();
1363+
target
1364+
->Set(env->context(), FIXED_ONE_BYTE_STRING(isolate, "fs"), fs_constants)
1365+
.Check();
1366+
target
1367+
->Set(env->context(),
1368+
FIXED_ONE_BYTE_STRING(isolate, "crypto"),
1369+
crypto_constants)
1370+
.Check();
1371+
target
1372+
->Set(env->context(),
1373+
FIXED_ONE_BYTE_STRING(isolate, "zlib"),
1374+
zlib_constants)
1375+
.Check();
1376+
target
1377+
->Set(env->context(),
1378+
FIXED_ONE_BYTE_STRING(isolate, "trace"),
1379+
trace_constants)
1380+
.Check();
13671381
}
13681382

13691383
} // namespace constants

‎src/node_errors.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ void OOMErrorHandler(const char* location, const v8::OOMDetails& details);
118118
inline v8::Local<v8::Object> code( \
119119
v8::Isolate* isolate, const char* format, Args&&... args) { \
120120
std::string message = SPrintF(format, std::forward<Args>(args)...); \
121-
v8::Local<v8::String> js_code = OneByteString(isolate, #code); \
121+
v8::Local<v8::String> js_code = FIXED_ONE_BYTE_STRING(isolate, #code); \
122122
v8::Local<v8::String> js_msg = \
123123
v8::String::NewFromUtf8(isolate, \
124124
message.c_str(), \
@@ -129,7 +129,7 @@ void OOMErrorHandler(const char* location, const v8::OOMDetails& details);
129129
->ToObject(isolate->GetCurrentContext()) \
130130
.ToLocalChecked(); \
131131
e->Set(isolate->GetCurrentContext(), \
132-
OneByteString(isolate, "code"), \
132+
FIXED_ONE_BYTE_STRING(isolate, "code"), \
133133
js_code) \
134134
.Check(); \
135135
return e; \

‎src/node_http2.cc

+8-7
Original file line numberDiff line numberDiff line change
@@ -725,13 +725,14 @@ MaybeLocal<Object> Http2SessionPerformanceEntryTraits::GetDetails(
725725
SET(stream_average_duration_string, stream_average_duration)
726726
SET(stream_count_string, stream_count)
727727

728-
if (!obj->Set(
729-
env->context(),
730-
env->type_string(),
731-
OneByteString(
732-
env->isolate(),
733-
(entry.details.session_type == NGHTTP2_SESSION_SERVER)
734-
? "server" : "client")).IsJust()) {
728+
if (!obj->Set(env->context(),
729+
env->type_string(),
730+
FIXED_ONE_BYTE_STRING(
731+
env->isolate(),
732+
(entry.details.session_type == NGHTTP2_SESSION_SERVER)
733+
? "server"
734+
: "client"))
735+
.IsJust()) {
735736
return MaybeLocal<Object>();
736737
}
737738

‎src/node_messaging.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -1660,7 +1660,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
16601660
Local<FunctionTemplate> t = FunctionTemplate::New(isolate);
16611661
t->InstanceTemplate()->SetInternalFieldCount(
16621662
JSTransferable::kInternalFieldCount);
1663-
t->SetClassName(OneByteString(isolate, "JSTransferable"));
1663+
t->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "JSTransferable"));
16641664
isolate_data->set_js_transferable_constructor_template(t);
16651665
}
16661666

‎src/node_perf.h

+6-6
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,12 @@ struct PerformanceEntry {
124124
}
125125

126126
v8::Local<v8::Value> argv[] = {
127-
OneByteString(env->isolate(), name.c_str()),
128-
OneByteString(env->isolate(), GetPerformanceEntryTypeName(Traits::kType)),
129-
v8::Number::New(env->isolate(), start_time),
130-
v8::Number::New(env->isolate(), duration),
131-
detail
132-
};
127+
OneByteString(env->isolate(), name),
128+
OneByteString(env->isolate(),
129+
GetPerformanceEntryTypeName(Traits::kType)),
130+
v8::Number::New(env->isolate(), start_time),
131+
v8::Number::New(env->isolate(), duration),
132+
detail};
133133

134134
node::MakeSyncCallback(
135135
env->isolate(),

‎src/node_process_events.cc

+1-2
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,7 @@ using v8::Value;
2121
Maybe<bool> ProcessEmitWarningSync(Environment* env, std::string_view message) {
2222
Isolate* isolate = env->isolate();
2323
Local<Context> context = env->context();
24-
Local<String> message_string =
25-
OneByteString(isolate, message.data(), message.size());
24+
Local<String> message_string = OneByteString(isolate, message);
2625

2726
Local<Value> argv[] = {message_string};
2827
Local<Function> emit_function = env->process_emit_warning_sync();

‎src/node_process_methods.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -312,12 +312,12 @@ static void GetActiveResourcesInfo(const FunctionCallbackInfo<Value>& args) {
312312
// Active timeouts
313313
resources_info.insert(resources_info.end(),
314314
env->timeout_info()[0],
315-
OneByteString(env->isolate(), "Timeout"));
315+
FIXED_ONE_BYTE_STRING(env->isolate(), "Timeout"));
316316

317317
// Active immediates
318318
resources_info.insert(resources_info.end(),
319319
env->immediate_info()->ref_count(),
320-
OneByteString(env->isolate(), "Immediate"));
320+
FIXED_ONE_BYTE_STRING(env->isolate(), "Immediate"));
321321

322322
args.GetReturnValue().Set(
323323
Array::New(env->isolate(), resources_info.data(), resources_info.size()));

‎src/node_process_object.cc

+4-6
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,10 @@ static void SetVersions(Isolate* isolate, Local<Object> versions) {
104104

105105
for (const auto& version : versions_array) {
106106
versions
107-
->DefineOwnProperty(
108-
context,
109-
OneByteString(isolate, version.first.data(), version.first.size()),
110-
OneByteString(
111-
isolate, version.second.data(), version.second.size()),
112-
v8::ReadOnly)
107+
->DefineOwnProperty(context,
108+
OneByteString(isolate, version.first),
109+
OneByteString(isolate, version.second),
110+
v8::ReadOnly)
113111
.Check();
114112
}
115113
}

‎src/node_sqlite.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -1735,7 +1735,7 @@ static void Initialize(Local<Object> target,
17351735
"StatementSync",
17361736
StatementSync::GetConstructorTemplate(env));
17371737

1738-
target->Set(context, OneByteString(isolate, "constants"), constants).Check();
1738+
target->Set(context, env->constants_string(), constants).Check();
17391739
}
17401740

17411741
} // namespace sqlite

‎src/util-inl.h

+5
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,11 @@ inline v8::Local<v8::String> OneByteString(v8::Isolate* isolate,
180180
.ToLocalChecked();
181181
}
182182

183+
inline v8::Local<v8::String> OneByteString(v8::Isolate* isolate,
184+
std::string_view str) {
185+
return OneByteString(isolate, str.data(), str.size());
186+
}
187+
183188
char ToLower(char c) {
184189
return std::tolower(c, std::locale::classic());
185190
}

‎src/util.h

+3
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,9 @@ inline v8::Local<v8::String> OneByteString(v8::Isolate* isolate,
340340
const unsigned char* data,
341341
int length = -1);
342342

343+
inline v8::Local<v8::String> OneByteString(v8::Isolate* isolate,
344+
std::string_view str);
345+
343346
// Used to be a macro, hence the uppercase name.
344347
template <int N>
345348
inline v8::Local<v8::String> FIXED_ONE_BYTE_STRING(

‎src/uv.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ void Initialize(Local<Object> target,
130130
for (size_t i = 0; i < errors_len; ++i) {
131131
const auto& error = per_process::uv_errors_map[i];
132132
const std::string prefixed_name = prefix + error.name;
133-
Local<String> name = OneByteString(isolate, prefixed_name.c_str());
133+
Local<String> name = OneByteString(isolate, prefixed_name);
134134
Local<Integer> value = Integer::New(isolate, error.value);
135135
target->DefineOwnProperty(context, name, value, attributes).Check();
136136
}

0 commit comments

Comments
 (0)