Skip to content

Commit 179a947

Browse files
jasnelltargos
jasnell
authored and
targos
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 <[email protected]> PR-URL: #56482 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Ethan Arrowood <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
1 parent 59e9798 commit 179a947

18 files changed

+92
-74
lines changed

‎src/crypto/crypto_ec.cc

Lines changed: 4 additions & 4 deletions
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

Lines changed: 1 addition & 1 deletion
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

Lines changed: 2 additions & 3 deletions
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

Lines changed: 1 addition & 1 deletion
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

Lines changed: 1 addition & 1 deletion
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

Lines changed: 8 additions & 9 deletions
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);
@@ -207,7 +207,7 @@ MaybeLocal<String> BuiltinLoader::LoadBuiltinSource(Isolate* isolate,
207207
uv_err_name(r),
208208
uv_strerror(r),
209209
filename);
210-
Local<String> message = OneByteString(isolate, buf.c_str());
210+
Local<String> message = OneByteString(isolate, buf);
211211
isolate->ThrowException(v8::Exception::Error(message));
212212
return MaybeLocal<String>();
213213
}
@@ -277,8 +277,7 @@ MaybeLocal<Function> BuiltinLoader::LookupAndCompileInternal(
277277
}
278278

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

284283
BuiltinCodeCacheData cached_data{};
@@ -595,7 +594,7 @@ void BuiltinLoader::GetBuiltinCategories(
595594
return;
596595
if (result
597596
->Set(context,
598-
OneByteString(isolate, "cannotBeRequired"),
597+
FIXED_ONE_BYTE_STRING(isolate, "cannotBeRequired"),
599598
cannot_be_required_js)
600599
.IsNothing())
601600
return;
@@ -604,7 +603,7 @@ void BuiltinLoader::GetBuiltinCategories(
604603
return;
605604
if (result
606605
->Set(context,
607-
OneByteString(isolate, "canBeRequired"),
606+
FIXED_ONE_BYTE_STRING(isolate, "canBeRequired"),
608607
can_be_required_js)
609608
.IsNothing()) {
610609
return;
@@ -627,7 +626,7 @@ void BuiltinLoader::GetCacheUsage(const FunctionCallbackInfo<Value>& args) {
627626
}
628627
if (result
629628
->Set(context,
630-
OneByteString(isolate, "compiledWithCache"),
629+
FIXED_ONE_BYTE_STRING(isolate, "compiledWithCache"),
631630
builtins_with_cache_js)
632631
.IsNothing()) {
633632
return;
@@ -639,7 +638,7 @@ void BuiltinLoader::GetCacheUsage(const FunctionCallbackInfo<Value>& args) {
639638
}
640639
if (result
641640
->Set(context,
642-
OneByteString(isolate, "compiledWithoutCache"),
641+
FIXED_ONE_BYTE_STRING(isolate, "compiledWithoutCache"),
643642
builtins_without_cache_js)
644643
.IsNothing()) {
645644
return;
@@ -651,7 +650,7 @@ void BuiltinLoader::GetCacheUsage(const FunctionCallbackInfo<Value>& args) {
651650
}
652651
if (result
653652
->Set(context,
654-
OneByteString(isolate, "compiledInSnapshot"),
653+
FIXED_ONE_BYTE_STRING(isolate, "compiledInSnapshot"),
655654
builtins_in_snapshot_js)
656655
.IsNothing()) {
657656
return;

‎src/node_constants.cc

Lines changed: 41 additions & 27 deletions
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

Lines changed: 2 additions & 2 deletions
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

Lines changed: 8 additions & 7 deletions
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

Lines changed: 1 addition & 1 deletion
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

Lines changed: 6 additions & 6 deletions
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

Lines changed: 1 addition & 2 deletions
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

Lines changed: 2 additions & 2 deletions
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

Lines changed: 4 additions & 6 deletions
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

Lines changed: 1 addition & 1 deletion
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

Lines changed: 5 additions & 0 deletions
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

Lines changed: 3 additions & 0 deletions
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

Lines changed: 1 addition & 1 deletion
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)