src: minor cleanups on OneByteString usage

jasnell · jasnell · commit c0c743fddf9d · 2025-01-05T12:36:27.000-08:00
* Provide a OneByteString variant that accepts std::string_view
* Use FIXED_ONE_BYTE_STRING in appropriate places
diff --git a/src/crypto/crypto_ec.cc b/src/crypto/crypto_ec.cc
@@ -767,16 +767,16 @@ Maybe<void> ExportJWKEcKey(Environment* env,
   const int nid = EC_GROUP_get_curve_name(group);
   switch (nid) {
     case NID_X9_62_prime256v1:
-      crv_name = OneByteString(env->isolate(), "P-256");
+      crv_name = FIXED_ONE_BYTE_STRING(env->isolate(), "P-256");
       break;
     case NID_secp256k1:
-      crv_name = OneByteString(env->isolate(), "secp256k1");
+      crv_name = FIXED_ONE_BYTE_STRING(env->isolate(), "secp256k1");
       break;
     case NID_secp384r1:
-      crv_name = OneByteString(env->isolate(), "P-384");
+      crv_name = FIXED_ONE_BYTE_STRING(env->isolate(), "P-384");
       break;
     case NID_secp521r1:
-      crv_name = OneByteString(env->isolate(), "P-521");
+      crv_name = FIXED_ONE_BYTE_STRING(env->isolate(), "P-521");
       break;
     default: {
       THROW_ERR_CRYPTO_JWK_UNSUPPORTED_CURVE(
diff --git a/src/crypto/crypto_hash.cc b/src/crypto/crypto_hash.cc
@@ -152,7 +152,7 @@ void Hash::GetCachedAliases(const FunctionCallbackInfo<Value>& args) {
   names.reserve(size);
   values.reserve(size);
   for (auto& [alias, id] : env->alias_to_md_id_map) {
-    names.push_back(OneByteString(isolate, alias.c_str(), alias.size()));
+    names.push_back(OneByteString(isolate, alias));
     values.push_back(v8::Uint32::New(isolate, id));
   }
 #else
diff --git a/src/crypto/crypto_tls.cc b/src/crypto/crypto_tls.cc
@@ -830,8 +830,7 @@ void TLSWrap::ClearOut() {
           if (context.IsEmpty()) [[unlikely]]
             return;
           const std::string error_str = GetBIOError();
-          Local<String> message = OneByteString(
-              env()->isolate(), error_str.c_str(), error_str.size());
+          Local<String> message = OneByteString(env()->isolate(), error_str);
           if (message.IsEmpty()) [[unlikely]]
             return;
           error = Exception::Error(message);
@@ -1938,7 +1937,7 @@ void TLSWrap::GetSharedSigalgs(const FunctionCallbackInfo<Value>& args) {
     } else {
       sig_with_md += "UNDEF";
     }
-    ret_arr[i] = OneByteString(env->isolate(), sig_with_md.c_str());
+    ret_arr[i] = OneByteString(env->isolate(), sig_with_md);
   }
 
   args.GetReturnValue().Set(
diff --git a/src/histogram.cc b/src/histogram.cc
@@ -341,7 +341,7 @@ Local<FunctionTemplate> IntervalHistogram::GetConstructorTemplate(
     Isolate* isolate = env->isolate();
     tmpl = NewFunctionTemplate(isolate, nullptr);
     tmpl->Inherit(HandleWrap::GetConstructorTemplate(env));
-    tmpl->SetClassName(OneByteString(isolate, "Histogram"));
+    tmpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "Histogram"));
     auto instance = tmpl->InstanceTemplate();
     instance->SetInternalFieldCount(HistogramImpl::kInternalFieldCount);
     HistogramImpl::AddMethods(isolate, tmpl);
diff --git a/src/inspector_js_api.cc b/src/inspector_js_api.cc
@@ -333,7 +333,7 @@ void Url(const FunctionCallbackInfo<Value>& args) {
   if (url.empty()) {
     return;
   }
-  args.GetReturnValue().Set(OneByteString(env->isolate(), url.c_str()));
+  args.GetReturnValue().Set(OneByteString(env->isolate(), url));
 }
 
 void Initialize(Local<Object> target, Local<Value> unused,
diff --git a/src/node_builtins.cc b/src/node_builtins.cc
@@ -78,7 +78,7 @@ void BuiltinLoader::GetNatives(Local<Name> property,
   Local<Object> out = Object::New(isolate);
   auto source = env->builtin_loader()->source_.read();
   for (auto const& x : *source) {
-    Local<String> key = OneByteString(isolate, x.first.c_str(), x.first.size());
+    Local<String> key = OneByteString(isolate, x.first);
     out->Set(context, key, x.second.ToStringChecked(isolate)).FromJust();
   }
   info.GetReturnValue().Set(out);
@@ -206,7 +206,7 @@ MaybeLocal<String> BuiltinLoader::LoadBuiltinSource(Isolate* isolate,
                                     uv_err_name(r),
                                     uv_strerror(r),
                                     filename);
-    Local<String> message = OneByteString(isolate, buf.c_str());
+    Local<String> message = OneByteString(isolate, buf);
     isolate->ThrowException(v8::Exception::Error(message));
     return MaybeLocal<String>();
   }
@@ -276,8 +276,7 @@ MaybeLocal<Function> BuiltinLoader::LookupAndCompileInternal(
   }
 
   std::string filename_s = std::string("node:") + id;
-  Local<String> filename =
-      OneByteString(isolate, filename_s.c_str(), filename_s.size());
+  Local<String> filename = OneByteString(isolate, filename_s);
   ScriptOrigin origin(filename, 0, 0, true);
 
   BuiltinCodeCacheData cached_data{};
@@ -594,7 +593,7 @@ void BuiltinLoader::GetBuiltinCategories(
     return;
   if (result
           ->Set(context,
-                OneByteString(isolate, "cannotBeRequired"),
+                FIXED_ONE_BYTE_STRING(isolate, "cannotBeRequired"),
                 cannot_be_required_js)
           .IsNothing())
     return;
@@ -603,7 +602,7 @@ void BuiltinLoader::GetBuiltinCategories(
     return;
   if (result
           ->Set(context,
-                OneByteString(isolate, "canBeRequired"),
+                FIXED_ONE_BYTE_STRING(isolate, "canBeRequired"),
                 can_be_required_js)
           .IsNothing()) {
     return;
@@ -626,7 +625,7 @@ void BuiltinLoader::GetCacheUsage(const FunctionCallbackInfo<Value>& args) {
   }
   if (result
           ->Set(context,
-                OneByteString(isolate, "compiledWithCache"),
+                FIXED_ONE_BYTE_STRING(isolate, "compiledWithCache"),
                 builtins_with_cache_js)
           .IsNothing()) {
     return;
@@ -638,7 +637,7 @@ void BuiltinLoader::GetCacheUsage(const FunctionCallbackInfo<Value>& args) {
   }
   if (result
           ->Set(context,
-                OneByteString(isolate, "compiledWithoutCache"),
+                FIXED_ONE_BYTE_STRING(isolate, "compiledWithoutCache"),
                 builtins_without_cache_js)
           .IsNothing()) {
     return;
@@ -650,7 +649,7 @@ void BuiltinLoader::GetCacheUsage(const FunctionCallbackInfo<Value>& args) {
   }
   if (result
           ->Set(context,
-                OneByteString(isolate, "compiledInSnapshot"),
+                FIXED_ONE_BYTE_STRING(isolate, "compiledInSnapshot"),
                 builtins_in_snapshot_js)
           .IsNothing()) {
     return;
diff --git a/src/node_constants.cc b/src/node_constants.cc
@@ -1338,31 +1338,31 @@ void CreatePerContextProperties(Local<Object> target,
   NODE_DEFINE_CONSTANT(os_constants, UV_UDP_REUSEADDR);
 
   os_constants->Set(env->context(),
-                    OneByteString(isolate, "dlopen"),
+                    FIXED_ONE_BYTE_STRING(isolate, "dlopen"),
                     dlopen_constants).Check();
   os_constants->Set(env->context(),
-                    OneByteString(isolate, "errno"),
+                    FIXED_ONE_BYTE_STRING(isolate, "errno"),
                     err_constants).Check();
   os_constants->Set(env->context(),
-                    OneByteString(isolate, "signals"),
+                    FIXED_ONE_BYTE_STRING(isolate, "signals"),
                     sig_constants).Check();
   os_constants->Set(env->context(),
-                    OneByteString(isolate, "priority"),
+                    FIXED_ONE_BYTE_STRING(isolate, "priority"),
                     priority_constants).Check();
   target->Set(env->context(),
-              OneByteString(isolate, "os"),
+              FIXED_ONE_BYTE_STRING(isolate, "os"),
               os_constants).Check();
   target->Set(env->context(),
-              OneByteString(isolate, "fs"),
+              FIXED_ONE_BYTE_STRING(isolate, "fs"),
               fs_constants).Check();
   target->Set(env->context(),
-              OneByteString(isolate, "crypto"),
+              FIXED_ONE_BYTE_STRING(isolate, "crypto"),
               crypto_constants).Check();
   target->Set(env->context(),
-              OneByteString(isolate, "zlib"),
+              FIXED_ONE_BYTE_STRING(isolate, "zlib"),
               zlib_constants).Check();
   target->Set(env->context(),
-              OneByteString(isolate, "trace"),
+              FIXED_ONE_BYTE_STRING(isolate, "trace"),
               trace_constants).Check();
 }
 
diff --git a/src/node_errors.h b/src/node_errors.h
@@ -118,7 +118,7 @@ void OOMErrorHandler(const char* location, const v8::OOMDetails& details);
   inline v8::Local<v8::Object> code(                                           \
       v8::Isolate* isolate, const char* format, Args&&... args) {              \
     std::string message = SPrintF(format, std::forward<Args>(args)...);        \
-    v8::Local<v8::String> js_code = OneByteString(isolate, #code);             \
+    v8::Local<v8::String> js_code = FIXED_ONE_BYTE_STRING(isolate, #code);     \
     v8::Local<v8::String> js_msg =                                             \
         v8::String::NewFromUtf8(isolate,                                       \
                                 message.c_str(),                               \
@@ -129,7 +129,7 @@ void OOMErrorHandler(const char* location, const v8::OOMDetails& details);
                                   ->ToObject(isolate->GetCurrentContext())     \
                                   .ToLocalChecked();                           \
     e->Set(isolate->GetCurrentContext(),                                       \
-           OneByteString(isolate, "code"),                                     \
+           FIXED_ONE_BYTE_STRING(isolate, "code"),                             \
            js_code)                                                            \
         .Check();                                                              \
     return e;                                                                  \
diff --git a/src/node_http2.cc b/src/node_http2.cc
@@ -728,7 +728,7 @@ MaybeLocal<Object> Http2SessionPerformanceEntryTraits::GetDetails(
   if (!obj->Set(
           env->context(),
           env->type_string(),
-          OneByteString(
+          FIXED_ONE_BYTE_STRING(
               env->isolate(),
               (entry.details.session_type == NGHTTP2_SESSION_SERVER)
                   ? "server" : "client")).IsJust()) {
diff --git a/src/node_messaging.cc b/src/node_messaging.cc
@@ -1660,7 +1660,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
     Local<FunctionTemplate> t = FunctionTemplate::New(isolate);
     t->InstanceTemplate()->SetInternalFieldCount(
         JSTransferable::kInternalFieldCount);
-    t->SetClassName(OneByteString(isolate, "JSTransferable"));
+    t->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "JSTransferable"));
     isolate_data->set_js_transferable_constructor_template(t);
   }
 
diff --git a/src/node_perf.h b/src/node_perf.h
@@ -124,7 +124,7 @@ struct PerformanceEntry {
     }
 
     v8::Local<v8::Value> argv[] = {
-      OneByteString(env->isolate(), name.c_str()),
+      OneByteString(env->isolate(), name),
       OneByteString(env->isolate(), GetPerformanceEntryTypeName(Traits::kType)),
       v8::Number::New(env->isolate(), start_time),
       v8::Number::New(env->isolate(), duration),
diff --git a/src/node_process_events.cc b/src/node_process_events.cc
@@ -21,8 +21,7 @@ using v8::Value;
 Maybe<bool> ProcessEmitWarningSync(Environment* env, std::string_view message) {
   Isolate* isolate = env->isolate();
   Local<Context> context = env->context();
-  Local<String> message_string =
-      OneByteString(isolate, message.data(), message.size());
+  Local<String> message_string = OneByteString(isolate, message);
 
   Local<Value> argv[] = {message_string};
   Local<Function> emit_function = env->process_emit_warning_sync();
diff --git a/src/node_process_methods.cc b/src/node_process_methods.cc
@@ -312,12 +312,12 @@ static void GetActiveResourcesInfo(const FunctionCallbackInfo<Value>& args) {
   // Active timeouts
   resources_info.insert(resources_info.end(),
                         env->timeout_info()[0],
-                        OneByteString(env->isolate(), "Timeout"));
+                        FIXED_ONE_BYTE_STRING(env->isolate(), "Timeout"));
 
   // Active immediates
   resources_info.insert(resources_info.end(),
                         env->immediate_info()->ref_count(),
-                        OneByteString(env->isolate(), "Immediate"));
+                        FIXED_ONE_BYTE_STRING(env->isolate(), "Immediate"));
 
   args.GetReturnValue().Set(
       Array::New(env->isolate(), resources_info.data(), resources_info.size()));
diff --git a/src/node_process_object.cc b/src/node_process_object.cc
@@ -106,9 +106,8 @@ static void SetVersions(Isolate* isolate, Local<Object> versions) {
     versions
         ->DefineOwnProperty(
             context,
-            OneByteString(isolate, version.first.data(), version.first.size()),
-            OneByteString(
-                isolate, version.second.data(), version.second.size()),
+            OneByteString(isolate, version.first),
+            OneByteString(isolate, version.second),
             v8::ReadOnly)
         .Check();
   }
diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc
@@ -1735,7 +1735,7 @@ static void Initialize(Local<Object> target,
                          "StatementSync",
                          StatementSync::GetConstructorTemplate(env));
 
-  target->Set(context, OneByteString(isolate, "constants"), constants).Check();
+  target->Set(context, env->constants_string(), constants).Check();
 }
 
 }  // namespace sqlite
diff --git a/src/util-inl.h b/src/util-inl.h
@@ -180,6 +180,11 @@ inline v8::Local<v8::String> OneByteString(v8::Isolate* isolate,
       .ToLocalChecked();
 }
 
+inline v8::Local<v8::String> OneByteString(v8::Isolate* isolate,
+                                           const std::string_view& str) {
+  return OneByteString(isolate, str.data(), str.size());
+}
+
 char ToLower(char c) {
   return std::tolower(c, std::locale::classic());
 }
diff --git a/src/util.h b/src/util.h
@@ -340,6 +340,9 @@ inline v8::Local<v8::String> OneByteString(v8::Isolate* isolate,
                                            const unsigned char* data,
                                            int length = -1);
 
+inline v8::Local<v8::String> OneByteString(v8::Isolate* isolate,
+                                           const std::string_view& str);
+
 // Used to be a macro, hence the uppercase name.
 template <int N>
 inline v8::Local<v8::String> FIXED_ONE_BYTE_STRING(
diff --git a/src/uv.cc b/src/uv.cc
@@ -130,7 +130,7 @@ void Initialize(Local<Object> target,
   for (size_t i = 0; i < errors_len; ++i) {
     const auto& error = per_process::uv_errors_map[i];
     const std::string prefixed_name = prefix + error.name;
-    Local<String> name = OneByteString(isolate, prefixed_name.c_str());
+    Local<String> name = OneByteString(isolate, prefixed_name);
     Local<Integer> value = Integer::New(isolate, error.value);
     target->DefineOwnProperty(context, name, value, attributes).Check();
   }

Original file line number	Diff line number	Diff line change
`@@ -152,7 +152,7 @@ void Hash::GetCachedAliases(const FunctionCallbackInfo<Value>& args) {`
`152`	`152`	`names.reserve(size);`
`153`	`153`	`values.reserve(size);`
`154`	`154`	`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));`
`156`	`156`	`values.push_back(v8::Uint32::New(isolate, id));`
`157`	`157`	`}`
`158`	`158`	`#else`
Original file line number	Diff line number	Diff line change
`@@ -333,7 +333,7 @@ void Url(const FunctionCallbackInfo<Value>& args) {`
`333`	`333`	`if (url.empty()) {`
`334`	`334`	`return;`
`335`	`335`	`}`
`336`		`- args.GetReturnValue().Set(OneByteString(env->isolate(), url.c_str()));`
	`336`	`+ args.GetReturnValue().Set(OneByteString(env->isolate(), url));`
`337`	`337`	`}`
`338`	`338`
`339`	`339`	`void Initialize(Local<Object> target, Local<Value> unused,`
Original file line number	Diff line number	Diff line change
`@@ -1660,7 +1660,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,`
`1660`	`1660`	`Local<FunctionTemplate> t = FunctionTemplate::New(isolate);`
`1661`	`1661`	`t->InstanceTemplate()->SetInternalFieldCount(`
`1662`	`1662`	`JSTransferable::kInternalFieldCount);`
`1663`		`- t->SetClassName(OneByteString(isolate, "JSTransferable"));`
	`1663`	`+ t->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "JSTransferable"));`
`1664`	`1664`	`isolate_data->set_js_transferable_constructor_template(t);`
`1665`	`1665`	`}`
`1666`	`1666`