From 39447be4e4e173bff3de5b864755d68e9182f748 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 13 Oct 2025 15:42:40 +0200 Subject: [PATCH] src: use `Utf8Value` and `TwoByteValue` instead of V8 helpers Our own helper classes have the advantage of using stack storage a lot of the time, so they should always be preferred. PR-URL: https://github.com/nodejs/node/pull/60244 Reviewed-By: Chengzhong Wu Reviewed-By: Rafael Gonzaga Reviewed-By: Edy Silva Reviewed-By: Colin Ihrig Reviewed-By: Ilyas Shabi Reviewed-By: Yagiz Nizipli Reviewed-By: Vladimir Morozov --- src/api/async_resource.cc | 7 ++++++ src/api/hooks.cc | 11 ++++++++- src/inspector_js_api.cc | 4 ++-- src/node.h | 8 +++++++ src/node_api.cc | 4 ++-- src/node_buffer.cc | 44 ++++++++++++++++-------------------- src/node_errors.cc | 15 ++++++------ src/node_report.cc | 3 +-- src/node_sqlite.cc | 6 ++--- src/node_v8.cc | 4 ++-- src/permission/permission.cc | 15 ++++-------- src/string_bytes.cc | 12 +++++----- 12 files changed, 72 insertions(+), 61 deletions(-) diff --git a/src/api/async_resource.cc b/src/api/async_resource.cc index 38d53a9aec0..621fca4ba4d 100644 --- a/src/api/async_resource.cc +++ b/src/api/async_resource.cc @@ -16,6 +16,13 @@ AsyncResource::AsyncResource(Isolate* isolate, Local resource, const char* name, async_id trigger_async_id) + : AsyncResource( + isolate, resource, std::string_view(name), trigger_async_id) {} + +AsyncResource::AsyncResource(Isolate* isolate, + Local resource, + std::string_view name, + async_id trigger_async_id) : env_(Environment::GetCurrent(isolate)), resource_(isolate, resource), context_frame_(isolate, async_context_frame::current(isolate)) { diff --git a/src/api/hooks.cc b/src/api/hooks.cc index 7b73f7db242..0594dca02cd 100644 --- a/src/api/hooks.cc +++ b/src/api/hooks.cc @@ -201,9 +201,18 @@ async_context EmitAsyncInit(Isolate* isolate, Local resource, const char* name, async_id trigger_async_id) { + return EmitAsyncInit( + isolate, resource, std::string_view(name), trigger_async_id); +} + +async_context EmitAsyncInit(Isolate* isolate, + Local resource, + std::string_view name, + async_id trigger_async_id) { HandleScope handle_scope(isolate); Local type = - String::NewFromUtf8(isolate, name, NewStringType::kInternalized) + String::NewFromUtf8( + isolate, name.data(), NewStringType::kInternalized, name.size()) .ToLocalChecked(); return EmitAsyncInit(isolate, resource, type, trigger_async_id); } diff --git a/src/inspector_js_api.cc b/src/inspector_js_api.cc index cbad4651f1d..54a91765ac6 100644 --- a/src/inspector_js_api.cc +++ b/src/inspector_js_api.cc @@ -240,8 +240,8 @@ static void AsyncTaskScheduledWrapper(const FunctionCallbackInfo& args) { CHECK(args[0]->IsString()); Local task_name = args[0].As(); - String::Value task_name_value(args.GetIsolate(), task_name); - StringView task_name_view(*task_name_value, task_name_value.length()); + TwoByteValue task_name_value(args.GetIsolate(), task_name); + StringView task_name_view(task_name_value.out(), task_name_value.length()); CHECK(args[1]->IsNumber()); int64_t task_id; diff --git a/src/node.h b/src/node.h index 94c307e3f84..70b60e79ac2 100644 --- a/src/node.h +++ b/src/node.h @@ -1402,6 +1402,10 @@ NODE_EXTERN async_context EmitAsyncInit(v8::Isolate* isolate, v8::Local resource, const char* name, async_id trigger_async_id = -1); +NODE_EXTERN async_context EmitAsyncInit(v8::Isolate* isolate, + v8::Local resource, + std::string_view name, + async_id trigger_async_id = -1); NODE_EXTERN async_context EmitAsyncInit(v8::Isolate* isolate, v8::Local resource, @@ -1508,6 +1512,10 @@ class NODE_EXTERN AsyncResource { v8::Local resource, const char* name, async_id trigger_async_id = -1); + AsyncResource(v8::Isolate* isolate, + v8::Local resource, + std::string_view name, + async_id trigger_async_id = -1); virtual ~AsyncResource(); diff --git a/src/node_api.cc b/src/node_api.cc index 7274030f949..a72ba0ddc96 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -212,7 +212,7 @@ class ThreadSafeFunction : public node::AsyncResource { napi_threadsafe_function_call_js call_js_cb_) : AsyncResource(env_->isolate, resource, - *v8::String::Utf8Value(env_->isolate, name)), + node::Utf8Value(env_->isolate, name).ToStringView()), thread_count(thread_count_), is_closing(false), dispatch_state(kDispatchIdle), @@ -1150,7 +1150,7 @@ class Work : public node::AsyncResource, public node::ThreadPoolWork { : AsyncResource( env->isolate, async_resource, - *v8::String::Utf8Value(env->isolate, async_resource_name)), + node::Utf8Value(env->isolate, async_resource_name).ToStringView()), ThreadPoolWork(env->node_env(), "node_api"), _env(env), _data(data), diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 0e4d437c1ea..cc418017f76 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -986,11 +986,7 @@ void IndexOfString(const FunctionCallbackInfo& args) { size_t result = haystack_length; if (enc == UCS2) { - String::Value needle_value(isolate, needle); - if (*needle_value == nullptr) { - return args.GetReturnValue().Set(-1); - } - + TwoByteValue needle_value(isolate, needle); if (haystack_length < 2 || needle_value.length() < 1) { return args.GetReturnValue().Set(-1); } @@ -1011,27 +1007,27 @@ void IndexOfString(const FunctionCallbackInfo& args) { offset / 2, is_forward); } else { - result = - nbytes::SearchString(reinterpret_cast(haystack), - haystack_length / 2, - reinterpret_cast(*needle_value), - needle_value.length(), - offset / 2, - is_forward); + result = nbytes::SearchString(reinterpret_cast(haystack), + haystack_length / 2, + needle_value.out(), + needle_value.length(), + offset / 2, + is_forward); } result *= 2; } else if (enc == UTF8) { - String::Utf8Value needle_value(isolate, needle); + Utf8Value needle_value(isolate, needle); if (*needle_value == nullptr) return args.GetReturnValue().Set(-1); + CHECK_GE(needle_length, needle_value.length()); - result = - nbytes::SearchString(reinterpret_cast(haystack), - haystack_length, - reinterpret_cast(*needle_value), - needle_length, - offset, - is_forward); + result = nbytes::SearchString( + reinterpret_cast(haystack), + haystack_length, + reinterpret_cast(needle_value.out()), + needle_length, + offset, + is_forward); } else if (enc == LATIN1) { uint8_t* needle_data = node::UncheckedMalloc(needle_length); if (needle_data == nullptr) { @@ -1316,10 +1312,10 @@ static void Btoa(const FunctionCallbackInfo& args) { input->Length(), buffer.out()); } else { - String::Value value(env->isolate(), input); + TwoByteValue value(env->isolate(), input); MaybeStackBuffer stack_buf(value.length()); size_t out_len = simdutf::convert_utf16_to_latin1( - reinterpret_cast(*value), + reinterpret_cast(value.out()), value.length(), stack_buf.out()); if (out_len == 0) { // error @@ -1370,8 +1366,8 @@ static void Atob(const FunctionCallbackInfo& args) { buffer.SetLength(expected_length); result = simdutf::base64_to_binary(data, input->Length(), buffer.out()); } else { // 16-bit case - String::Value value(env->isolate(), input); - auto data = reinterpret_cast(*value); + TwoByteValue value(env->isolate(), input); + auto data = reinterpret_cast(value.out()); size_t expected_length = simdutf::maximal_binary_length_from_base64(data, value.length()); buffer.AllocateSufficientStorage(expected_length); diff --git a/src/node_errors.cc b/src/node_errors.cc index 7ad099a11b9..ec84aaafbdc 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -1024,15 +1024,14 @@ void PerIsolateMessageListener(Local message, Local error) { break; } Utf8Value filename(isolate, message->GetScriptOrigin().ResourceName()); + Utf8Value msg(isolate, message->Get()); // (filename):(line) (message) - std::stringstream warning; - warning << *filename; - warning << ":"; - warning << message->GetLineNumber(env->context()).FromMaybe(-1); - warning << " "; - v8::String::Utf8Value msg(isolate, message->Get()); - warning << *msg; - USE(ProcessEmitWarningGeneric(env, warning.str().c_str(), "V8")); + std::string warning = + SPrintF("%s:%s %s", + filename, + message->GetLineNumber(env->context()).FromMaybe(-1), + msg); + USE(ProcessEmitWarningGeneric(env, warning, "V8")); break; } case Isolate::MessageErrorLevel::kMessageError: diff --git a/src/node_report.cc b/src/node_report.cc index 0bbf829b45f..e4cb681fd76 100644 --- a/src/node_report.cc +++ b/src/node_report.cc @@ -455,8 +455,7 @@ static Maybe ErrorToString(Isolate* isolate, if (!maybe_str.ToLocal(&js_str)) { return Nothing(); } - String::Utf8Value sv(isolate, js_str); - return Just<>(std::string(*sv, sv.length())); + return Just(Utf8Value(isolate, js_str).ToString()); } static void PrintEmptyJavaScriptStack(JSONWriter* writer) { diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index cf9ce6686cf..1cab6dde9df 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1508,8 +1508,7 @@ void DatabaseSync::CreateSession(const FunctionCallbackInfo& args) { } if (table_value->IsString()) { - String::Utf8Value str(env->isolate(), table_value); - table = *str; + table = Utf8Value(env->isolate(), table_value).ToString(); } else { THROW_ERR_INVALID_ARG_TYPE( env->isolate(), "The \"options.table\" argument must be a string."); @@ -1529,8 +1528,7 @@ void DatabaseSync::CreateSession(const FunctionCallbackInfo& args) { return; } if (db_value->IsString()) { - String::Utf8Value str(env->isolate(), db_value); - db_name = std::string(*str); + db_name = Utf8Value(env->isolate(), db_value).ToString(); } else { THROW_ERR_INVALID_ARG_TYPE( env->isolate(), "The \"options.db\" argument must be a string."); diff --git a/src/node_v8.cc b/src/node_v8.cc index ce253d7fa0a..935ea2cf515 100644 --- a/src/node_v8.cc +++ b/src/node_v8.cc @@ -242,8 +242,8 @@ void UpdateHeapCodeStatisticsBuffer(const FunctionCallbackInfo& args) { void SetFlagsFromString(const FunctionCallbackInfo& args) { CHECK(args[0]->IsString()); - String::Utf8Value flags(args.GetIsolate(), args[0]); - V8::SetFlagsFromString(*flags, static_cast(flags.length())); + Utf8Value flags(args.GetIsolate(), args[0]); + V8::SetFlagsFromString(flags.out(), flags.length()); } void StartCpuProfile(const FunctionCallbackInfo& args) { diff --git a/src/permission/permission.cc b/src/permission/permission.cc index 199b36ee76b..2d0e1723050 100644 --- a/src/permission/permission.cc +++ b/src/permission/permission.cc @@ -21,7 +21,6 @@ using v8::IntegrityLevel; using v8::Local; using v8::MaybeLocal; using v8::Object; -using v8::String; using v8::Value; namespace permission { @@ -34,24 +33,20 @@ static void Has(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); CHECK(args[0]->IsString()); - String::Utf8Value utf8_deny_scope(env->isolate(), args[0]); - if (*utf8_deny_scope == nullptr) { - return; - } - - const std::string deny_scope = *utf8_deny_scope; + const std::string deny_scope = Utf8Value(env->isolate(), args[0]).ToString(); PermissionScope scope = Permission::StringToPermission(deny_scope); if (scope == PermissionScope::kPermissionsRoot) { return args.GetReturnValue().Set(false); } if (args.Length() > 1 && !args[1]->IsUndefined()) { - String::Utf8Value utf8_arg(env->isolate(), args[1]); - if (*utf8_arg == nullptr) { + Utf8Value utf8_arg(env->isolate(), args[1]); + if (utf8_arg.length() == 0) { + args.GetReturnValue().Set(false); return; } return args.GetReturnValue().Set( - env->permission()->is_granted(env, scope, *utf8_arg)); + env->permission()->is_granted(env, scope, utf8_arg.ToStringView())); } return args.GetReturnValue().Set(env->permission()->is_granted(env, scope)); diff --git a/src/string_bytes.cc b/src/string_bytes.cc index d78bbb23732..03b5fd7ebe3 100644 --- a/src/string_bytes.cc +++ b/src/string_bytes.cc @@ -304,10 +304,10 @@ size_t StringBytes::Write(Isolate* isolate, input_view.length()); } } else { - String::Value value(isolate, str); + TwoByteValue value(isolate, str); size_t written_len = buflen; auto result = simdutf::base64_to_binary_safe( - reinterpret_cast(*value), + reinterpret_cast(value.out()), value.length(), buf, written_len, @@ -343,10 +343,10 @@ size_t StringBytes::Write(Isolate* isolate, input_view.length()); } } else { - String::Value value(isolate, str); + TwoByteValue value(isolate, str); size_t written_len = buflen; auto result = simdutf::base64_to_binary_safe( - reinterpret_cast(*value), + reinterpret_cast(value.out()), value.length(), buf, written_len); @@ -368,8 +368,8 @@ size_t StringBytes::Write(Isolate* isolate, reinterpret_cast(input_view.data8()), input_view.length()); } else { - String::Value value(isolate, str); - nbytes = nbytes::HexDecode(buf, buflen, *value, value.length()); + TwoByteValue value(isolate, str); + nbytes = nbytes::HexDecode(buf, buflen, value.out(), value.length()); } break;