From bd8045b2e2ef77d5f7486dc5d3d6e5f640397a27 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Fri, 2 Jun 2023 16:03:55 -0700 Subject: [PATCH 01/31] node-api: unify string creation --- src/js_native_api_v8.cc | 79 ++++++++++++++++++----------------------- 1 file changed, 35 insertions(+), 44 deletions(-) diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 032a01053f386b..89e5ea8da30145 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -61,6 +61,25 @@ namespace v8impl { namespace { +template +napi_status NewString(napi_env env, + const CCharType* str, + size_t length, + napi_value* result, + StringMaker string_maker) { + CHECK_ENV(env); + if (length > 0) CHECK_ARG(env, str); + CHECK_ARG(env, result); + RETURN_STATUS_IF_FALSE( + env, (length == NAPI_AUTO_LENGTH) || length <= INT_MAX, napi_invalid_arg); + + auto isolate = env->isolate; + auto str_maybe = string_maker(isolate); + CHECK_MAYBE_EMPTY(env, str_maybe, napi_generic_failure); + *result = v8impl::JsValueFromV8LocalValue(str_maybe.ToLocalChecked()); + return napi_clear_last_error(env); +} + inline napi_status V8NameFromPropertyDescriptor( napi_env env, const napi_property_descriptor* p, @@ -1389,62 +1408,34 @@ napi_status NAPI_CDECL napi_create_string_latin1(napi_env env, const char* str, size_t length, napi_value* result) { - CHECK_ENV(env); - if (length > 0) CHECK_ARG(env, str); - CHECK_ARG(env, result); - RETURN_STATUS_IF_FALSE( - env, (length == NAPI_AUTO_LENGTH) || length <= INT_MAX, napi_invalid_arg); - - auto isolate = env->isolate; - auto str_maybe = - v8::String::NewFromOneByte(isolate, - reinterpret_cast(str), - v8::NewStringType::kNormal, - length); - CHECK_MAYBE_EMPTY(env, str_maybe, napi_generic_failure); - - *result = v8impl::JsValueFromV8LocalValue(str_maybe.ToLocalChecked()); - return napi_clear_last_error(env); + return v8impl::NewString(env, str, length, result, [&](v8::Isolate* isolate) { + return v8::String::NewFromOneByte(isolate, + reinterpret_cast(str), + v8::NewStringType::kNormal, + length); + }); } napi_status NAPI_CDECL napi_create_string_utf8(napi_env env, const char* str, size_t length, napi_value* result) { - CHECK_ENV(env); - if (length > 0) CHECK_ARG(env, str); - CHECK_ARG(env, result); - RETURN_STATUS_IF_FALSE( - env, (length == NAPI_AUTO_LENGTH) || length <= INT_MAX, napi_invalid_arg); - - auto isolate = env->isolate; - auto str_maybe = v8::String::NewFromUtf8( - isolate, str, v8::NewStringType::kNormal, static_cast(length)); - CHECK_MAYBE_EMPTY(env, str_maybe, napi_generic_failure); - *result = v8impl::JsValueFromV8LocalValue(str_maybe.ToLocalChecked()); - return napi_clear_last_error(env); + return v8impl::NewString(env, str, length, result, [&](v8::Isolate* isolate) { + return v8::String::NewFromUtf8( + isolate, str, v8::NewStringType::kNormal, static_cast(length)); + }); } napi_status NAPI_CDECL napi_create_string_utf16(napi_env env, const char16_t* str, size_t length, napi_value* result) { - CHECK_ENV(env); - if (length > 0) CHECK_ARG(env, str); - CHECK_ARG(env, result); - RETURN_STATUS_IF_FALSE( - env, (length == NAPI_AUTO_LENGTH) || length <= INT_MAX, napi_invalid_arg); - - auto isolate = env->isolate; - auto str_maybe = - v8::String::NewFromTwoByte(isolate, - reinterpret_cast(str), - v8::NewStringType::kNormal, - length); - CHECK_MAYBE_EMPTY(env, str_maybe, napi_generic_failure); - - *result = v8impl::JsValueFromV8LocalValue(str_maybe.ToLocalChecked()); - return napi_clear_last_error(env); + return v8impl::NewString(env, str, length, result, [&](v8::Isolate* isolate) { + return v8::String::NewFromTwoByte(isolate, + reinterpret_cast(str), + v8::NewStringType::kNormal, + length); + }); } napi_status NAPI_CDECL napi_create_double(napi_env env, From 50a210976fd1cbb88cc6b250e9a17dcaec9e9b24 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Fri, 2 Jun 2023 17:14:01 -0700 Subject: [PATCH 02/31] node-api: add string creation benchmark --- Makefile | 1 + benchmark/napi/string/.gitignore | 1 + benchmark/napi/string/binding.c | 56 +++++++++++++++++++++++++++++++ benchmark/napi/string/binding.gyp | 8 +++++ benchmark/napi/string/index.js | 19 +++++++++++ 5 files changed, 85 insertions(+) create mode 100644 benchmark/napi/string/.gitignore create mode 100644 benchmark/napi/string/binding.c create mode 100644 benchmark/napi/string/binding.gyp create mode 100644 benchmark/napi/string/index.js diff --git a/Makefile b/Makefile index 8e10ec6ad21fbb..455add478aaaa8 100644 --- a/Makefile +++ b/Makefile @@ -1394,6 +1394,7 @@ LINT_CPP_EXCLUDE += src/tracing/trace_event.h src/tracing/trace_event_common.h LINT_CPP_FILES = $(filter-out $(LINT_CPP_EXCLUDE), $(wildcard \ benchmark/napi/*/*.cc \ + benchmark/napi/*/*.c \ src/*.c \ src/*.cc \ src/*.h \ diff --git a/benchmark/napi/string/.gitignore b/benchmark/napi/string/.gitignore new file mode 100644 index 00000000000000..567609b1234a9b --- /dev/null +++ b/benchmark/napi/string/.gitignore @@ -0,0 +1 @@ +build/ diff --git a/benchmark/napi/string/binding.c b/benchmark/napi/string/binding.c new file mode 100644 index 00000000000000..9ac7de5875ef75 --- /dev/null +++ b/benchmark/napi/string/binding.c @@ -0,0 +1,56 @@ +#include +#define NAPI_EXPERIMENTAL +#include + +#define NAPI_CALL(call) \ + do { \ + napi_status status = call; \ + assert(status == napi_ok && #call " failed"); \ + } while (0); + +#define EXPORT_FUNC(env, exports, name, func) \ + do { \ + napi_value js_func; \ + NAPI_CALL(napi_create_function( \ + (env), (name), NAPI_AUTO_LENGTH, (func), NULL, &js_func)); \ + NAPI_CALL(napi_set_named_property((env), (exports), (name), js_func)); \ + } while (0); + +const char* one_byte_string = "The Quick Brown Fox Jumped Over The Lazy Dog."; +const char16_t* two_byte_string = + u"The Quick Brown Fox Jumped Over The Lazy Dog."; + +#define DECLARE_BINDING(CapName, lowercase_name, var_name) \ + static napi_value CreateString##CapName(napi_env env, \ + napi_callback_info info) { \ + size_t argc = 4; \ + napi_value argv[4]; \ + uint32_t n; \ + uint32_t index; \ + napi_handle_scope scope; \ + napi_value js_string; \ + \ + NAPI_CALL(napi_get_cb_info(env, info, &argc, argv, NULL, NULL)); \ + NAPI_CALL(napi_get_value_uint32(env, argv[0], &n)); \ + NAPI_CALL(napi_open_handle_scope(env, &scope)); \ + NAPI_CALL(napi_call_function(env, argv[1], argv[2], 0, NULL, NULL)); \ + for (index = 0; index < n; index++) { \ + NAPI_CALL(napi_create_string_##lowercase_name( \ + env, (var_name), NAPI_AUTO_LENGTH, &js_string)); \ + } \ + NAPI_CALL(napi_call_function(env, argv[1], argv[3], 1, &argv[0], NULL)); \ + NAPI_CALL(napi_close_handle_scope(env, scope)); \ + \ + return NULL; \ + } + +DECLARE_BINDING(Latin1, latin1, one_byte_string) +DECLARE_BINDING(Utf8, utf8, one_byte_string) +DECLARE_BINDING(Utf16, utf16, two_byte_string) + +NAPI_MODULE_INIT() { + EXPORT_FUNC(env, exports, "createStringLatin1", CreateStringLatin1); + EXPORT_FUNC(env, exports, "createStringUtf8", CreateStringUtf8); + EXPORT_FUNC(env, exports, "createStringUtf16", CreateStringUtf16); + return exports; +} diff --git a/benchmark/napi/string/binding.gyp b/benchmark/napi/string/binding.gyp new file mode 100644 index 00000000000000..413621ade335a1 --- /dev/null +++ b/benchmark/napi/string/binding.gyp @@ -0,0 +1,8 @@ +{ + 'targets': [ + { + 'target_name': 'binding', + 'sources': [ 'binding.c' ] + } + ] +} diff --git a/benchmark/napi/string/index.js b/benchmark/napi/string/index.js new file mode 100644 index 00000000000000..434a9d86132c20 --- /dev/null +++ b/benchmark/napi/string/index.js @@ -0,0 +1,19 @@ +'use strict'; +const common = require('../../common.js'); + +let binding; +try { + binding = require(`./build/${common.buildType}/binding`); +} catch { + console.error(`${__filename}: Binding failed to load`); + process.exit(0); +} + +const bench = common.createBenchmark(main, { + n: [1e5, 1e6, 1e7], + stringType: ['Latin1', 'Utf8', 'Utf16'] +}); + +function main({ n, stringType }) { + binding[`createString${stringType}`](n, bench, bench.start, bench.end); +} From 136487284d97ec86d3a2730f0ebc3d99dd72b12e Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Fri, 2 Jun 2023 18:01:36 -0700 Subject: [PATCH 03/31] node-api: implement external strings --- src/js_native_api.h | 8 ++++++ src/js_native_api_v8.cc | 61 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/src/js_native_api.h b/src/js_native_api.h index 8fe93ecb1d1815..0cf0ba461ff784 100644 --- a/src/js_native_api.h +++ b/src/js_native_api.h @@ -92,6 +92,14 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_create_string_utf16(napi_env env, const char16_t* str, size_t length, napi_value* result); +#ifdef NAPI_EXPERIMENTAL +NAPI_EXTERN napi_status NAPI_CDECL napi_create_external_string_latin1( + napi_env env, const char* str, size_t length, napi_value* result); +NAPI_EXTERN napi_status NAPI_CDECL napi_create_external_string_utf8( + napi_env env, const char* str, size_t length, napi_value* result); +NAPI_EXTERN napi_status NAPI_CDECL napi_create_external_string_utf16( + napi_env env, const char16_t* str, size_t length, napi_value* result); +#endif // NAPI_EXPERIMENTAL NAPI_EXTERN napi_status NAPI_CDECL napi_create_symbol(napi_env env, napi_value description, napi_value* result); diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 89e5ea8da30145..80ede2c89032bf 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -61,6 +61,31 @@ namespace v8impl { namespace { +class ExternalOneByteStringResource + : public v8::String::ExternalOneByteStringResource { + public: + ExternalOneByteStringResource(const char* string, const size_t length) + : _string(string), _length(length) {} + const char* data() const { return _string; } + size_t length() const { return _length; } + + private: + const char* _string; + const size_t _length; +}; + +class ExternalStringResource : public v8::String::ExternalStringResource { + public: + ExternalStringResource(const uint16_t* string, const size_t length) + : _string(string), _length(length) {} + const uint16_t* data() const { return _string; } + size_t length() const { return _length; } + + private: + const uint16_t* _string; + const size_t _length; +}; + template napi_status NewString(napi_env env, const CCharType* str, @@ -1438,6 +1463,42 @@ napi_status NAPI_CDECL napi_create_string_utf16(napi_env env, }); } +napi_status NAPI_CDECL napi_create_external_string_latin1(napi_env env, + const char* str, + size_t length, + napi_value* result) { +#if defined(V8_ENABLE_SANDBOX) + return napi_create_string_latin1(env, str, length, result); +#else + return v8impl::NewString(env, str, length, result, [&](v8::Isolate* isolate) { + auto resource = new v8impl::ExternalOneByteStringResource(str, length); + return v8::String::NewExternalOneByte(isolate, resource); + }); +#endif // V8_ENABLE_SANDBOX +} + +napi_status NAPI_CDECL napi_create_external_string_utf8(napi_env env, + const char* str, + size_t length, + napi_value* result) { + return napi_create_string_utf8(env, str, length, result); +} + +napi_status NAPI_CDECL napi_create_external_string_utf16(napi_env env, + const char16_t* str, + size_t length, + napi_value* result) { +#if defined(V8_ENABLE_SANDBOX) + return napi_create_string_utf16(env, str, length, result); +#else + return v8impl::NewString(env, str, length, result, [&](v8::Isolate* isolate) { + auto resource = new v8impl::ExternalStringResource( + reinterpret_cast(str), length); + return v8::String::NewExternalTwoByte(isolate, resource); + }); +#endif // V8_ENABLE_SANDBOX +} + napi_status NAPI_CDECL napi_create_double(napi_env env, double value, napi_value* result) { From a9e24b987310bab0278d7ef8c3874c07fcbe3f4a Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sat, 3 Jun 2023 11:09:19 -0700 Subject: [PATCH 04/31] add test --- test/js-native-api/common.h | 11 + test/js-native-api/test_string/test.js | 23 +- test/js-native-api/test_string/test_string.c | 331 +++++++++++------- .../test_reference_by_node_api_version.c | 21 +- 4 files changed, 252 insertions(+), 134 deletions(-) diff --git a/test/js-native-api/common.h b/test/js-native-api/common.h index 46784059a1f70a..25b26fb09137c5 100644 --- a/test/js-native-api/common.h +++ b/test/js-native-api/common.h @@ -56,6 +56,17 @@ #define NODE_API_CALL_RETURN_VOID(env, the_call) \ NODE_API_CALL_BASE(env, the_call, NODE_API_RETVAL_NOTHING) +#define NODE_API_CHECK_STATUS(the_call) \ + do { \ + napi_status status = (the_call); \ + if (status != napi_ok) { \ + return status; \ + } \ + } while (0) + +#define NODE_API_ASSERT_STATUS(env, assertion, message) \ + NODE_API_ASSERT_BASE(env, assertion, message, napi_generic_failure) + #define DECLARE_NODE_API_PROPERTY(name, func) \ { (name), NULL, (func), NULL, NULL, NULL, napi_default, NULL } diff --git a/test/js-native-api/test_string/test.js b/test/js-native-api/test_string/test.js index b03cd0b3612044..550aa04dac06f5 100644 --- a/test/js-native-api/test_string/test.js +++ b/test/js-native-api/test_string/test.js @@ -3,12 +3,16 @@ const common = require('../../common'); const assert = require('assert'); // Testing api calls for string -const test_string = require(`./build/${common.buildType}/test_string`); +// const test_string = require(`./build/${common.buildType}/test_string`); +const test_string = require(`./build/Debug/test_string`); const empty = ''; assert.strictEqual(test_string.TestLatin1(empty), empty); assert.strictEqual(test_string.TestUtf8(empty), empty); assert.strictEqual(test_string.TestUtf16(empty), empty); +assert.strictEqual(test_string.TestLatin1External(empty), empty); +assert.strictEqual(test_string.TestUtf8External(empty), empty); +assert.strictEqual(test_string.TestUtf16External(empty), empty); assert.strictEqual(test_string.Utf16Length(empty), 0); assert.strictEqual(test_string.Utf8Length(empty), 0); @@ -16,6 +20,9 @@ const str1 = 'hello world'; assert.strictEqual(test_string.TestLatin1(str1), str1); assert.strictEqual(test_string.TestUtf8(str1), str1); assert.strictEqual(test_string.TestUtf16(str1), str1); +assert.strictEqual(test_string.TestLatin1External(str1), str1); +assert.strictEqual(test_string.TestUtf8External(str1), str1); +assert.strictEqual(test_string.TestUtf16External(str1), str1); assert.strictEqual(test_string.TestLatin1Insufficient(str1), str1.slice(0, 3)); assert.strictEqual(test_string.TestUtf8Insufficient(str1), str1.slice(0, 3)); assert.strictEqual(test_string.TestUtf16Insufficient(str1), str1.slice(0, 3)); @@ -26,6 +33,9 @@ const str2 = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'; assert.strictEqual(test_string.TestLatin1(str2), str2); assert.strictEqual(test_string.TestUtf8(str2), str2); assert.strictEqual(test_string.TestUtf16(str2), str2); +assert.strictEqual(test_string.TestLatin1External(str2), str2); +assert.strictEqual(test_string.TestUtf8External(str2), str2); +assert.strictEqual(test_string.TestUtf16External(str2), str2); assert.strictEqual(test_string.TestLatin1Insufficient(str2), str2.slice(0, 3)); assert.strictEqual(test_string.TestUtf8Insufficient(str2), str2.slice(0, 3)); assert.strictEqual(test_string.TestUtf16Insufficient(str2), str2.slice(0, 3)); @@ -36,6 +46,9 @@ const str3 = '?!@#$%^&*()_+-=[]{}/.,<>\'"\\'; assert.strictEqual(test_string.TestLatin1(str3), str3); assert.strictEqual(test_string.TestUtf8(str3), str3); assert.strictEqual(test_string.TestUtf16(str3), str3); +assert.strictEqual(test_string.TestLatin1External(str3), str3); +assert.strictEqual(test_string.TestUtf8External(str3), str3); +assert.strictEqual(test_string.TestUtf16External(str3), str3); assert.strictEqual(test_string.TestLatin1Insufficient(str3), str3.slice(0, 3)); assert.strictEqual(test_string.TestUtf8Insufficient(str3), str3.slice(0, 3)); assert.strictEqual(test_string.TestUtf16Insufficient(str3), str3.slice(0, 3)); @@ -46,6 +59,9 @@ const str4 = '¡¢£¤¥¦§¨©ª«¬­®¯°±²³´µ¶·¸¹º»¼½¾¿'; assert.strictEqual(test_string.TestLatin1(str4), str4); assert.strictEqual(test_string.TestUtf8(str4), str4); assert.strictEqual(test_string.TestUtf16(str4), str4); +assert.strictEqual(test_string.TestLatin1External(str4), str4); +assert.strictEqual(test_string.TestUtf8External(str4), str4); +assert.strictEqual(test_string.TestUtf16External(str4), str4); assert.strictEqual(test_string.TestLatin1Insufficient(str4), str4.slice(0, 3)); assert.strictEqual(test_string.TestUtf8Insufficient(str4), str4.slice(0, 1)); assert.strictEqual(test_string.TestUtf16Insufficient(str4), str4.slice(0, 3)); @@ -56,6 +72,9 @@ const str5 = 'ÀÁÂÃÄÅÆÇÈÉÊËÌÍÎÏÐÑÒÓÔÕÖ×ØÙÚÛÜÝÞßà assert.strictEqual(test_string.TestLatin1(str5), str5); assert.strictEqual(test_string.TestUtf8(str5), str5); assert.strictEqual(test_string.TestUtf16(str5), str5); +assert.strictEqual(test_string.TestLatin1External(str5), str5); +assert.strictEqual(test_string.TestUtf8External(str5), str5); +assert.strictEqual(test_string.TestUtf16External(str5), str5); assert.strictEqual(test_string.TestLatin1Insufficient(str5), str5.slice(0, 3)); assert.strictEqual(test_string.TestUtf8Insufficient(str5), str5.slice(0, 1)); assert.strictEqual(test_string.TestUtf16Insufficient(str5), str5.slice(0, 3)); @@ -65,6 +84,8 @@ assert.strictEqual(test_string.Utf8Length(str5), 126); const str6 = '\u{2003}\u{2101}\u{2001}\u{202}\u{2011}'; assert.strictEqual(test_string.TestUtf8(str6), str6); assert.strictEqual(test_string.TestUtf16(str6), str6); +assert.strictEqual(test_string.TestUtf8External(str6), str6); +assert.strictEqual(test_string.TestUtf16External(str6), str6); assert.strictEqual(test_string.TestUtf8Insufficient(str6), str6.slice(0, 1)); assert.strictEqual(test_string.TestUtf16Insufficient(str6), str6.slice(0, 3)); assert.strictEqual(test_string.Utf16Length(str6), 5); diff --git a/test/js-native-api/test_string/test_string.c b/test/js-native-api/test_string/test_string.c index c78d761fb2ee59..e5378645b395dc 100644 --- a/test/js-native-api/test_string/test_string.c +++ b/test/js-native-api/test_string/test_string.c @@ -1,106 +1,223 @@ #include // INT_MAX +#include #include +#define NAPI_EXPERIMENTAL #include #include "../common.h" #include "test_null.h" -static napi_value TestLatin1(napi_env env, napi_callback_info info) { +static napi_status validate_and_retrieve_single_string_arg( + napi_env env, napi_callback_info info, napi_value* arg) { size_t argc = 1; - napi_value args[1]; - NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); + NODE_API_CHECK_STATUS(napi_get_cb_info(env, info, &argc, arg, NULL, NULL)); - NODE_API_ASSERT(env, argc >= 1, "Wrong number of arguments"); + NODE_API_ASSERT_STATUS(env, argc >= 1, "Wrong number of arguments"); napi_valuetype valuetype; - NODE_API_CALL(env, napi_typeof(env, args[0], &valuetype)); + NODE_API_CHECK_STATUS(napi_typeof(env, *arg, &valuetype)); + + NODE_API_ASSERT_STATUS(env, + valuetype == napi_string, + "Wrong type of argment. Expects a string."); + + return napi_ok; +} - NODE_API_ASSERT(env, valuetype == napi_string, - "Wrong type of argment. Expects a string."); +// Define a free list for collecting "external" strings, which are really +// strings we copy from strings that are coming in from JS. +struct free_list { + void* data; + struct free_list* next; +}; +static void free_items(napi_env env, void* data, void* hint) { + struct free_list* item = data; + while (item != NULL) { + struct free_list* to_free = item; + item = to_free->next; + free(to_free->data); + free(to_free); + } +} +static void insert_item(struct free_list* first_item, void* data) { + struct free_list* new_item = malloc(sizeof(*new_item)); + new_item->next = first_item->next; + first_item->next = new_item; + new_item->data = data; +} + +// These help us factor out code that is common between the bindings. +typedef napi_status (*OneByteCreateAPI)(napi_env, + const char*, + size_t, + napi_value*); +typedef napi_status (*OneByteGetAPI)( + napi_env, napi_value, char*, size_t, size_t*); +typedef napi_status (*TwoByteCreateAPI)(napi_env, + const char16_t*, + size_t, + napi_value*); +typedef napi_status (*TwoByteGetAPI)( + napi_env, napi_value, char16_t*, size_t, size_t*); + +// Test passing back the one-byte string we got from JS as an external string. +static napi_value TestOneByteImpl(napi_env env, + napi_callback_info info, + OneByteGetAPI get_api, + OneByteCreateAPI create_api) { + napi_value args[1]; + NODE_API_CALL(env, validate_and_retrieve_single_string_arg(env, info, args)); char buffer[128]; size_t buffer_size = 128; size_t copied; - NODE_API_CALL(env, - napi_get_value_string_latin1(env, args[0], buffer, buffer_size, &copied)); + NODE_API_CALL(env, get_api(env, args[0], buffer, buffer_size, &copied)); napi_value output; - NODE_API_CALL(env, napi_create_string_latin1(env, buffer, copied, &output)); + NODE_API_CALL(env, create_api(env, buffer, copied, &output)); return output; } -static napi_value TestUtf8(napi_env env, napi_callback_info info) { - size_t argc = 1; +// Test passing back the two-byte string we got from JS as an external string. +static napi_value TestTwoByteImpl(napi_env env, + napi_callback_info info, + TwoByteGetAPI get_api, + TwoByteCreateAPI create_api) { napi_value args[1]; - NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); - - NODE_API_ASSERT(env, argc >= 1, "Wrong number of arguments"); + NODE_API_CALL(env, validate_and_retrieve_single_string_arg(env, info, args)); - napi_valuetype valuetype; - NODE_API_CALL(env, napi_typeof(env, args[0], &valuetype)); - - NODE_API_ASSERT(env, valuetype == napi_string, - "Wrong type of argment. Expects a string."); - - char buffer[128]; + char16_t buffer[128]; size_t buffer_size = 128; size_t copied; - NODE_API_CALL(env, - napi_get_value_string_utf8(env, args[0], buffer, buffer_size, &copied)); + NODE_API_CALL(env, get_api(env, args[0], buffer, buffer_size, &copied)); napi_value output; - NODE_API_CALL(env, napi_create_string_utf8(env, buffer, copied, &output)); + NODE_API_CALL(env, create_api(env, buffer, copied, &output)); return output; } -static napi_value TestUtf16(napi_env env, napi_callback_info info) { - size_t argc = 1; - napi_value args[1]; - NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); +// Common code for copying a stack-allocated string to a heap-allocated one, and +// adding the latter to a free list which will be freed at the end of the test. +// That way, we can treat it as an external string. There's no point factoring +// out such common code for two-byte strings because there's only one API to +// test. +static napi_status create_external_one_byte(napi_env env, + const char* string, + size_t length, + napi_value* result, + OneByteCreateAPI create_api) { + napi_status status; + char* string_copy; + const size_t length_bytes = (length + 1) * sizeof(*string_copy); + string_copy = malloc(length_bytes); + memcpy(string_copy, string, length_bytes); + string_copy[length] = 0; + + status = create_api(env, string_copy, length, result); + if (status != napi_ok) { + free(string_copy); + return status; + } - NODE_API_ASSERT(env, argc >= 1, "Wrong number of arguments"); + struct free_list* first_item; + status = napi_get_instance_data(env, (void**)&first_item); + if (status != napi_ok) { + free(string_copy); + return status; + } - napi_valuetype valuetype; - NODE_API_CALL(env, napi_typeof(env, args[0], &valuetype)); + insert_item(first_item, string_copy); + return napi_ok; +} - NODE_API_ASSERT(env, valuetype == napi_string, - "Wrong type of argment. Expects a string."); +static napi_status create_external_latin1(napi_env env, + const char* string, + size_t length, + napi_value* result) { + return create_external_one_byte( + env, string, length, result, napi_create_external_string_latin1); +} - char16_t buffer[128]; - size_t buffer_size = 128; - size_t copied; +static napi_status create_external_utf8(napi_env env, + const char* string, + size_t length, + napi_value* result) { + return create_external_one_byte( + env, string, length, result, napi_create_external_string_utf8); +} - NODE_API_CALL(env, - napi_get_value_string_utf16(env, args[0], buffer, buffer_size, &copied)); +static napi_status create_external_utf16(napi_env env, + const char16_t* string, + size_t length, + napi_value* result) { + napi_status status; + char16_t* string_copy; + const size_t length_bytes = (length + 1) * sizeof(*string_copy); + string_copy = malloc(length_bytes); + memcpy(string_copy, string, length_bytes); + string_copy[length] = 0; + + status = napi_create_external_string_utf16(env, string_copy, length, result); + if (status != napi_ok) { + free(string_copy); + return status; + } - napi_value output; - NODE_API_CALL(env, napi_create_string_utf16(env, buffer, copied, &output)); + struct free_list* first_item; + status = napi_get_instance_data(env, (void**)&first_item); + if (status != napi_ok) { + free(string_copy); + return status; + } - return output; + insert_item(first_item, string_copy); + return napi_ok; } -static napi_value -TestLatin1Insufficient(napi_env env, napi_callback_info info) { - size_t argc = 1; - napi_value args[1]; - NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); +static napi_value TestLatin1(napi_env env, napi_callback_info info) { + return TestOneByteImpl( + env, info, napi_get_value_string_latin1, napi_create_string_latin1); +} - NODE_API_ASSERT(env, argc >= 1, "Wrong number of arguments"); +static napi_value TestUtf8(napi_env env, napi_callback_info info) { + return TestOneByteImpl( + env, info, napi_get_value_string_utf8, napi_create_string_utf8); +} - napi_valuetype valuetype; - NODE_API_CALL(env, napi_typeof(env, args[0], &valuetype)); +static napi_value TestUtf16(napi_env env, napi_callback_info info) { + return TestTwoByteImpl( + env, info, napi_get_value_string_utf16, napi_create_string_utf16); +} - NODE_API_ASSERT(env, valuetype == napi_string, - "Wrong type of argment. Expects a string."); +static napi_value TestLatin1External(napi_env env, napi_callback_info info) { + return TestOneByteImpl( + env, info, napi_get_value_string_latin1, create_external_latin1); +} + +static napi_value TestUtf8External(napi_env env, napi_callback_info info) { + return TestOneByteImpl( + env, info, napi_get_value_string_utf8, create_external_utf8); +} + +static napi_value TestUtf16External(napi_env env, napi_callback_info info) { + return TestTwoByteImpl( + env, info, napi_get_value_string_utf16, create_external_utf16); +} + +static napi_value TestLatin1Insufficient(napi_env env, + napi_callback_info info) { + napi_value args[1]; + NODE_API_CALL(env, validate_and_retrieve_single_string_arg(env, info, args)); char buffer[4]; size_t buffer_size = 4; size_t copied; - NODE_API_CALL(env, + NODE_API_CALL( + env, napi_get_value_string_latin1(env, args[0], buffer, buffer_size, &copied)); napi_value output; @@ -110,23 +227,15 @@ TestLatin1Insufficient(napi_env env, napi_callback_info info) { } static napi_value TestUtf8Insufficient(napi_env env, napi_callback_info info) { - size_t argc = 1; napi_value args[1]; - NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); - - NODE_API_ASSERT(env, argc >= 1, "Wrong number of arguments"); - - napi_valuetype valuetype; - NODE_API_CALL(env, napi_typeof(env, args[0], &valuetype)); - - NODE_API_ASSERT(env, valuetype == napi_string, - "Wrong type of argment. Expects a string."); + NODE_API_CALL(env, validate_and_retrieve_single_string_arg(env, info, args)); char buffer[4]; size_t buffer_size = 4; size_t copied; - NODE_API_CALL(env, + NODE_API_CALL( + env, napi_get_value_string_utf8(env, args[0], buffer, buffer_size, &copied)); napi_value output; @@ -136,23 +245,15 @@ static napi_value TestUtf8Insufficient(napi_env env, napi_callback_info info) { } static napi_value TestUtf16Insufficient(napi_env env, napi_callback_info info) { - size_t argc = 1; napi_value args[1]; - NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); - - NODE_API_ASSERT(env, argc >= 1, "Wrong number of arguments"); - - napi_valuetype valuetype; - NODE_API_CALL(env, napi_typeof(env, args[0], &valuetype)); - - NODE_API_ASSERT(env, valuetype == napi_string, - "Wrong type of argment. Expects a string."); + NODE_API_CALL(env, validate_and_retrieve_single_string_arg(env, info, args)); char16_t buffer[4]; size_t buffer_size = 4; size_t copied; - NODE_API_CALL(env, + NODE_API_CALL( + env, napi_get_value_string_utf16(env, args[0], buffer, buffer_size, &copied)); napi_value output; @@ -162,20 +263,12 @@ static napi_value TestUtf16Insufficient(napi_env env, napi_callback_info info) { } static napi_value Utf16Length(napi_env env, napi_callback_info info) { - size_t argc = 1; napi_value args[1]; - NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); - - NODE_API_ASSERT(env, argc >= 1, "Wrong number of arguments"); - - napi_valuetype valuetype; - NODE_API_CALL(env, napi_typeof(env, args[0], &valuetype)); - - NODE_API_ASSERT(env, valuetype == napi_string, - "Wrong type of argment. Expects a string."); + NODE_API_CALL(env, validate_and_retrieve_single_string_arg(env, info, args)); size_t length; - NODE_API_CALL(env, napi_get_value_string_utf16(env, args[0], NULL, 0, &length)); + NODE_API_CALL(env, + napi_get_value_string_utf16(env, args[0], NULL, 0, &length)); napi_value output; NODE_API_CALL(env, napi_create_uint32(env, (uint32_t)length, &output)); @@ -184,21 +277,12 @@ static napi_value Utf16Length(napi_env env, napi_callback_info info) { } static napi_value Utf8Length(napi_env env, napi_callback_info info) { - size_t argc = 1; napi_value args[1]; - NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); - - NODE_API_ASSERT(env, argc >= 1, "Wrong number of arguments"); - - napi_valuetype valuetype; - NODE_API_CALL(env, napi_typeof(env, args[0], &valuetype)); - - NODE_API_ASSERT(env, valuetype == napi_string, - "Wrong type of argment. Expects a string."); + NODE_API_CALL(env, validate_and_retrieve_single_string_arg(env, info, args)); size_t length; NODE_API_CALL(env, - napi_get_value_string_utf8(env, args[0], NULL, 0, &length)); + napi_get_value_string_utf8(env, args[0], NULL, 0, &length)); napi_value output; NODE_API_CALL(env, napi_create_uint32(env, (uint32_t)length, &output)); @@ -209,8 +293,8 @@ static napi_value Utf8Length(napi_env env, napi_callback_info info) { static napi_value TestLargeUtf8(napi_env env, napi_callback_info info) { napi_value output; if (SIZE_MAX > INT_MAX) { - NODE_API_CALL(env, - napi_create_string_utf8(env, "", ((size_t)INT_MAX) + 1, &output)); + NODE_API_CALL( + env, napi_create_string_utf8(env, "", ((size_t)INT_MAX) + 1, &output)); } else { // just throw the expected error as there is nothing to test // in this case since we can't overflow @@ -223,7 +307,8 @@ static napi_value TestLargeUtf8(napi_env env, napi_callback_info info) { static napi_value TestLargeLatin1(napi_env env, napi_callback_info info) { napi_value output; if (SIZE_MAX > INT_MAX) { - NODE_API_CALL(env, + NODE_API_CALL( + env, napi_create_string_latin1(env, "", ((size_t)INT_MAX) + 1, &output)); } else { // just throw the expected error as there is nothing to test @@ -237,7 +322,8 @@ static napi_value TestLargeLatin1(napi_env env, napi_callback_info info) { static napi_value TestLargeUtf16(napi_env env, napi_callback_info info) { napi_value output; if (SIZE_MAX > INT_MAX) { - NODE_API_CALL(env, + NODE_API_CALL( + env, napi_create_string_utf16( env, ((const char16_t*)""), ((size_t)INT_MAX) + 1, &output)); } else { @@ -256,10 +342,10 @@ static napi_value TestMemoryCorruption(napi_env env, napi_callback_info info) { NODE_API_ASSERT(env, argc == 1, "Wrong number of arguments"); - char buf[10] = { 0 }; + char buf[10] = {0}; NODE_API_CALL(env, napi_get_value_string_utf8(env, args[0], buf, 0, NULL)); - char zero[10] = { 0 }; + char zero[10] = {0}; if (memcmp(buf, zero, sizeof(buf)) != 0) { NODE_API_CALL(env, napi_throw_error(env, NULL, "Buffer overwritten")); } @@ -269,25 +355,36 @@ static napi_value TestMemoryCorruption(napi_env env, napi_callback_info info) { EXTERN_C_START napi_value Init(napi_env env, napi_value exports) { + struct free_list* first_item = malloc(sizeof(*first_item)); + first_item->next = NULL; + first_item->data = NULL; + NODE_API_CALL(env, napi_set_instance_data(env, first_item, free_items, NULL)); + napi_property_descriptor properties[] = { - DECLARE_NODE_API_PROPERTY("TestLatin1", TestLatin1), - DECLARE_NODE_API_PROPERTY("TestLatin1Insufficient", TestLatin1Insufficient), - DECLARE_NODE_API_PROPERTY("TestUtf8", TestUtf8), - DECLARE_NODE_API_PROPERTY("TestUtf8Insufficient", TestUtf8Insufficient), - DECLARE_NODE_API_PROPERTY("TestUtf16", TestUtf16), - DECLARE_NODE_API_PROPERTY("TestUtf16Insufficient", TestUtf16Insufficient), - DECLARE_NODE_API_PROPERTY("Utf16Length", Utf16Length), - DECLARE_NODE_API_PROPERTY("Utf8Length", Utf8Length), - DECLARE_NODE_API_PROPERTY("TestLargeUtf8", TestLargeUtf8), - DECLARE_NODE_API_PROPERTY("TestLargeLatin1", TestLargeLatin1), - DECLARE_NODE_API_PROPERTY("TestLargeUtf16", TestLargeUtf16), - DECLARE_NODE_API_PROPERTY("TestMemoryCorruption", TestMemoryCorruption), + DECLARE_NODE_API_PROPERTY("TestLatin1", TestLatin1), + DECLARE_NODE_API_PROPERTY("TestLatin1External", TestLatin1External), + DECLARE_NODE_API_PROPERTY("TestLatin1Insufficient", + TestLatin1Insufficient), + DECLARE_NODE_API_PROPERTY("TestUtf8", TestUtf8), + DECLARE_NODE_API_PROPERTY("TestUtf8External", TestUtf8External), + DECLARE_NODE_API_PROPERTY("TestUtf8Insufficient", TestUtf8Insufficient), + DECLARE_NODE_API_PROPERTY("TestUtf16", TestUtf16), + DECLARE_NODE_API_PROPERTY("TestUtf16External", TestUtf16External), + DECLARE_NODE_API_PROPERTY("TestUtf16Insufficient", TestUtf16Insufficient), + DECLARE_NODE_API_PROPERTY("Utf16Length", Utf16Length), + DECLARE_NODE_API_PROPERTY("Utf8Length", Utf8Length), + DECLARE_NODE_API_PROPERTY("TestLargeUtf8", TestLargeUtf8), + DECLARE_NODE_API_PROPERTY("TestLargeLatin1", TestLargeLatin1), + DECLARE_NODE_API_PROPERTY("TestLargeUtf16", TestLargeUtf16), + DECLARE_NODE_API_PROPERTY("TestMemoryCorruption", TestMemoryCorruption), }; init_test_null(env, exports); - NODE_API_CALL(env, napi_define_properties( - env, exports, sizeof(properties) / sizeof(*properties), properties)); + NODE_API_CALL( + env, + napi_define_properties( + env, exports, sizeof(properties) / sizeof(*properties), properties)); return exports; } diff --git a/test/node-api/test_reference_by_node_api_version/test_reference_by_node_api_version.c b/test/node-api/test_reference_by_node_api_version/test_reference_by_node_api_version.c index 23e5b1988b441c..ae4fa7cb0e8856 100644 --- a/test/node-api/test_reference_by_node_api_version/test_reference_by_node_api_version.c +++ b/test/node-api/test_reference_by_node_api_version/test_reference_by_node_api_version.c @@ -2,17 +2,6 @@ #include "../../js-native-api/common.h" #include "stdlib.h" -#define NODE_API_ASSERT_STATUS(env, assertion, message) \ - NODE_API_ASSERT_BASE(env, assertion, message, napi_generic_failure) - -#define NODE_API_CHECK_STATUS(env, the_call) \ - do { \ - napi_status status = (the_call); \ - if (status != napi_ok) { \ - return status; \ - } \ - } while (0) - static uint32_t finalizeCount = 0; static void FreeData(napi_env env, void* data, void* hint) { @@ -29,7 +18,7 @@ static napi_status GetArgValue(napi_env env, napi_value* argValue) { size_t argc = 1; NODE_API_CHECK_STATUS( - env, napi_get_cb_info(env, info, &argc, argValue, NULL, NULL)); + napi_get_cb_info(env, info, &argc, argValue, NULL, NULL)); NODE_API_ASSERT_STATUS(env, argc == 1, "Expects one arg."); return napi_ok; @@ -39,10 +28,10 @@ static napi_status GetArgValueAsIndex(napi_env env, napi_callback_info info, uint32_t* index) { napi_value argValue; - NODE_API_CHECK_STATUS(env, GetArgValue(env, info, &argValue)); + NODE_API_CHECK_STATUS(GetArgValue(env, info, &argValue)); napi_valuetype valueType; - NODE_API_CHECK_STATUS(env, napi_typeof(env, argValue, &valueType)); + NODE_API_CHECK_STATUS(napi_typeof(env, argValue, &valueType)); NODE_API_ASSERT_STATUS( env, valueType == napi_number, "Argument must be a number."); @@ -53,10 +42,10 @@ static napi_status GetRef(napi_env env, napi_callback_info info, napi_ref* ref) { uint32_t index; - NODE_API_CHECK_STATUS(env, GetArgValueAsIndex(env, info, &index)); + NODE_API_CHECK_STATUS(GetArgValueAsIndex(env, info, &index)); napi_ref* refValues; - NODE_API_CHECK_STATUS(env, napi_get_instance_data(env, (void**)&refValues)); + NODE_API_CHECK_STATUS(napi_get_instance_data(env, (void**)&refValues)); NODE_API_ASSERT_STATUS(env, refValues != NULL, "Cannot get instance data."); *ref = refValues[index]; From e1127d16ab7c6eb9384cf6d23e9a79490d5330da Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sun, 4 Jun 2023 11:04:25 -0700 Subject: [PATCH 05/31] rename to node_api_* --- src/js_native_api.h | 6 +++--- src/js_native_api_v8.cc | 18 ++++++------------ test/js-native-api/test_string/test_string.c | 7 ++++--- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/src/js_native_api.h b/src/js_native_api.h index 0cf0ba461ff784..4b10993bd93403 100644 --- a/src/js_native_api.h +++ b/src/js_native_api.h @@ -93,11 +93,11 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_create_string_utf16(napi_env env, size_t length, napi_value* result); #ifdef NAPI_EXPERIMENTAL -NAPI_EXTERN napi_status NAPI_CDECL napi_create_external_string_latin1( +NAPI_EXTERN napi_status NAPI_CDECL node_api_create_external_string_latin1( napi_env env, const char* str, size_t length, napi_value* result); -NAPI_EXTERN napi_status NAPI_CDECL napi_create_external_string_utf8( +NAPI_EXTERN napi_status NAPI_CDECL node_api_create_external_string_utf8( napi_env env, const char* str, size_t length, napi_value* result); -NAPI_EXTERN napi_status NAPI_CDECL napi_create_external_string_utf16( +NAPI_EXTERN napi_status NAPI_CDECL node_api_create_external_string_utf16( napi_env env, const char16_t* str, size_t length, napi_value* result); #endif // NAPI_EXPERIMENTAL NAPI_EXTERN napi_status NAPI_CDECL napi_create_symbol(napi_env env, diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 80ede2c89032bf..f3f8a8e12d896f 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -1463,10 +1463,8 @@ napi_status NAPI_CDECL napi_create_string_utf16(napi_env env, }); } -napi_status NAPI_CDECL napi_create_external_string_latin1(napi_env env, - const char* str, - size_t length, - napi_value* result) { +napi_status NAPI_CDECL node_api_create_external_string_latin1( + napi_env env, const char* str, size_t length, napi_value* result) { #if defined(V8_ENABLE_SANDBOX) return napi_create_string_latin1(env, str, length, result); #else @@ -1477,17 +1475,13 @@ napi_status NAPI_CDECL napi_create_external_string_latin1(napi_env env, #endif // V8_ENABLE_SANDBOX } -napi_status NAPI_CDECL napi_create_external_string_utf8(napi_env env, - const char* str, - size_t length, - napi_value* result) { +napi_status NAPI_CDECL node_api_create_external_string_utf8( + napi_env env, const char* str, size_t length, napi_value* result) { return napi_create_string_utf8(env, str, length, result); } -napi_status NAPI_CDECL napi_create_external_string_utf16(napi_env env, - const char16_t* str, - size_t length, - napi_value* result) { +napi_status NAPI_CDECL node_api_create_external_string_utf16( + napi_env env, const char16_t* str, size_t length, napi_value* result) { #if defined(V8_ENABLE_SANDBOX) return napi_create_string_utf16(env, str, length, result); #else diff --git a/test/js-native-api/test_string/test_string.c b/test/js-native-api/test_string/test_string.c index e5378645b395dc..a942aef5f9858d 100644 --- a/test/js-native-api/test_string/test_string.c +++ b/test/js-native-api/test_string/test_string.c @@ -138,7 +138,7 @@ static napi_status create_external_latin1(napi_env env, size_t length, napi_value* result) { return create_external_one_byte( - env, string, length, result, napi_create_external_string_latin1); + env, string, length, result, node_api_create_external_string_latin1); } static napi_status create_external_utf8(napi_env env, @@ -146,7 +146,7 @@ static napi_status create_external_utf8(napi_env env, size_t length, napi_value* result) { return create_external_one_byte( - env, string, length, result, napi_create_external_string_utf8); + env, string, length, result, node_api_create_external_string_utf8); } static napi_status create_external_utf16(napi_env env, @@ -160,7 +160,8 @@ static napi_status create_external_utf16(napi_env env, memcpy(string_copy, string, length_bytes); string_copy[length] = 0; - status = napi_create_external_string_utf16(env, string_copy, length, result); + status = + node_api_create_external_string_utf16(env, string_copy, length, result); if (status != napi_ok) { free(string_copy); return status; From 953304c4df66e2dcf8d4bfde783397133ca328e2 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sun, 4 Jun 2023 21:15:41 -0700 Subject: [PATCH 06/31] put back build type --- test/js-native-api/test_string/test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/js-native-api/test_string/test.js b/test/js-native-api/test_string/test.js index 550aa04dac06f5..660e67dfb560cf 100644 --- a/test/js-native-api/test_string/test.js +++ b/test/js-native-api/test_string/test.js @@ -3,8 +3,7 @@ const common = require('../../common'); const assert = require('assert'); // Testing api calls for string -// const test_string = require(`./build/${common.buildType}/test_string`); -const test_string = require(`./build/Debug/test_string`); +const test_string = require(`./build/${common.buildType}/test_string`); const empty = ''; assert.strictEqual(test_string.TestLatin1(empty), empty); From 09f7cdf79e96faecfeca4c451b8185373c1e2db8 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sun, 4 Jun 2023 21:33:28 -0700 Subject: [PATCH 07/31] make linter work --- Makefile | 1 - benchmark/napi/string/index.js | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 455add478aaaa8..8e10ec6ad21fbb 100644 --- a/Makefile +++ b/Makefile @@ -1394,7 +1394,6 @@ LINT_CPP_EXCLUDE += src/tracing/trace_event.h src/tracing/trace_event_common.h LINT_CPP_FILES = $(filter-out $(LINT_CPP_EXCLUDE), $(wildcard \ benchmark/napi/*/*.cc \ - benchmark/napi/*/*.c \ src/*.c \ src/*.cc \ src/*.h \ diff --git a/benchmark/napi/string/index.js b/benchmark/napi/string/index.js index 434a9d86132c20..2d2f82b39bdcaf 100644 --- a/benchmark/napi/string/index.js +++ b/benchmark/napi/string/index.js @@ -11,7 +11,7 @@ try { const bench = common.createBenchmark(main, { n: [1e5, 1e6, 1e7], - stringType: ['Latin1', 'Utf8', 'Utf16'] + stringType: ['Latin1', 'Utf8', 'Utf16'], }); function main({ n, stringType }) { From 902cb33307f6c15408782fb0673142ba8d9be5bd Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Mon, 5 Jun 2023 20:14:04 -0700 Subject: [PATCH 08/31] rename as requested --- src/js_native_api_v8.cc | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index f3f8a8e12d896f..68e215976c93a4 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -65,25 +65,25 @@ class ExternalOneByteStringResource : public v8::String::ExternalOneByteStringResource { public: ExternalOneByteStringResource(const char* string, const size_t length) - : _string(string), _length(length) {} - const char* data() const { return _string; } - size_t length() const { return _length; } + : string_(string), length_(length) {} + const char* data() const { return string_; } + size_t length() const { return length_; } private: - const char* _string; - const size_t _length; + const char* string_; + const size_t length_; }; class ExternalStringResource : public v8::String::ExternalStringResource { public: ExternalStringResource(const uint16_t* string, const size_t length) - : _string(string), _length(length) {} - const uint16_t* data() const { return _string; } - size_t length() const { return _length; } + : string_(string), length_(length) {} + const uint16_t* data() const { return string_; } + size_t length() const { return length_; } private: - const uint16_t* _string; - const size_t _length; + const uint16_t* string_; + const size_t length_; }; template From 754864eefa47edae6b1be308ff6bc04d839f35d4 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Tue, 6 Jun 2023 08:13:18 -0700 Subject: [PATCH 09/31] use std::basic_string_view to calc length in case of NAPI_AUTO_LENGTH --- src/js_native_api_v8.cc | 6 ++++++ test/js-native-api/test_string/test_string.c | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 68e215976c93a4..8e8fe1706b3ea4 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -1469,6 +1469,9 @@ napi_status NAPI_CDECL node_api_create_external_string_latin1( return napi_create_string_latin1(env, str, length, result); #else return v8impl::NewString(env, str, length, result, [&](v8::Isolate* isolate) { + if (length == NAPI_AUTO_LENGTH) { + length = (std::string_view(str)).length(); + } auto resource = new v8impl::ExternalOneByteStringResource(str, length); return v8::String::NewExternalOneByte(isolate, resource); }); @@ -1486,6 +1489,9 @@ napi_status NAPI_CDECL node_api_create_external_string_utf16( return napi_create_string_utf16(env, str, length, result); #else return v8impl::NewString(env, str, length, result, [&](v8::Isolate* isolate) { + if (length == NAPI_AUTO_LENGTH) { + length = (std::u16string_view(str)).length(); + } auto resource = new v8impl::ExternalStringResource( reinterpret_cast(str), length); return v8::String::NewExternalTwoByte(isolate, resource); diff --git a/test/js-native-api/test_string/test_string.c b/test/js-native-api/test_string/test_string.c index a942aef5f9858d..5bd117da1109e8 100644 --- a/test/js-native-api/test_string/test_string.c +++ b/test/js-native-api/test_string/test_string.c @@ -6,6 +6,11 @@ #include "../common.h" #include "test_null.h" +enum length_mode { + own_length, + auto_length +}; + static napi_status validate_and_retrieve_single_string_arg( napi_env env, napi_callback_info info, napi_value* arg) { size_t argc = 1; From 95a9971ab80378d9dec3c6b28bb5232bd9882466 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Tue, 6 Jun 2023 23:01:02 -0700 Subject: [PATCH 10/31] test for auto-length --- test/js-native-api/test_string/test.js | 40 ++++++++++ test/js-native-api/test_string/test_string.c | 81 +++++++++++++++++--- 2 files changed, 109 insertions(+), 12 deletions(-) diff --git a/test/js-native-api/test_string/test.js b/test/js-native-api/test_string/test.js index 660e67dfb560cf..fa4d0fd24ecb52 100644 --- a/test/js-native-api/test_string/test.js +++ b/test/js-native-api/test_string/test.js @@ -9,9 +9,15 @@ const empty = ''; assert.strictEqual(test_string.TestLatin1(empty), empty); assert.strictEqual(test_string.TestUtf8(empty), empty); assert.strictEqual(test_string.TestUtf16(empty), empty); +assert.strictEqual(test_string.TestLatin1AutoLength(empty), empty); +assert.strictEqual(test_string.TestUtf8AutoLength(empty), empty); +assert.strictEqual(test_string.TestUtf16AutoLength(empty), empty); assert.strictEqual(test_string.TestLatin1External(empty), empty); assert.strictEqual(test_string.TestUtf8External(empty), empty); assert.strictEqual(test_string.TestUtf16External(empty), empty); +assert.strictEqual(test_string.TestLatin1ExternalAutoLength(empty), empty); +assert.strictEqual(test_string.TestUtf8ExternalAutoLength(empty), empty); +assert.strictEqual(test_string.TestUtf16ExternalAutoLength(empty), empty); assert.strictEqual(test_string.Utf16Length(empty), 0); assert.strictEqual(test_string.Utf8Length(empty), 0); @@ -19,9 +25,15 @@ const str1 = 'hello world'; assert.strictEqual(test_string.TestLatin1(str1), str1); assert.strictEqual(test_string.TestUtf8(str1), str1); assert.strictEqual(test_string.TestUtf16(str1), str1); +assert.strictEqual(test_string.TestLatin1AutoLength(str1), str1); +assert.strictEqual(test_string.TestUtf8AutoLength(str1), str1); +assert.strictEqual(test_string.TestUtf16AutoLength(str1), str1); assert.strictEqual(test_string.TestLatin1External(str1), str1); assert.strictEqual(test_string.TestUtf8External(str1), str1); assert.strictEqual(test_string.TestUtf16External(str1), str1); +assert.strictEqual(test_string.TestLatin1ExternalAutoLength(str1), str1); +assert.strictEqual(test_string.TestUtf8ExternalAutoLength(str1), str1); +assert.strictEqual(test_string.TestUtf16ExternalAutoLength(str1), str1); assert.strictEqual(test_string.TestLatin1Insufficient(str1), str1.slice(0, 3)); assert.strictEqual(test_string.TestUtf8Insufficient(str1), str1.slice(0, 3)); assert.strictEqual(test_string.TestUtf16Insufficient(str1), str1.slice(0, 3)); @@ -32,9 +44,15 @@ const str2 = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'; assert.strictEqual(test_string.TestLatin1(str2), str2); assert.strictEqual(test_string.TestUtf8(str2), str2); assert.strictEqual(test_string.TestUtf16(str2), str2); +assert.strictEqual(test_string.TestLatin1AutoLength(str2), str2); +assert.strictEqual(test_string.TestUtf8AutoLength(str2), str2); +assert.strictEqual(test_string.TestUtf16AutoLength(str2), str2); assert.strictEqual(test_string.TestLatin1External(str2), str2); assert.strictEqual(test_string.TestUtf8External(str2), str2); assert.strictEqual(test_string.TestUtf16External(str2), str2); +assert.strictEqual(test_string.TestLatin1ExternalAutoLength(str2), str2); +assert.strictEqual(test_string.TestUtf8ExternalAutoLength(str2), str2); +assert.strictEqual(test_string.TestUtf16ExternalAutoLength(str2), str2); assert.strictEqual(test_string.TestLatin1Insufficient(str2), str2.slice(0, 3)); assert.strictEqual(test_string.TestUtf8Insufficient(str2), str2.slice(0, 3)); assert.strictEqual(test_string.TestUtf16Insufficient(str2), str2.slice(0, 3)); @@ -45,9 +63,15 @@ const str3 = '?!@#$%^&*()_+-=[]{}/.,<>\'"\\'; assert.strictEqual(test_string.TestLatin1(str3), str3); assert.strictEqual(test_string.TestUtf8(str3), str3); assert.strictEqual(test_string.TestUtf16(str3), str3); +assert.strictEqual(test_string.TestLatin1AutoLength(str3), str3); +assert.strictEqual(test_string.TestUtf8AutoLength(str3), str3); +assert.strictEqual(test_string.TestUtf16AutoLength(str3), str3); assert.strictEqual(test_string.TestLatin1External(str3), str3); assert.strictEqual(test_string.TestUtf8External(str3), str3); assert.strictEqual(test_string.TestUtf16External(str3), str3); +assert.strictEqual(test_string.TestLatin1ExternalAutoLength(str3), str3); +assert.strictEqual(test_string.TestUtf8ExternalAutoLength(str3), str3); +assert.strictEqual(test_string.TestUtf16ExternalAutoLength(str3), str3); assert.strictEqual(test_string.TestLatin1Insufficient(str3), str3.slice(0, 3)); assert.strictEqual(test_string.TestUtf8Insufficient(str3), str3.slice(0, 3)); assert.strictEqual(test_string.TestUtf16Insufficient(str3), str3.slice(0, 3)); @@ -58,9 +82,15 @@ const str4 = '¡¢£¤¥¦§¨©ª«¬­®¯°±²³´µ¶·¸¹º»¼½¾¿'; assert.strictEqual(test_string.TestLatin1(str4), str4); assert.strictEqual(test_string.TestUtf8(str4), str4); assert.strictEqual(test_string.TestUtf16(str4), str4); +assert.strictEqual(test_string.TestLatin1AutoLength(str4), str4); +assert.strictEqual(test_string.TestUtf8AutoLength(str4), str4); +assert.strictEqual(test_string.TestUtf16AutoLength(str4), str4); assert.strictEqual(test_string.TestLatin1External(str4), str4); assert.strictEqual(test_string.TestUtf8External(str4), str4); assert.strictEqual(test_string.TestUtf16External(str4), str4); +assert.strictEqual(test_string.TestLatin1ExternalAutoLength(str4), str4); +assert.strictEqual(test_string.TestUtf8ExternalAutoLength(str4), str4); +assert.strictEqual(test_string.TestUtf16ExternalAutoLength(str4), str4); assert.strictEqual(test_string.TestLatin1Insufficient(str4), str4.slice(0, 3)); assert.strictEqual(test_string.TestUtf8Insufficient(str4), str4.slice(0, 1)); assert.strictEqual(test_string.TestUtf16Insufficient(str4), str4.slice(0, 3)); @@ -71,9 +101,15 @@ const str5 = 'ÀÁÂÃÄÅÆÇÈÉÊËÌÍÎÏÐÑÒÓÔÕÖ×ØÙÚÛÜÝÞßà assert.strictEqual(test_string.TestLatin1(str5), str5); assert.strictEqual(test_string.TestUtf8(str5), str5); assert.strictEqual(test_string.TestUtf16(str5), str5); +assert.strictEqual(test_string.TestLatin1AutoLength(str5), str5); +assert.strictEqual(test_string.TestUtf8AutoLength(str5), str5); +assert.strictEqual(test_string.TestUtf16AutoLength(str5), str5); assert.strictEqual(test_string.TestLatin1External(str5), str5); assert.strictEqual(test_string.TestUtf8External(str5), str5); assert.strictEqual(test_string.TestUtf16External(str5), str5); +assert.strictEqual(test_string.TestLatin1ExternalAutoLength(str5), str5); +assert.strictEqual(test_string.TestUtf8ExternalAutoLength(str5), str5); +assert.strictEqual(test_string.TestUtf16ExternalAutoLength(str5), str5); assert.strictEqual(test_string.TestLatin1Insufficient(str5), str5.slice(0, 3)); assert.strictEqual(test_string.TestUtf8Insufficient(str5), str5.slice(0, 1)); assert.strictEqual(test_string.TestUtf16Insufficient(str5), str5.slice(0, 3)); @@ -83,8 +119,12 @@ assert.strictEqual(test_string.Utf8Length(str5), 126); const str6 = '\u{2003}\u{2101}\u{2001}\u{202}\u{2011}'; assert.strictEqual(test_string.TestUtf8(str6), str6); assert.strictEqual(test_string.TestUtf16(str6), str6); +assert.strictEqual(test_string.TestUtf8AutoLength(str6), str6); +assert.strictEqual(test_string.TestUtf16AutoLength(str6), str6); assert.strictEqual(test_string.TestUtf8External(str6), str6); assert.strictEqual(test_string.TestUtf16External(str6), str6); +assert.strictEqual(test_string.TestUtf8ExternalAutoLength(str6), str6); +assert.strictEqual(test_string.TestUtf16ExternalAutoLength(str6), str6); assert.strictEqual(test_string.TestUtf8Insufficient(str6), str6.slice(0, 1)); assert.strictEqual(test_string.TestUtf16Insufficient(str6), str6.slice(0, 3)); assert.strictEqual(test_string.Utf16Length(str6), 5); diff --git a/test/js-native-api/test_string/test_string.c b/test/js-native-api/test_string/test_string.c index 5bd117da1109e8..70ca8cfde1bbdf 100644 --- a/test/js-native-api/test_string/test_string.c +++ b/test/js-native-api/test_string/test_string.c @@ -6,8 +6,8 @@ #include "../common.h" #include "test_null.h" -enum length_mode { - own_length, +enum length_type { + actual_length, auto_length }; @@ -68,7 +68,8 @@ typedef napi_status (*TwoByteGetAPI)( static napi_value TestOneByteImpl(napi_env env, napi_callback_info info, OneByteGetAPI get_api, - OneByteCreateAPI create_api) { + OneByteCreateAPI create_api, + enum length_type length_mode) { napi_value args[1]; NODE_API_CALL(env, validate_and_retrieve_single_string_arg(env, info, args)); @@ -79,6 +80,9 @@ static napi_value TestOneByteImpl(napi_env env, NODE_API_CALL(env, get_api(env, args[0], buffer, buffer_size, &copied)); napi_value output; + if (length_mode == auto_length) { + copied = NAPI_AUTO_LENGTH; + } NODE_API_CALL(env, create_api(env, buffer, copied, &output)); return output; @@ -88,7 +92,8 @@ static napi_value TestOneByteImpl(napi_env env, static napi_value TestTwoByteImpl(napi_env env, napi_callback_info info, TwoByteGetAPI get_api, - TwoByteCreateAPI create_api) { + TwoByteCreateAPI create_api, + enum length_type length_mode) { napi_value args[1]; NODE_API_CALL(env, validate_and_retrieve_single_string_arg(env, info, args)); @@ -99,6 +104,9 @@ static napi_value TestTwoByteImpl(napi_env env, NODE_API_CALL(env, get_api(env, args[0], buffer, buffer_size, &copied)); napi_value output; + if (length_mode == auto_length) { + copied = NAPI_AUTO_LENGTH; + } NODE_API_CALL(env, create_api(env, buffer, copied, &output)); return output; @@ -116,7 +124,8 @@ static napi_status create_external_one_byte(napi_env env, OneByteCreateAPI create_api) { napi_status status; char* string_copy; - const size_t length_bytes = (length + 1) * sizeof(*string_copy); + const size_t actual_length = (length == NAPI_AUTO_LENGTH ? strlen(string) : length); + const size_t length_bytes = (actual_length + 1) * sizeof(*string_copy); string_copy = malloc(length_bytes); memcpy(string_copy, string, length_bytes); string_copy[length] = 0; @@ -154,13 +163,25 @@ static napi_status create_external_utf8(napi_env env, env, string, length, result, node_api_create_external_string_utf8); } +// strlen for char16_t. Needed in case we're copying a string of length +// NAPI_AUTO_LENGTH. +static size_t strlen16(const char16_t* string) { + for (const char16_t* iter = string;; iter++) { + if (*iter == 0) { + return iter - string; + } + } + return NAPI_AUTO_LENGTH; +} + static napi_status create_external_utf16(napi_env env, const char16_t* string, size_t length, napi_value* result) { napi_status status; char16_t* string_copy; - const size_t length_bytes = (length + 1) * sizeof(*string_copy); + const size_t actual_length = (length == NAPI_AUTO_LENGTH ? strlen16(string) : length); + const size_t length_bytes = (actual_length + 1) * sizeof(*string_copy); string_copy = malloc(length_bytes); memcpy(string_copy, string, length_bytes); string_copy[length] = 0; @@ -185,32 +206,62 @@ static napi_status create_external_utf16(napi_env env, static napi_value TestLatin1(napi_env env, napi_callback_info info) { return TestOneByteImpl( - env, info, napi_get_value_string_latin1, napi_create_string_latin1); + env, info, napi_get_value_string_latin1, napi_create_string_latin1, actual_length); } static napi_value TestUtf8(napi_env env, napi_callback_info info) { return TestOneByteImpl( - env, info, napi_get_value_string_utf8, napi_create_string_utf8); + env, info, napi_get_value_string_utf8, napi_create_string_utf8, actual_length); } static napi_value TestUtf16(napi_env env, napi_callback_info info) { return TestTwoByteImpl( - env, info, napi_get_value_string_utf16, napi_create_string_utf16); + env, info, napi_get_value_string_utf16, napi_create_string_utf16, actual_length); +} + +static napi_value TestLatin1AutoLength(napi_env env, napi_callback_info info) { + return TestOneByteImpl( + env, info, napi_get_value_string_latin1, napi_create_string_latin1, auto_length); +} + +static napi_value TestUtf8AutoLength(napi_env env, napi_callback_info info) { + return TestOneByteImpl( + env, info, napi_get_value_string_utf8, napi_create_string_utf8, auto_length); +} + +static napi_value TestUtf16AutoLength(napi_env env, napi_callback_info info) { + return TestTwoByteImpl( + env, info, napi_get_value_string_utf16, napi_create_string_utf16, auto_length); } static napi_value TestLatin1External(napi_env env, napi_callback_info info) { return TestOneByteImpl( - env, info, napi_get_value_string_latin1, create_external_latin1); + env, info, napi_get_value_string_latin1, create_external_latin1, actual_length); } static napi_value TestUtf8External(napi_env env, napi_callback_info info) { return TestOneByteImpl( - env, info, napi_get_value_string_utf8, create_external_utf8); + env, info, napi_get_value_string_utf8, create_external_utf8, actual_length); } static napi_value TestUtf16External(napi_env env, napi_callback_info info) { return TestTwoByteImpl( - env, info, napi_get_value_string_utf16, create_external_utf16); + env, info, napi_get_value_string_utf16, create_external_utf16, actual_length); +} + +static napi_value TestLatin1ExternalAutoLength(napi_env env, napi_callback_info info) { + return TestOneByteImpl( + env, info, napi_get_value_string_latin1, create_external_latin1, auto_length); +} + +static napi_value TestUtf8ExternalAutoLength(napi_env env, napi_callback_info info) { + return TestOneByteImpl( + env, info, napi_get_value_string_utf8, create_external_utf8, auto_length); +} + +static napi_value TestUtf16ExternalAutoLength(napi_env env, napi_callback_info info) { + return TestTwoByteImpl( + env, info, napi_get_value_string_utf16, create_external_utf16, auto_length); } static napi_value TestLatin1Insufficient(napi_env env, @@ -368,14 +419,20 @@ napi_value Init(napi_env env, napi_value exports) { napi_property_descriptor properties[] = { DECLARE_NODE_API_PROPERTY("TestLatin1", TestLatin1), + DECLARE_NODE_API_PROPERTY("TestLatin1AutoLength", TestLatin1AutoLength), DECLARE_NODE_API_PROPERTY("TestLatin1External", TestLatin1External), + DECLARE_NODE_API_PROPERTY("TestLatin1ExternalAutoLength", TestLatin1ExternalAutoLength), DECLARE_NODE_API_PROPERTY("TestLatin1Insufficient", TestLatin1Insufficient), DECLARE_NODE_API_PROPERTY("TestUtf8", TestUtf8), + DECLARE_NODE_API_PROPERTY("TestUtf8AutoLength", TestUtf8AutoLength), DECLARE_NODE_API_PROPERTY("TestUtf8External", TestUtf8External), + DECLARE_NODE_API_PROPERTY("TestUtf8ExternalAutoLength", TestUtf8ExternalAutoLength), DECLARE_NODE_API_PROPERTY("TestUtf8Insufficient", TestUtf8Insufficient), DECLARE_NODE_API_PROPERTY("TestUtf16", TestUtf16), + DECLARE_NODE_API_PROPERTY("TestUtf16AutoLength", TestUtf16AutoLength), DECLARE_NODE_API_PROPERTY("TestUtf16External", TestUtf16External), + DECLARE_NODE_API_PROPERTY("TestUtf16ExternalAutoLength", TestUtf16ExternalAutoLength), DECLARE_NODE_API_PROPERTY("TestUtf16Insufficient", TestUtf16Insufficient), DECLARE_NODE_API_PROPERTY("Utf16Length", Utf16Length), DECLARE_NODE_API_PROPERTY("Utf8Length", Utf8Length), From b4f383b846a571fe93d9adb0141e24c462fcc5a3 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Tue, 6 Jun 2023 23:03:08 -0700 Subject: [PATCH 11/31] format --- test/js-native-api/test_string/test_string.c | 106 +++++++++++++------ 1 file changed, 72 insertions(+), 34 deletions(-) diff --git a/test/js-native-api/test_string/test_string.c b/test/js-native-api/test_string/test_string.c index 70ca8cfde1bbdf..5a5b9dcdc1784b 100644 --- a/test/js-native-api/test_string/test_string.c +++ b/test/js-native-api/test_string/test_string.c @@ -6,10 +6,7 @@ #include "../common.h" #include "test_null.h" -enum length_type { - actual_length, - auto_length -}; +enum length_type { actual_length, auto_length }; static napi_status validate_and_retrieve_single_string_arg( napi_env env, napi_callback_info info, napi_value* arg) { @@ -124,7 +121,8 @@ static napi_status create_external_one_byte(napi_env env, OneByteCreateAPI create_api) { napi_status status; char* string_copy; - const size_t actual_length = (length == NAPI_AUTO_LENGTH ? strlen(string) : length); + const size_t actual_length = + (length == NAPI_AUTO_LENGTH ? strlen(string) : length); const size_t length_bytes = (actual_length + 1) * sizeof(*string_copy); string_copy = malloc(length_bytes); memcpy(string_copy, string, length_bytes); @@ -180,7 +178,8 @@ static napi_status create_external_utf16(napi_env env, napi_value* result) { napi_status status; char16_t* string_copy; - const size_t actual_length = (length == NAPI_AUTO_LENGTH ? strlen16(string) : length); + const size_t actual_length = + (length == NAPI_AUTO_LENGTH ? strlen16(string) : length); const size_t length_bytes = (actual_length + 1) * sizeof(*string_copy); string_copy = malloc(length_bytes); memcpy(string_copy, string, length_bytes); @@ -205,63 +204,99 @@ static napi_status create_external_utf16(napi_env env, } static napi_value TestLatin1(napi_env env, napi_callback_info info) { - return TestOneByteImpl( - env, info, napi_get_value_string_latin1, napi_create_string_latin1, actual_length); + return TestOneByteImpl(env, + info, + napi_get_value_string_latin1, + napi_create_string_latin1, + actual_length); } static napi_value TestUtf8(napi_env env, napi_callback_info info) { - return TestOneByteImpl( - env, info, napi_get_value_string_utf8, napi_create_string_utf8, actual_length); + return TestOneByteImpl(env, + info, + napi_get_value_string_utf8, + napi_create_string_utf8, + actual_length); } static napi_value TestUtf16(napi_env env, napi_callback_info info) { - return TestTwoByteImpl( - env, info, napi_get_value_string_utf16, napi_create_string_utf16, actual_length); + return TestTwoByteImpl(env, + info, + napi_get_value_string_utf16, + napi_create_string_utf16, + actual_length); } static napi_value TestLatin1AutoLength(napi_env env, napi_callback_info info) { - return TestOneByteImpl( - env, info, napi_get_value_string_latin1, napi_create_string_latin1, auto_length); + return TestOneByteImpl(env, + info, + napi_get_value_string_latin1, + napi_create_string_latin1, + auto_length); } static napi_value TestUtf8AutoLength(napi_env env, napi_callback_info info) { - return TestOneByteImpl( - env, info, napi_get_value_string_utf8, napi_create_string_utf8, auto_length); + return TestOneByteImpl(env, + info, + napi_get_value_string_utf8, + napi_create_string_utf8, + auto_length); } static napi_value TestUtf16AutoLength(napi_env env, napi_callback_info info) { - return TestTwoByteImpl( - env, info, napi_get_value_string_utf16, napi_create_string_utf16, auto_length); + return TestTwoByteImpl(env, + info, + napi_get_value_string_utf16, + napi_create_string_utf16, + auto_length); } static napi_value TestLatin1External(napi_env env, napi_callback_info info) { - return TestOneByteImpl( - env, info, napi_get_value_string_latin1, create_external_latin1, actual_length); + return TestOneByteImpl(env, + info, + napi_get_value_string_latin1, + create_external_latin1, + actual_length); } static napi_value TestUtf8External(napi_env env, napi_callback_info info) { - return TestOneByteImpl( - env, info, napi_get_value_string_utf8, create_external_utf8, actual_length); + return TestOneByteImpl(env, + info, + napi_get_value_string_utf8, + create_external_utf8, + actual_length); } static napi_value TestUtf16External(napi_env env, napi_callback_info info) { - return TestTwoByteImpl( - env, info, napi_get_value_string_utf16, create_external_utf16, actual_length); + return TestTwoByteImpl(env, + info, + napi_get_value_string_utf16, + create_external_utf16, + actual_length); } -static napi_value TestLatin1ExternalAutoLength(napi_env env, napi_callback_info info) { - return TestOneByteImpl( - env, info, napi_get_value_string_latin1, create_external_latin1, auto_length); +static napi_value TestLatin1ExternalAutoLength(napi_env env, + napi_callback_info info) { + return TestOneByteImpl(env, + info, + napi_get_value_string_latin1, + create_external_latin1, + auto_length); } -static napi_value TestUtf8ExternalAutoLength(napi_env env, napi_callback_info info) { +static napi_value TestUtf8ExternalAutoLength(napi_env env, + napi_callback_info info) { return TestOneByteImpl( env, info, napi_get_value_string_utf8, create_external_utf8, auto_length); } -static napi_value TestUtf16ExternalAutoLength(napi_env env, napi_callback_info info) { - return TestTwoByteImpl( - env, info, napi_get_value_string_utf16, create_external_utf16, auto_length); +static napi_value TestUtf16ExternalAutoLength(napi_env env, + napi_callback_info info) { + return TestTwoByteImpl(env, + info, + napi_get_value_string_utf16, + create_external_utf16, + auto_length); } static napi_value TestLatin1Insufficient(napi_env env, @@ -421,18 +456,21 @@ napi_value Init(napi_env env, napi_value exports) { DECLARE_NODE_API_PROPERTY("TestLatin1", TestLatin1), DECLARE_NODE_API_PROPERTY("TestLatin1AutoLength", TestLatin1AutoLength), DECLARE_NODE_API_PROPERTY("TestLatin1External", TestLatin1External), - DECLARE_NODE_API_PROPERTY("TestLatin1ExternalAutoLength", TestLatin1ExternalAutoLength), + DECLARE_NODE_API_PROPERTY("TestLatin1ExternalAutoLength", + TestLatin1ExternalAutoLength), DECLARE_NODE_API_PROPERTY("TestLatin1Insufficient", TestLatin1Insufficient), DECLARE_NODE_API_PROPERTY("TestUtf8", TestUtf8), DECLARE_NODE_API_PROPERTY("TestUtf8AutoLength", TestUtf8AutoLength), DECLARE_NODE_API_PROPERTY("TestUtf8External", TestUtf8External), - DECLARE_NODE_API_PROPERTY("TestUtf8ExternalAutoLength", TestUtf8ExternalAutoLength), + DECLARE_NODE_API_PROPERTY("TestUtf8ExternalAutoLength", + TestUtf8ExternalAutoLength), DECLARE_NODE_API_PROPERTY("TestUtf8Insufficient", TestUtf8Insufficient), DECLARE_NODE_API_PROPERTY("TestUtf16", TestUtf16), DECLARE_NODE_API_PROPERTY("TestUtf16AutoLength", TestUtf16AutoLength), DECLARE_NODE_API_PROPERTY("TestUtf16External", TestUtf16External), - DECLARE_NODE_API_PROPERTY("TestUtf16ExternalAutoLength", TestUtf16ExternalAutoLength), + DECLARE_NODE_API_PROPERTY("TestUtf16ExternalAutoLength", + TestUtf16ExternalAutoLength), DECLARE_NODE_API_PROPERTY("TestUtf16Insufficient", TestUtf16Insufficient), DECLARE_NODE_API_PROPERTY("Utf16Length", Utf16Length), DECLARE_NODE_API_PROPERTY("Utf8Length", Utf8Length), From 05317ac024683aafad10e8057a2602926a62c4bd Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Wed, 7 Jun 2023 07:57:01 -0700 Subject: [PATCH 12/31] correct length --- test/js-native-api/test_string/test_string.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/js-native-api/test_string/test_string.c b/test/js-native-api/test_string/test_string.c index 5a5b9dcdc1784b..36e2abf7ab18b5 100644 --- a/test/js-native-api/test_string/test_string.c +++ b/test/js-native-api/test_string/test_string.c @@ -126,7 +126,7 @@ static napi_status create_external_one_byte(napi_env env, const size_t length_bytes = (actual_length + 1) * sizeof(*string_copy); string_copy = malloc(length_bytes); memcpy(string_copy, string, length_bytes); - string_copy[length] = 0; + string_copy[length_bytes - 1] = 0; status = create_api(env, string_copy, length, result); if (status != napi_ok) { @@ -183,7 +183,7 @@ static napi_status create_external_utf16(napi_env env, const size_t length_bytes = (actual_length + 1) * sizeof(*string_copy); string_copy = malloc(length_bytes); memcpy(string_copy, string, length_bytes); - string_copy[length] = 0; + string_copy[length_bytes - 1] = 0; status = node_api_create_external_string_utf16(env, string_copy, length, result); From 20ab0fbae61552b22c6ba7fb0e38e4a56db265b3 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Wed, 7 Jun 2023 08:22:41 -0700 Subject: [PATCH 13/31] doc --- doc/api/n-api.md | 90 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index a96b1be67c5889..faf5cbb9f25da3 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -2886,6 +2886,36 @@ string. The native string is copied. The JavaScript `string` type is described in [Section 6.1.4][] of the ECMAScript Language Specification. +#### `node_api_create_external_string_latin1` + + + +> Stability: 1 - Experimental + +```c +napi_status node_api_create_string_latin1(napi_env env, + const char* str, + size_t length, + napi_value* result); +``` + +* `[in] env`: The environment that the API is invoked under. +* `[in] str`: Character buffer representing an ISO-8859-1-encoded string. +* `[in] length`: The length of the string in bytes, or `NAPI_AUTO_LENGTH` if it + is null-terminated. +* `[out] result`: A `napi_value` representing a JavaScript `string`. + +Returns `napi_ok` if the API succeeded. + +This API creates a JavaScript `string` value from an ISO-8859-1-encoded C +string. The native string may not be copied and must thus exist for the entire +life cycle of the JavaScript value. + +The JavaScript `string` type is described in +[Section 6.1.4][] of the ECMAScript Language Specification. + #### `napi_create_string_utf16` + +> Stability: 1 - Experimental + +```c +napi_status node_api_create_string_utf16(napi_env env, + const char16_t* str, + size_t length, + napi_value* result) +``` + +* `[in] env`: The environment that the API is invoked under. +* `[in] str`: Character buffer representing a UTF16-LE-encoded string. +* `[in] length`: The length of the string in two-byte code units, or + `NAPI_AUTO_LENGTH` if it is null-terminated. +* `[out] result`: A `napi_value` representing a JavaScript `string`. + +Returns `napi_ok` if the API succeeded. + +This API creates a JavaScript `string` value from a UTF16-LE-encoded C string. +The native string may not be copied and must thus exist for the entire life +cycle of the JavaScript value. + +The JavaScript `string` type is described in +[Section 6.1.4][] of the ECMAScript Language Specification. + #### `napi_create_string_utf8` + +> Stability: 1 - Experimental + +```c +napi_status node_api_create_string_utf8(napi_env env, + const char* str, + size_t length, + napi_value* result) +``` + +* `[in] env`: The environment that the API is invoked under. +* `[in] str`: Character buffer representing a UTF8-encoded string. +* `[in] length`: The length of the string in bytes, or `NAPI_AUTO_LENGTH` if it + is null-terminated. +* `[out] result`: A `napi_value` representing a JavaScript `string`. + +Returns `napi_ok` if the API succeeded. + +This API creates a JavaScript `string` value from a UTF8-encoded C string. +The native string may not be copied and must thus exist for the entire life +cycle of the JavaScript value. + +The JavaScript `string` type is described in +[Section 6.1.4][] of the ECMAScript Language Specification. + ### Functions to convert from Node-API to C types #### `napi_get_array_length` From ca4c08c5c5bd0770224f8be13910690e16f0aad4 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Wed, 7 Jun 2023 17:53:25 -0700 Subject: [PATCH 14/31] use right length indicator --- test/js-native-api/test_string/test_string.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/js-native-api/test_string/test_string.c b/test/js-native-api/test_string/test_string.c index 36e2abf7ab18b5..be396fb4b26e4a 100644 --- a/test/js-native-api/test_string/test_string.c +++ b/test/js-native-api/test_string/test_string.c @@ -126,7 +126,7 @@ static napi_status create_external_one_byte(napi_env env, const size_t length_bytes = (actual_length + 1) * sizeof(*string_copy); string_copy = malloc(length_bytes); memcpy(string_copy, string, length_bytes); - string_copy[length_bytes - 1] = 0; + string_copy[actual_length - 1] = 0; status = create_api(env, string_copy, length, result); if (status != napi_ok) { @@ -183,7 +183,7 @@ static napi_status create_external_utf16(napi_env env, const size_t length_bytes = (actual_length + 1) * sizeof(*string_copy); string_copy = malloc(length_bytes); memcpy(string_copy, string, length_bytes); - string_copy[length_bytes - 1] = 0; + string_copy[actual_length - 1] = 0; status = node_api_create_external_string_utf16(env, string_copy, length, result); From 5de34d450685180eec537078fd445efd55c41e91 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Wed, 7 Jun 2023 18:14:37 -0700 Subject: [PATCH 15/31] final adjustment --- test/js-native-api/test_string/test_string.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/js-native-api/test_string/test_string.c b/test/js-native-api/test_string/test_string.c index be396fb4b26e4a..ecf3eafa5b6081 100644 --- a/test/js-native-api/test_string/test_string.c +++ b/test/js-native-api/test_string/test_string.c @@ -126,7 +126,7 @@ static napi_status create_external_one_byte(napi_env env, const size_t length_bytes = (actual_length + 1) * sizeof(*string_copy); string_copy = malloc(length_bytes); memcpy(string_copy, string, length_bytes); - string_copy[actual_length - 1] = 0; + string_copy[actual_length] = 0; status = create_api(env, string_copy, length, result); if (status != napi_ok) { @@ -183,7 +183,7 @@ static napi_status create_external_utf16(napi_env env, const size_t length_bytes = (actual_length + 1) * sizeof(*string_copy); string_copy = malloc(length_bytes); memcpy(string_copy, string, length_bytes); - string_copy[actual_length - 1] = 0; + string_copy[actual_length] = 0; status = node_api_create_external_string_utf16(env, string_copy, length, result); From 64f24984267826e2b8c6baf2506be27ac9e17bd5 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Wed, 7 Jun 2023 21:45:20 -0700 Subject: [PATCH 16/31] Apply suggestions from code review --- test/js-native-api/test_string/test_string.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/js-native-api/test_string/test_string.c b/test/js-native-api/test_string/test_string.c index ecf3eafa5b6081..d0eedab2bd601c 100644 --- a/test/js-native-api/test_string/test_string.c +++ b/test/js-native-api/test_string/test_string.c @@ -61,7 +61,7 @@ typedef napi_status (*TwoByteCreateAPI)(napi_env, typedef napi_status (*TwoByteGetAPI)( napi_env, napi_value, char16_t*, size_t, size_t*); -// Test passing back the one-byte string we got from JS as an external string. +// Test passing back the one-byte string we got from JS. static napi_value TestOneByteImpl(napi_env env, napi_callback_info info, OneByteGetAPI get_api, @@ -85,7 +85,7 @@ static napi_value TestOneByteImpl(napi_env env, return output; } -// Test passing back the two-byte string we got from JS as an external string. +// Test passing back the two-byte string we got from JS. static napi_value TestTwoByteImpl(napi_env env, napi_callback_info info, TwoByteGetAPI get_api, @@ -169,7 +169,8 @@ static size_t strlen16(const char16_t* string) { return iter - string; } } - return NAPI_AUTO_LENGTH; + // We should never get here. + abort(); } static napi_status create_external_utf16(napi_env env, From cb9d95a752bbfd88a28f69dbb7e64e594ff0e71f Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Wed, 7 Jun 2023 22:05:50 -0700 Subject: [PATCH 17/31] Apply suggestions from code review Co-authored-by: Daeyeon Jeong --- doc/api/n-api.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index faf5cbb9f25da3..431e07125cd25d 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -2895,7 +2895,7 @@ added: REPLACEME > Stability: 1 - Experimental ```c -napi_status node_api_create_string_latin1(napi_env env, +napi_status node_api_create_external_string_latin1(napi_env env, const char* str, size_t length, napi_value* result); @@ -2953,7 +2953,7 @@ added: REPLACEME > Stability: 1 - Experimental ```c -napi_status node_api_create_string_utf16(napi_env env, +napi_status node_api_create_external_string_utf16(napi_env env, const char16_t* str, size_t length, napi_value* result) @@ -3011,7 +3011,7 @@ added: REPLACEME > Stability: 1 - Experimental ```c -napi_status node_api_create_string_utf8(napi_env env, +napi_status node_api_create_external_string_utf8(napi_env env, const char* str, size_t length, napi_value* result) From ed83494430241b039d32938251f3c74bd1cb6df6 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Fri, 9 Jun 2023 14:20:44 -0700 Subject: [PATCH 18/31] remove utf8 --- doc/api/n-api.md | 30 ------------ src/js_native_api.h | 2 - src/js_native_api_v8.cc | 5 -- test/js-native-api/test_string/test.js | 14 ------ test/js-native-api/test_string/test_string.c | 49 ++------------------ 5 files changed, 5 insertions(+), 95 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index 431e07125cd25d..eb820cf3298587 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -3002,36 +3002,6 @@ The native string is copied. The JavaScript `string` type is described in [Section 6.1.4][] of the ECMAScript Language Specification. -#### `node_api_create_external_string_utf8` - - - -> Stability: 1 - Experimental - -```c -napi_status node_api_create_external_string_utf8(napi_env env, - const char* str, - size_t length, - napi_value* result) -``` - -* `[in] env`: The environment that the API is invoked under. -* `[in] str`: Character buffer representing a UTF8-encoded string. -* `[in] length`: The length of the string in bytes, or `NAPI_AUTO_LENGTH` if it - is null-terminated. -* `[out] result`: A `napi_value` representing a JavaScript `string`. - -Returns `napi_ok` if the API succeeded. - -This API creates a JavaScript `string` value from a UTF8-encoded C string. -The native string may not be copied and must thus exist for the entire life -cycle of the JavaScript value. - -The JavaScript `string` type is described in -[Section 6.1.4][] of the ECMAScript Language Specification. - ### Functions to convert from Node-API to C types #### `napi_get_array_length` diff --git a/src/js_native_api.h b/src/js_native_api.h index 4b10993bd93403..dc9275de7c23c1 100644 --- a/src/js_native_api.h +++ b/src/js_native_api.h @@ -95,8 +95,6 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_create_string_utf16(napi_env env, #ifdef NAPI_EXPERIMENTAL NAPI_EXTERN napi_status NAPI_CDECL node_api_create_external_string_latin1( napi_env env, const char* str, size_t length, napi_value* result); -NAPI_EXTERN napi_status NAPI_CDECL node_api_create_external_string_utf8( - napi_env env, const char* str, size_t length, napi_value* result); NAPI_EXTERN napi_status NAPI_CDECL node_api_create_external_string_utf16( napi_env env, const char16_t* str, size_t length, napi_value* result); #endif // NAPI_EXPERIMENTAL diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 8e8fe1706b3ea4..f91e36651cf90c 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -1478,11 +1478,6 @@ napi_status NAPI_CDECL node_api_create_external_string_latin1( #endif // V8_ENABLE_SANDBOX } -napi_status NAPI_CDECL node_api_create_external_string_utf8( - napi_env env, const char* str, size_t length, napi_value* result) { - return napi_create_string_utf8(env, str, length, result); -} - napi_status NAPI_CDECL node_api_create_external_string_utf16( napi_env env, const char16_t* str, size_t length, napi_value* result) { #if defined(V8_ENABLE_SANDBOX) diff --git a/test/js-native-api/test_string/test.js b/test/js-native-api/test_string/test.js index fa4d0fd24ecb52..8926b9451175bd 100644 --- a/test/js-native-api/test_string/test.js +++ b/test/js-native-api/test_string/test.js @@ -13,10 +13,8 @@ assert.strictEqual(test_string.TestLatin1AutoLength(empty), empty); assert.strictEqual(test_string.TestUtf8AutoLength(empty), empty); assert.strictEqual(test_string.TestUtf16AutoLength(empty), empty); assert.strictEqual(test_string.TestLatin1External(empty), empty); -assert.strictEqual(test_string.TestUtf8External(empty), empty); assert.strictEqual(test_string.TestUtf16External(empty), empty); assert.strictEqual(test_string.TestLatin1ExternalAutoLength(empty), empty); -assert.strictEqual(test_string.TestUtf8ExternalAutoLength(empty), empty); assert.strictEqual(test_string.TestUtf16ExternalAutoLength(empty), empty); assert.strictEqual(test_string.Utf16Length(empty), 0); assert.strictEqual(test_string.Utf8Length(empty), 0); @@ -29,10 +27,8 @@ assert.strictEqual(test_string.TestLatin1AutoLength(str1), str1); assert.strictEqual(test_string.TestUtf8AutoLength(str1), str1); assert.strictEqual(test_string.TestUtf16AutoLength(str1), str1); assert.strictEqual(test_string.TestLatin1External(str1), str1); -assert.strictEqual(test_string.TestUtf8External(str1), str1); assert.strictEqual(test_string.TestUtf16External(str1), str1); assert.strictEqual(test_string.TestLatin1ExternalAutoLength(str1), str1); -assert.strictEqual(test_string.TestUtf8ExternalAutoLength(str1), str1); assert.strictEqual(test_string.TestUtf16ExternalAutoLength(str1), str1); assert.strictEqual(test_string.TestLatin1Insufficient(str1), str1.slice(0, 3)); assert.strictEqual(test_string.TestUtf8Insufficient(str1), str1.slice(0, 3)); @@ -48,10 +44,8 @@ assert.strictEqual(test_string.TestLatin1AutoLength(str2), str2); assert.strictEqual(test_string.TestUtf8AutoLength(str2), str2); assert.strictEqual(test_string.TestUtf16AutoLength(str2), str2); assert.strictEqual(test_string.TestLatin1External(str2), str2); -assert.strictEqual(test_string.TestUtf8External(str2), str2); assert.strictEqual(test_string.TestUtf16External(str2), str2); assert.strictEqual(test_string.TestLatin1ExternalAutoLength(str2), str2); -assert.strictEqual(test_string.TestUtf8ExternalAutoLength(str2), str2); assert.strictEqual(test_string.TestUtf16ExternalAutoLength(str2), str2); assert.strictEqual(test_string.TestLatin1Insufficient(str2), str2.slice(0, 3)); assert.strictEqual(test_string.TestUtf8Insufficient(str2), str2.slice(0, 3)); @@ -67,10 +61,8 @@ assert.strictEqual(test_string.TestLatin1AutoLength(str3), str3); assert.strictEqual(test_string.TestUtf8AutoLength(str3), str3); assert.strictEqual(test_string.TestUtf16AutoLength(str3), str3); assert.strictEqual(test_string.TestLatin1External(str3), str3); -assert.strictEqual(test_string.TestUtf8External(str3), str3); assert.strictEqual(test_string.TestUtf16External(str3), str3); assert.strictEqual(test_string.TestLatin1ExternalAutoLength(str3), str3); -assert.strictEqual(test_string.TestUtf8ExternalAutoLength(str3), str3); assert.strictEqual(test_string.TestUtf16ExternalAutoLength(str3), str3); assert.strictEqual(test_string.TestLatin1Insufficient(str3), str3.slice(0, 3)); assert.strictEqual(test_string.TestUtf8Insufficient(str3), str3.slice(0, 3)); @@ -86,10 +78,8 @@ assert.strictEqual(test_string.TestLatin1AutoLength(str4), str4); assert.strictEqual(test_string.TestUtf8AutoLength(str4), str4); assert.strictEqual(test_string.TestUtf16AutoLength(str4), str4); assert.strictEqual(test_string.TestLatin1External(str4), str4); -assert.strictEqual(test_string.TestUtf8External(str4), str4); assert.strictEqual(test_string.TestUtf16External(str4), str4); assert.strictEqual(test_string.TestLatin1ExternalAutoLength(str4), str4); -assert.strictEqual(test_string.TestUtf8ExternalAutoLength(str4), str4); assert.strictEqual(test_string.TestUtf16ExternalAutoLength(str4), str4); assert.strictEqual(test_string.TestLatin1Insufficient(str4), str4.slice(0, 3)); assert.strictEqual(test_string.TestUtf8Insufficient(str4), str4.slice(0, 1)); @@ -105,10 +95,8 @@ assert.strictEqual(test_string.TestLatin1AutoLength(str5), str5); assert.strictEqual(test_string.TestUtf8AutoLength(str5), str5); assert.strictEqual(test_string.TestUtf16AutoLength(str5), str5); assert.strictEqual(test_string.TestLatin1External(str5), str5); -assert.strictEqual(test_string.TestUtf8External(str5), str5); assert.strictEqual(test_string.TestUtf16External(str5), str5); assert.strictEqual(test_string.TestLatin1ExternalAutoLength(str5), str5); -assert.strictEqual(test_string.TestUtf8ExternalAutoLength(str5), str5); assert.strictEqual(test_string.TestUtf16ExternalAutoLength(str5), str5); assert.strictEqual(test_string.TestLatin1Insufficient(str5), str5.slice(0, 3)); assert.strictEqual(test_string.TestUtf8Insufficient(str5), str5.slice(0, 1)); @@ -121,9 +109,7 @@ assert.strictEqual(test_string.TestUtf8(str6), str6); assert.strictEqual(test_string.TestUtf16(str6), str6); assert.strictEqual(test_string.TestUtf8AutoLength(str6), str6); assert.strictEqual(test_string.TestUtf16AutoLength(str6), str6); -assert.strictEqual(test_string.TestUtf8External(str6), str6); assert.strictEqual(test_string.TestUtf16External(str6), str6); -assert.strictEqual(test_string.TestUtf8ExternalAutoLength(str6), str6); assert.strictEqual(test_string.TestUtf16ExternalAutoLength(str6), str6); assert.strictEqual(test_string.TestUtf8Insufficient(str6), str6.slice(0, 1)); assert.strictEqual(test_string.TestUtf16Insufficient(str6), str6.slice(0, 3)); diff --git a/test/js-native-api/test_string/test_string.c b/test/js-native-api/test_string/test_string.c index d0eedab2bd601c..86b7d5c6fe7155 100644 --- a/test/js-native-api/test_string/test_string.c +++ b/test/js-native-api/test_string/test_string.c @@ -109,16 +109,10 @@ static napi_value TestTwoByteImpl(napi_env env, return output; } -// Common code for copying a stack-allocated string to a heap-allocated one, and -// adding the latter to a free list which will be freed at the end of the test. -// That way, we can treat it as an external string. There's no point factoring -// out such common code for two-byte strings because there's only one API to -// test. -static napi_status create_external_one_byte(napi_env env, - const char* string, - size_t length, - napi_value* result, - OneByteCreateAPI create_api) { +static napi_status create_external_latin1(napi_env env, + const char* string, + size_t length, + napi_value* result) { napi_status status; char* string_copy; const size_t actual_length = @@ -128,7 +122,7 @@ static napi_status create_external_one_byte(napi_env env, memcpy(string_copy, string, length_bytes); string_copy[actual_length] = 0; - status = create_api(env, string_copy, length, result); + status = node_api_create_external_string_latin1(env, string_copy, length, result); if (status != napi_ok) { free(string_copy); return status; @@ -145,22 +139,6 @@ static napi_status create_external_one_byte(napi_env env, return napi_ok; } -static napi_status create_external_latin1(napi_env env, - const char* string, - size_t length, - napi_value* result) { - return create_external_one_byte( - env, string, length, result, node_api_create_external_string_latin1); -} - -static napi_status create_external_utf8(napi_env env, - const char* string, - size_t length, - napi_value* result) { - return create_external_one_byte( - env, string, length, result, node_api_create_external_string_utf8); -} - // strlen for char16_t. Needed in case we're copying a string of length // NAPI_AUTO_LENGTH. static size_t strlen16(const char16_t* string) { @@ -260,14 +238,6 @@ static napi_value TestLatin1External(napi_env env, napi_callback_info info) { actual_length); } -static napi_value TestUtf8External(napi_env env, napi_callback_info info) { - return TestOneByteImpl(env, - info, - napi_get_value_string_utf8, - create_external_utf8, - actual_length); -} - static napi_value TestUtf16External(napi_env env, napi_callback_info info) { return TestTwoByteImpl(env, info, @@ -285,12 +255,6 @@ static napi_value TestLatin1ExternalAutoLength(napi_env env, auto_length); } -static napi_value TestUtf8ExternalAutoLength(napi_env env, - napi_callback_info info) { - return TestOneByteImpl( - env, info, napi_get_value_string_utf8, create_external_utf8, auto_length); -} - static napi_value TestUtf16ExternalAutoLength(napi_env env, napi_callback_info info) { return TestTwoByteImpl(env, @@ -463,9 +427,6 @@ napi_value Init(napi_env env, napi_value exports) { TestLatin1Insufficient), DECLARE_NODE_API_PROPERTY("TestUtf8", TestUtf8), DECLARE_NODE_API_PROPERTY("TestUtf8AutoLength", TestUtf8AutoLength), - DECLARE_NODE_API_PROPERTY("TestUtf8External", TestUtf8External), - DECLARE_NODE_API_PROPERTY("TestUtf8ExternalAutoLength", - TestUtf8ExternalAutoLength), DECLARE_NODE_API_PROPERTY("TestUtf8Insufficient", TestUtf8Insufficient), DECLARE_NODE_API_PROPERTY("TestUtf16", TestUtf16), DECLARE_NODE_API_PROPERTY("TestUtf16AutoLength", TestUtf16AutoLength), From e06390131db4d77a605da82bcc9545698d9122c5 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sat, 10 Jun 2023 16:05:21 -0700 Subject: [PATCH 19/31] handle the case where we copy the string --- src/js_native_api.h | 20 ++++++++++--- src/js_native_api_v8.cc | 63 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 73 insertions(+), 10 deletions(-) diff --git a/src/js_native_api.h b/src/js_native_api.h index dc9275de7c23c1..3aa0ee5c1c5c5c 100644 --- a/src/js_native_api.h +++ b/src/js_native_api.h @@ -93,10 +93,22 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_create_string_utf16(napi_env env, size_t length, napi_value* result); #ifdef NAPI_EXPERIMENTAL -NAPI_EXTERN napi_status NAPI_CDECL node_api_create_external_string_latin1( - napi_env env, const char* str, size_t length, napi_value* result); -NAPI_EXTERN napi_status NAPI_CDECL node_api_create_external_string_utf16( - napi_env env, const char16_t* str, size_t length, napi_value* result); +NAPI_EXTERN napi_status NAPI_CDECL +node_api_create_external_string_latin1(napi_env env, + char* str, + size_t length, + napi_finalize finalize_callback, + void* finalize_hint, + napi_value* result, + bool* copied); +NAPI_EXTERN napi_status NAPI_CDECL +node_api_create_external_string_utf16(napi_env env, + char16_t* str, + size_t length, + napi_finalize finalize_callback, + void* finalize_hint, + napi_value* result, + bool* copied); #endif // NAPI_EXPERIMENTAL NAPI_EXTERN napi_status NAPI_CDECL napi_create_symbol(napi_env env, napi_value description, diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index f91e36651cf90c..e0397bbf13bb13 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -61,6 +61,28 @@ namespace v8impl { namespace { +template +napi_status CopyExternalString(napi_env env, + CharType* str, + size_t length, + napi_finalize finalize_callback, + void* finalize_hint, + napi_value* result, + bool* copied, + CreateAPI create_api) { + napi_status status = create_api(env, str, length, result); + if (status == napi_ok) { + if (copied) { + *copied = true; + } + if (finalize_callback) { + env->CallFinalizer( + finalize_callback, static_cast(str), finalize_hint); + } + } + return status; +} + class ExternalOneByteStringResource : public v8::String::ExternalOneByteStringResource { public: @@ -1463,11 +1485,27 @@ napi_status NAPI_CDECL napi_create_string_utf16(napi_env env, }); } -napi_status NAPI_CDECL node_api_create_external_string_latin1( - napi_env env, const char* str, size_t length, napi_value* result) { +napi_status NAPI_CDECL +node_api_create_external_string_latin1(napi_env env, + char* str, + size_t length, + napi_finalize finalize_callback, + void* finalize_hint, + napi_value* result, + bool* copied) { #if defined(V8_ENABLE_SANDBOX) - return napi_create_string_latin1(env, str, length, result); + return v8impl::CopyExternalString(env, + str, + length, + finalize_callback, + finalize_hint, + result, + copied, + napi_create_string_latin1); #else + if (copied != nullptr) { + *copied = false; + } return v8impl::NewString(env, str, length, result, [&](v8::Isolate* isolate) { if (length == NAPI_AUTO_LENGTH) { length = (std::string_view(str)).length(); @@ -1478,10 +1516,23 @@ napi_status NAPI_CDECL node_api_create_external_string_latin1( #endif // V8_ENABLE_SANDBOX } -napi_status NAPI_CDECL node_api_create_external_string_utf16( - napi_env env, const char16_t* str, size_t length, napi_value* result) { +napi_status NAPI_CDECL +node_api_create_external_string_utf16(napi_env env, + char16_t* str, + size_t length, + napi_finalize finalize_callback, + void* finalize_hint, + napi_value* result, + bool* copied) { #if defined(V8_ENABLE_SANDBOX) - return napi_create_string_utf16(env, str, length, result); + return v8impl::CopyExternalString(env, + str, + length, + finalize_callback, + finalize_hint, + result, + copied, + napi_create_string_utf16); #else return v8impl::NewString(env, str, length, result, [&](v8::Isolate* isolate) { if (length == NAPI_AUTO_LENGTH) { From 4ff5e53d53a5688d2ddc6c605f82b29ddb35d343 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sat, 10 Jun 2023 16:48:32 -0700 Subject: [PATCH 20/31] implement the non-copying case --- src/js_native_api_v8.cc | 39 +++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index e0397bbf13bb13..f056d3d4444b06 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -84,26 +84,48 @@ napi_status CopyExternalString(napi_env env, } class ExternalOneByteStringResource - : public v8::String::ExternalOneByteStringResource { + : public v8::String::ExternalOneByteStringResource, + Finalizer { public: - ExternalOneByteStringResource(const char* string, const size_t length) - : string_(string), length_(length) {} + ExternalOneByteStringResource(napi_env env, + char* string, + const size_t length, + napi_finalize finalize_callback, + void* finalize_hint) + : Finalizer(env, finalize_callback, string, finalize_hint), + string_(string), + length_(length) {} const char* data() const { return string_; } size_t length() const { return length_; } private: + void Dispose() { + if (finalize_callback_ == nullptr) return; + env_->CallFinalizer(finalize_callback_, finalize_data_, finalize_hint_); + } const char* string_; const size_t length_; }; -class ExternalStringResource : public v8::String::ExternalStringResource { +class ExternalStringResource : public v8::String::ExternalStringResource, + Finalizer { public: - ExternalStringResource(const uint16_t* string, const size_t length) - : string_(string), length_(length) {} + ExternalStringResource(napi_env env, + char16_t* string, + const size_t length, + napi_finalize finalize_callback, + void* finalize_hint) + : Finalizer(env, finalize_callback, string, finalize_hint), + string_(reinterpret_cast(string)), + length_(length) {} const uint16_t* data() const { return string_; } size_t length() const { return length_; } private: + void Dispose() { + if (finalize_callback_ == nullptr) return; + env_->CallFinalizer(finalize_callback_, finalize_data_, finalize_hint_); + } const uint16_t* string_; const size_t length_; }; @@ -1510,7 +1532,8 @@ node_api_create_external_string_latin1(napi_env env, if (length == NAPI_AUTO_LENGTH) { length = (std::string_view(str)).length(); } - auto resource = new v8impl::ExternalOneByteStringResource(str, length); + auto resource = new v8impl::ExternalOneByteStringResource( + env, str, length, finalize_callback, finalize_hint); return v8::String::NewExternalOneByte(isolate, resource); }); #endif // V8_ENABLE_SANDBOX @@ -1539,7 +1562,7 @@ node_api_create_external_string_utf16(napi_env env, length = (std::u16string_view(str)).length(); } auto resource = new v8impl::ExternalStringResource( - reinterpret_cast(str), length); + env, str, length, finalize_callback, finalize_hint); return v8::String::NewExternalTwoByte(isolate, resource); }); #endif // V8_ENABLE_SANDBOX From 402c93e5bfaad71c0ac0e0c0915fb641f2b870f6 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sat, 10 Jun 2023 17:05:10 -0700 Subject: [PATCH 21/31] Unify the two external cases --- src/js_native_api_v8.cc | 144 ++++++++++++++++++++-------------------- 1 file changed, 73 insertions(+), 71 deletions(-) diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index f056d3d4444b06..899170eba7e4c7 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -61,18 +61,40 @@ namespace v8impl { namespace { -template -napi_status CopyExternalString(napi_env env, - CharType* str, - size_t length, - napi_finalize finalize_callback, - void* finalize_hint, - napi_value* result, - bool* copied, - CreateAPI create_api) { - napi_status status = create_api(env, str, length, result); +template +napi_status NewString(napi_env env, + const CCharType* str, + size_t length, + napi_value* result, + StringMaker string_maker) { + CHECK_ENV(env); + if (length > 0) CHECK_ARG(env, str); + CHECK_ARG(env, result); + RETURN_STATUS_IF_FALSE( + env, (length == NAPI_AUTO_LENGTH) || length <= INT_MAX, napi_invalid_arg); + + auto isolate = env->isolate; + auto str_maybe = string_maker(isolate); + CHECK_MAYBE_EMPTY(env, str_maybe, napi_generic_failure); + *result = v8impl::JsValueFromV8LocalValue(str_maybe.ToLocalChecked()); + return napi_clear_last_error(env); +} + +template +napi_status NewExternalString(napi_env env, + CharType* str, + size_t length, + napi_finalize finalize_callback, + void* finalize_hint, + napi_value* result, + bool* copied, + CreateAPI create_api, + StringMaker string_maker) { + napi_status status; +#if defined(V8_ENABLE_SANDBOX) + status = create_api(env, str, length, result); if (status == napi_ok) { - if (copied) { + if (copied != nullptr) { *copied = true; } if (finalize_callback) { @@ -80,6 +102,12 @@ napi_status CopyExternalString(napi_env env, finalize_callback, static_cast(str), finalize_hint); } } +#else + status = NewString(env, str, length, result, string_maker); + if (status == napi_ok && copied != nullptr) { + *copied = false; + } +#endif // V8_ENABLE_SANDBOX return status; } @@ -130,25 +158,6 @@ class ExternalStringResource : public v8::String::ExternalStringResource, const size_t length_; }; -template -napi_status NewString(napi_env env, - const CCharType* str, - size_t length, - napi_value* result, - StringMaker string_maker) { - CHECK_ENV(env); - if (length > 0) CHECK_ARG(env, str); - CHECK_ARG(env, result); - RETURN_STATUS_IF_FALSE( - env, (length == NAPI_AUTO_LENGTH) || length <= INT_MAX, napi_invalid_arg); - - auto isolate = env->isolate; - auto str_maybe = string_maker(isolate); - CHECK_MAYBE_EMPTY(env, str_maybe, napi_generic_failure); - *result = v8impl::JsValueFromV8LocalValue(str_maybe.ToLocalChecked()); - return napi_clear_last_error(env); -} - inline napi_status V8NameFromPropertyDescriptor( napi_env env, const napi_property_descriptor* p, @@ -1515,28 +1524,23 @@ node_api_create_external_string_latin1(napi_env env, void* finalize_hint, napi_value* result, bool* copied) { -#if defined(V8_ENABLE_SANDBOX) - return v8impl::CopyExternalString(env, - str, - length, - finalize_callback, - finalize_hint, - result, - copied, - napi_create_string_latin1); -#else - if (copied != nullptr) { - *copied = false; - } - return v8impl::NewString(env, str, length, result, [&](v8::Isolate* isolate) { - if (length == NAPI_AUTO_LENGTH) { - length = (std::string_view(str)).length(); - } - auto resource = new v8impl::ExternalOneByteStringResource( - env, str, length, finalize_callback, finalize_hint); - return v8::String::NewExternalOneByte(isolate, resource); - }); -#endif // V8_ENABLE_SANDBOX + return v8impl::NewExternalString( + env, + str, + length, + finalize_callback, + finalize_hint, + result, + copied, + napi_create_string_latin1, + [&](v8::Isolate* isolate) { + if (length == NAPI_AUTO_LENGTH) { + length = (std::string_view(str)).length(); + } + auto resource = new v8impl::ExternalOneByteStringResource( + env, str, length, finalize_callback, finalize_hint); + return v8::String::NewExternalOneByte(isolate, resource); + }); } napi_status NAPI_CDECL @@ -1547,25 +1551,23 @@ node_api_create_external_string_utf16(napi_env env, void* finalize_hint, napi_value* result, bool* copied) { -#if defined(V8_ENABLE_SANDBOX) - return v8impl::CopyExternalString(env, - str, - length, - finalize_callback, - finalize_hint, - result, - copied, - napi_create_string_utf16); -#else - return v8impl::NewString(env, str, length, result, [&](v8::Isolate* isolate) { - if (length == NAPI_AUTO_LENGTH) { - length = (std::u16string_view(str)).length(); - } - auto resource = new v8impl::ExternalStringResource( - env, str, length, finalize_callback, finalize_hint); - return v8::String::NewExternalTwoByte(isolate, resource); - }); -#endif // V8_ENABLE_SANDBOX + return v8impl::NewExternalString( + env, + str, + length, + finalize_callback, + finalize_hint, + result, + copied, + napi_create_string_utf16, + [&](v8::Isolate* isolate) { + if (length == NAPI_AUTO_LENGTH) { + length = (std::u16string_view(str)).length(); + } + auto resource = new v8impl::ExternalStringResource( + env, str, length, finalize_callback, finalize_hint); + return v8::String::NewExternalTwoByte(isolate, resource); + }); } napi_status NAPI_CDECL napi_create_double(napi_env env, From 32a5600513e48a87ffa2235d874cbe4cb9f854c6 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sat, 10 Jun 2023 22:18:00 -0700 Subject: [PATCH 22/31] factor out tracking --- src/js_native_api_v8.cc | 64 +++++++++++++++----- test/js-native-api/test_string/test_string.c | 61 +++++-------------- 2 files changed, 63 insertions(+), 62 deletions(-) diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 899170eba7e4c7..d11150e14cfee4 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -111,49 +111,81 @@ napi_status NewExternalString(napi_env env, return status; } +class TrackedStringResource : public Finalizer, RefTracker { + public: + TrackedStringResource(napi_env env, + napi_finalize finalize_callback, + void* data, + void* finalize_hint) + : Finalizer(env, finalize_callback, data, finalize_hint) { + Link(finalize_callback == nullptr ? &env->reflist + : &env->finalizing_reflist); + } + + protected: + // The only time Finalize() gets called before Dispose() is if the + // environment is dying. Finalize() expects that the item will be unlinked, + // so we do it here. V8 will still call Dispose() on us later, so we don't do + // any deleting here. We just null out env_ to avoid passing a stale pointer + // to the user's finalizer when V8 does finally call Dispose(). + void Finalize() override { + Unlink(); + env_ = nullptr; + } + + void DoDispose() { + if (finalize_callback_ == nullptr) return; + if (env_ == nullptr) { + // The environment is dead. Call the finalizer directly. + finalize_callback_(nullptr, finalize_data_, finalize_hint_); + } else { + // The environment is still alive. Let's remove ourselves from its list + // of references and call the user's finalizer. V8 will delete us upon + // returning from this method. + Unlink(); + env_->CallFinalizer(finalize_callback_, finalize_data_, finalize_hint_); + } + } +}; + class ExternalOneByteStringResource : public v8::String::ExternalOneByteStringResource, - Finalizer { + TrackedStringResource { public: ExternalOneByteStringResource(napi_env env, char* string, const size_t length, napi_finalize finalize_callback, void* finalize_hint) - : Finalizer(env, finalize_callback, string, finalize_hint), + : TrackedStringResource(env, finalize_callback, string, finalize_hint), string_(string), length_(length) {} - const char* data() const { return string_; } - size_t length() const { return length_; } + + const char* data() const override { return string_; } + size_t length() const override { return length_; } private: - void Dispose() { - if (finalize_callback_ == nullptr) return; - env_->CallFinalizer(finalize_callback_, finalize_data_, finalize_hint_); - } + void Dispose() override { DoDispose(); } const char* string_; const size_t length_; }; class ExternalStringResource : public v8::String::ExternalStringResource, - Finalizer { + TrackedStringResource { public: ExternalStringResource(napi_env env, char16_t* string, const size_t length, napi_finalize finalize_callback, void* finalize_hint) - : Finalizer(env, finalize_callback, string, finalize_hint), + : TrackedStringResource(env, finalize_callback, string, finalize_hint), string_(reinterpret_cast(string)), length_(length) {} - const uint16_t* data() const { return string_; } - size_t length() const { return length_; } + const uint16_t* data() const override { return string_; } + size_t length() const override { return length_; } private: - void Dispose() { - if (finalize_callback_ == nullptr) return; - env_->CallFinalizer(finalize_callback_, finalize_data_, finalize_hint_); - } + void Dispose() override { DoDispose(); } const uint16_t* string_; const size_t length_; }; diff --git a/test/js-native-api/test_string/test_string.c b/test/js-native-api/test_string/test_string.c index 86b7d5c6fe7155..e4b79fe50f0774 100644 --- a/test/js-native-api/test_string/test_string.c +++ b/test/js-native-api/test_string/test_string.c @@ -25,28 +25,6 @@ static napi_status validate_and_retrieve_single_string_arg( return napi_ok; } -// Define a free list for collecting "external" strings, which are really -// strings we copy from strings that are coming in from JS. -struct free_list { - void* data; - struct free_list* next; -}; -static void free_items(napi_env env, void* data, void* hint) { - struct free_list* item = data; - while (item != NULL) { - struct free_list* to_free = item; - item = to_free->next; - free(to_free->data); - free(to_free); - } -} -static void insert_item(struct free_list* first_item, void* data) { - struct free_list* new_item = malloc(sizeof(*new_item)); - new_item->next = first_item->next; - first_item->next = new_item; - new_item->data = data; -} - // These help us factor out code that is common between the bindings. typedef napi_status (*OneByteCreateAPI)(napi_env, const char*, @@ -109,11 +87,17 @@ static napi_value TestTwoByteImpl(napi_env env, return output; } +static void free_string(napi_env env, void* data, void* hint) { + free(data); +} + static napi_status create_external_latin1(napi_env env, const char* string, size_t length, napi_value* result) { napi_status status; + // Initialize to true, because that is the value we don't want. + bool copied = true; char* string_copy; const size_t actual_length = (length == NAPI_AUTO_LENGTH ? strlen(string) : length); @@ -122,20 +106,16 @@ static napi_status create_external_latin1(napi_env env, memcpy(string_copy, string, length_bytes); string_copy[actual_length] = 0; - status = node_api_create_external_string_latin1(env, string_copy, length, result); - if (status != napi_ok) { - free(string_copy); - return status; + status = node_api_create_external_string_latin1( + env, string_copy, length, free_string, NULL, result, &copied); + // We do not want the string to be copied. + if (copied) { + return napi_generic_failure; } - - struct free_list* first_item; - status = napi_get_instance_data(env, (void**)&first_item); if (status != napi_ok) { free(string_copy); return status; } - - insert_item(first_item, string_copy); return napi_ok; } @@ -156,6 +136,8 @@ static napi_status create_external_utf16(napi_env env, size_t length, napi_value* result) { napi_status status; + // Initialize to true, because that is the value we don't want. + bool copied = true; char16_t* string_copy; const size_t actual_length = (length == NAPI_AUTO_LENGTH ? strlen16(string) : length); @@ -164,21 +146,13 @@ static napi_status create_external_utf16(napi_env env, memcpy(string_copy, string, length_bytes); string_copy[actual_length] = 0; - status = - node_api_create_external_string_utf16(env, string_copy, length, result); - if (status != napi_ok) { - free(string_copy); - return status; - } - - struct free_list* first_item; - status = napi_get_instance_data(env, (void**)&first_item); + status = node_api_create_external_string_utf16( + env, string_copy, length, free_string, NULL, result, &copied); if (status != napi_ok) { free(string_copy); return status; } - insert_item(first_item, string_copy); return napi_ok; } @@ -412,11 +386,6 @@ static napi_value TestMemoryCorruption(napi_env env, napi_callback_info info) { EXTERN_C_START napi_value Init(napi_env env, napi_value exports) { - struct free_list* first_item = malloc(sizeof(*first_item)); - first_item->next = NULL; - first_item->data = NULL; - NODE_API_CALL(env, napi_set_instance_data(env, first_item, free_items, NULL)); - napi_property_descriptor properties[] = { DECLARE_NODE_API_PROPERTY("TestLatin1", TestLatin1), DECLARE_NODE_API_PROPERTY("TestLatin1AutoLength", TestLatin1AutoLength), From 0b72009975836d4d4b7153241f5e6dd9dd435b73 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sat, 10 Jun 2023 22:53:53 -0700 Subject: [PATCH 23/31] document --- doc/api/n-api.md | 61 +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 52 insertions(+), 9 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index eb820cf3298587..a0a8c5732916b4 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -801,7 +801,7 @@ napiVersion: 1 Function pointer type for add-on provided functions that allow the user to be notified when externally-owned data is ready to be cleaned up because the -object with which it was associated with, has been garbage-collected. The user +object with which it was associated with has been garbage-collected. The user must provide a function satisfying the following signature which would get called upon the object's collection. Currently, `napi_finalize` can be used for finding out when objects that have external data are collected. @@ -819,6 +819,11 @@ Since these functions may be called while the JavaScript engine is in a state where it cannot execute JavaScript code, some Node-API calls may return `napi_pending_exception` even when there is no exception pending. +In the case of [`node_api_create_external_string_latin1`][] and +[`node_api_create_external_string_utf16`][] the `env` parameter may be null, +because external strings can be collected during the latter part of environment +shutdown. + Change History: * experimental (`NAPI_EXPERIMENTAL` is defined): @@ -2895,17 +2900,35 @@ added: REPLACEME > Stability: 1 - Experimental ```c -napi_status node_api_create_external_string_latin1(napi_env env, - const char* str, - size_t length, - napi_value* result); +napi_status +node_api_create_external_string_latin1(napi_env env, + char* str, + size_t length, + napi_finalize finalize_callback, + void* finalize_hint, + napi_value* result, + bool* copied); ``` * `[in] env`: The environment that the API is invoked under. * `[in] str`: Character buffer representing an ISO-8859-1-encoded string. * `[in] length`: The length of the string in bytes, or `NAPI_AUTO_LENGTH` if it is null-terminated. +* `[in] finalize_callback`: The function to call when the string is being + collected. The function will be called with the following parameters: + * `[in] env`: The environment in which the add-on is running. This value + may be null if the string is being collected as part of the termination + of the worker or the main Node.js instance. + * `[in] data`: This is the value `str` as a `void*` pointer. + * `[in] finalize_hint`: This is the value `finalize_hint` that was given + to the API. + [`napi_finalize`][] provides more details. + This parameter is optional. Passing a null value means that the add-on + doesn't need to be notified when the corresponding JavaScript string is + collected. * `[out] result`: A `napi_value` representing a JavaScript `string`. +* `[out] copied`: Whether the string was copied. If it was, the finalizer will + already have been invoked to destroy `str`. Returns `napi_ok` if the API succeeded. @@ -2953,17 +2976,35 @@ added: REPLACEME > Stability: 1 - Experimental ```c -napi_status node_api_create_external_string_utf16(napi_env env, - const char16_t* str, - size_t length, - napi_value* result) +napi_status +node_api_create_external_string_utf16(napi_env env, + char16_t* str, + size_t length, + napi_finalize finalize_callback, + void* finalize_hint, + napi_value* result, + bool* copied); ``` * `[in] env`: The environment that the API is invoked under. * `[in] str`: Character buffer representing a UTF16-LE-encoded string. * `[in] length`: The length of the string in two-byte code units, or `NAPI_AUTO_LENGTH` if it is null-terminated. +* `[in] finalize_callback`: The function to call when the string is being + collected. The function will be called with the following parameters: + * `[in] env`: The environment in which the add-on is running. This value + may be null if the string is being collected as part of the termination + of the worker or the main Node.js instance. + * `[in] data`: This is the value `str` as a `void*` pointer. + * `[in] finalize_hint`: This is the value `finalize_hint` that was given + to the API. + [`napi_finalize`][] provides more details. + This parameter is optional. Passing a null value means that the add-on + doesn't need to be notified when the corresponding JavaScript string is + collected. * `[out] result`: A `napi_value` representing a JavaScript `string`. +* `[out] copied`: Whether the string was copied. If it was, the finalizer will + already have been invoked to destroy `str`. Returns `napi_ok` if the API succeeded. @@ -6531,6 +6572,8 @@ the add-on's file name during loading. [`napi_wrap`]: #napi_wrap [`node-addon-api`]: https://github.com/nodejs/node-addon-api [`node_api.h`]: https://github.com/nodejs/node/blob/HEAD/src/node_api.h +[`node_api_create_external_string_latin1`]: #node_api_create_external_string_latin1 +[`node_api_create_external_string_utf16`]: #node_api_create_external_string_utf16 [`node_api_create_syntax_error`]: #node_api_create_syntax_error [`node_api_throw_syntax_error`]: #node_api_throw_syntax_error [`process.release`]: process.md#processrelease From 50dfa5dfc45e15d383df1ae6bd7ad54afdd0077a Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sun, 11 Jun 2023 02:49:54 -0700 Subject: [PATCH 24/31] debug ctor dtor --- src/js_native_api_v8.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index d11150e14cfee4..498358dd62b33f 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -159,13 +159,13 @@ class ExternalOneByteStringResource void* finalize_hint) : TrackedStringResource(env, finalize_callback, string, finalize_hint), string_(string), - length_(length) {} + length_(length) {fprintf(stderr, "%p: one byte ctor\n", this);} const char* data() const override { return string_; } size_t length() const override { return length_; } private: - void Dispose() override { DoDispose(); } + void Dispose() override { fprintf(stderr, "%p: two byte dtor\n", this); DoDispose(); } const char* string_; const size_t length_; }; @@ -180,12 +180,12 @@ class ExternalStringResource : public v8::String::ExternalStringResource, void* finalize_hint) : TrackedStringResource(env, finalize_callback, string, finalize_hint), string_(reinterpret_cast(string)), - length_(length) {} + length_(length) {fprintf(stderr, "%p: two byte ctor\n", this);} const uint16_t* data() const override { return string_; } size_t length() const override { return length_; } private: - void Dispose() override { DoDispose(); } + void Dispose() override { fprintf(stderr, "%p: two byte dtor\n", this); DoDispose(); } const uint16_t* string_; const size_t length_; }; From 3a96b046004180fbd1110b48a2636b7c6c47758b Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sun, 11 Jun 2023 08:06:25 -0700 Subject: [PATCH 25/31] typo --- src/js_native_api_v8.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 498358dd62b33f..7867bfec5a6ffe 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -165,7 +165,7 @@ class ExternalOneByteStringResource size_t length() const override { return length_; } private: - void Dispose() override { fprintf(stderr, "%p: two byte dtor\n", this); DoDispose(); } + void Dispose() override { fprintf(stderr, "%p: one byte dtor\n", this); DoDispose(); } const char* string_; const size_t length_; }; From caccb8ecf85c4a78459e7d2dfa623e3700a6878b Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sun, 11 Jun 2023 15:12:35 -0700 Subject: [PATCH 26/31] Apply suggestions from code review --- src/js_native_api_v8.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 7867bfec5a6ffe..ea57258f331f37 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -165,7 +165,7 @@ class ExternalOneByteStringResource size_t length() const override { return length_; } private: - void Dispose() override { fprintf(stderr, "%p: one byte dtor\n", this); DoDispose(); } + void ~ExternalOneByteStringResource() { fprintf(stderr, "%p: one byte dtor\n", this); DoDispose(); } const char* string_; const size_t length_; }; @@ -185,7 +185,7 @@ class ExternalStringResource : public v8::String::ExternalStringResource, size_t length() const override { return length_; } private: - void Dispose() override { fprintf(stderr, "%p: two byte dtor\n", this); DoDispose(); } + void ~ExternalStringResource() { fprintf(stderr, "%p: two byte dtor\n", this); DoDispose(); } const uint16_t* string_; const size_t length_; }; From 5c11c819861b30014586790cead8cca8bd3c1d6c Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sun, 11 Jun 2023 15:35:15 -0700 Subject: [PATCH 27/31] Apply suggestions from code review --- src/js_native_api_v8.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index ea57258f331f37..ebb04aab8eedeb 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -165,7 +165,7 @@ class ExternalOneByteStringResource size_t length() const override { return length_; } private: - void ~ExternalOneByteStringResource() { fprintf(stderr, "%p: one byte dtor\n", this); DoDispose(); } + ~ExternalOneByteStringResource() { fprintf(stderr, "%p: one byte dtor\n", this); DoDispose(); } const char* string_; const size_t length_; }; @@ -185,7 +185,7 @@ class ExternalStringResource : public v8::String::ExternalStringResource, size_t length() const override { return length_; } private: - void ~ExternalStringResource() { fprintf(stderr, "%p: two byte dtor\n", this); DoDispose(); } + ~ExternalStringResource() { fprintf(stderr, "%p: two byte dtor\n", this); DoDispose(); } const uint16_t* string_; const size_t length_; }; From b651fca80444e4d6e1111d40b6212ae80178b5b8 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sun, 11 Jun 2023 18:40:50 -0700 Subject: [PATCH 28/31] use common distructor --- src/js_native_api_v8.cc | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index ebb04aab8eedeb..9dd71f9aba1d8b 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -133,15 +133,14 @@ class TrackedStringResource : public Finalizer, RefTracker { env_ = nullptr; } - void DoDispose() { + ~TrackedStringResource() { if (finalize_callback_ == nullptr) return; if (env_ == nullptr) { // The environment is dead. Call the finalizer directly. finalize_callback_(nullptr, finalize_data_, finalize_hint_); } else { // The environment is still alive. Let's remove ourselves from its list - // of references and call the user's finalizer. V8 will delete us upon - // returning from this method. + // of references and call the user's finalizer. Unlink(); env_->CallFinalizer(finalize_callback_, finalize_data_, finalize_hint_); } @@ -165,7 +164,6 @@ class ExternalOneByteStringResource size_t length() const override { return length_; } private: - ~ExternalOneByteStringResource() { fprintf(stderr, "%p: one byte dtor\n", this); DoDispose(); } const char* string_; const size_t length_; }; @@ -185,7 +183,6 @@ class ExternalStringResource : public v8::String::ExternalStringResource, size_t length() const override { return length_; } private: - ~ExternalStringResource() { fprintf(stderr, "%p: two byte dtor\n", this); DoDispose(); } const uint16_t* string_; const size_t length_; }; From 668fd4d6d619ba8daaf3bfe18e88eef4ccf532a3 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sun, 11 Jun 2023 19:01:45 -0700 Subject: [PATCH 29/31] format --- src/js_native_api_v8.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 9dd71f9aba1d8b..f9ccd0616b2de5 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -158,7 +158,7 @@ class ExternalOneByteStringResource void* finalize_hint) : TrackedStringResource(env, finalize_callback, string, finalize_hint), string_(string), - length_(length) {fprintf(stderr, "%p: one byte ctor\n", this);} + length_(length) {} const char* data() const override { return string_; } size_t length() const override { return length_; } @@ -178,7 +178,8 @@ class ExternalStringResource : public v8::String::ExternalStringResource, void* finalize_hint) : TrackedStringResource(env, finalize_callback, string, finalize_hint), string_(reinterpret_cast(string)), - length_(length) {fprintf(stderr, "%p: two byte ctor\n", this);} + length_(length) {} + const uint16_t* data() const override { return string_; } size_t length() const override { return length_; } From c721439976bffa2ffa27859aa576f308f70718eb Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sun, 11 Jun 2023 20:36:58 -0700 Subject: [PATCH 30/31] Apply suggestions from code review Co-authored-by: Daeyeon Jeong --- doc/api/n-api.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index a0a8c5732916b4..2b6a7caec9066f 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -2926,6 +2926,7 @@ node_api_create_external_string_latin1(napi_env env, This parameter is optional. Passing a null value means that the add-on doesn't need to be notified when the corresponding JavaScript string is collected. +* `[in] finalize_hint`: Optional hint to pass to the finalize callback during collection. * `[out] result`: A `napi_value` representing a JavaScript `string`. * `[out] copied`: Whether the string was copied. If it was, the finalizer will already have been invoked to destroy `str`. @@ -3002,6 +3003,7 @@ node_api_create_external_string_utf16(napi_env env, This parameter is optional. Passing a null value means that the add-on doesn't need to be notified when the corresponding JavaScript string is collected. +* `[in] finalize_hint`: Optional hint to pass to the finalize callback during collection. * `[out] result`: A `napi_value` representing a JavaScript `string`. * `[out] copied`: Whether the string was copied. If it was, the finalizer will already have been invoked to destroy `str`. From 7f67a2f05775cd79201e290c149857743a714196 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sun, 11 Jun 2023 20:50:01 -0700 Subject: [PATCH 31/31] md lint --- doc/api/n-api.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index 2b6a7caec9066f..7b12b5c763d079 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -2926,7 +2926,8 @@ node_api_create_external_string_latin1(napi_env env, This parameter is optional. Passing a null value means that the add-on doesn't need to be notified when the corresponding JavaScript string is collected. -* `[in] finalize_hint`: Optional hint to pass to the finalize callback during collection. +* `[in] finalize_hint`: Optional hint to pass to the finalize callback during + collection. * `[out] result`: A `napi_value` representing a JavaScript `string`. * `[out] copied`: Whether the string was copied. If it was, the finalizer will already have been invoked to destroy `str`. @@ -3003,7 +3004,8 @@ node_api_create_external_string_utf16(napi_env env, This parameter is optional. Passing a null value means that the add-on doesn't need to be notified when the corresponding JavaScript string is collected. -* `[in] finalize_hint`: Optional hint to pass to the finalize callback during collection. +* `[in] finalize_hint`: Optional hint to pass to the finalize callback during + collection. * `[out] result`: A `napi_value` representing a JavaScript `string`. * `[out] copied`: Whether the string was copied. If it was, the finalizer will already have been invoked to destroy `str`.