Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

48198 node api external strings #48339

Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
bd8045b
node-api: unify string creation
gabrielschulhof Jun 2, 2023
50a2109
node-api: add string creation benchmark
gabrielschulhof Jun 3, 2023
1364872
node-api: implement external strings
gabrielschulhof Jun 3, 2023
a9e24b9
add test
gabrielschulhof Jun 3, 2023
e1127d1
rename to node_api_*
gabrielschulhof Jun 4, 2023
953304c
put back build type
gabrielschulhof Jun 5, 2023
09f7cdf
make linter work
gabrielschulhof Jun 5, 2023
902cb33
rename as requested
gabrielschulhof Jun 6, 2023
754864e
use std::basic_string_view to calc length in case of NAPI_AUTO_LENGTH
gabrielschulhof Jun 6, 2023
95a9971
test for auto-length
gabrielschulhof Jun 7, 2023
b4f383b
format
gabrielschulhof Jun 7, 2023
05317ac
correct length
gabrielschulhof Jun 7, 2023
20ab0fb
doc
gabrielschulhof Jun 7, 2023
ca4c08c
use right length indicator
gabrielschulhof Jun 8, 2023
5de34d4
final adjustment
gabrielschulhof Jun 8, 2023
64f2498
Apply suggestions from code review
gabrielschulhof Jun 8, 2023
cb9d95a
Apply suggestions from code review
gabrielschulhof Jun 8, 2023
ed83494
remove utf8
gabrielschulhof Jun 9, 2023
e063901
handle the case where we copy the string
gabrielschulhof Jun 10, 2023
4ff5e53
implement the non-copying case
gabrielschulhof Jun 10, 2023
402c93e
Unify the two external cases
gabrielschulhof Jun 11, 2023
32a5600
factor out tracking
gabrielschulhof Jun 11, 2023
0b72009
document
gabrielschulhof Jun 11, 2023
50dfa5d
debug ctor dtor
gabrielschulhof Jun 11, 2023
3a96b04
typo
gabrielschulhof Jun 11, 2023
caccb8e
Apply suggestions from code review
gabrielschulhof Jun 11, 2023
5c11c81
Apply suggestions from code review
gabrielschulhof Jun 11, 2023
b651fca
use common distructor
gabrielschulhof Jun 12, 2023
668fd4d
format
gabrielschulhof Jun 12, 2023
c721439
Apply suggestions from code review
gabrielschulhof Jun 12, 2023
7f67a2f
md lint
gabrielschulhof Jun 12, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
remove utf8
  • Loading branch information
gabrielschulhof committed Jun 9, 2023
commit ed83494430241b039d32938251f3c74bd1cb6df6
30 changes: 0 additions & 30 deletions doc/api/n-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`

<!-- YAML
added: REPLACEME
-->

> 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`
Expand Down
2 changes: 0 additions & 2 deletions src/js_native_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 0 additions & 5 deletions src/js_native_api_v8.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check with Electron if this is needed for strings.

Expand Down
14 changes: 0 additions & 14 deletions test/js-native-api/test_string/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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));
Expand All @@ -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));
Expand All @@ -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));
Expand All @@ -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));
Expand All @@ -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));
Expand All @@ -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));
Expand Down
49 changes: 5 additions & 44 deletions test/js-native-api/test_string/test_string.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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),
Expand Down