-
Notifications
You must be signed in to change notification settings - Fork 30.3k
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
n-api: add optional string length parameter #15343
Changes from 9 commits
07e9df4
53c4e81
7a7dc8b
ef1af4a
8762a3c
c0b202c
cf0f4fa
90ef051
b0175ae
8e7bf47
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -933,12 +933,27 @@ napi_status napi_get_last_error_info(napi_env env, | |
} | ||
|
||
NAPI_NO_RETURN void napi_fatal_error(const char* location, | ||
const char* message) { | ||
node::FatalError(location, message); | ||
size_t location_len, | ||
const char* message, | ||
size_t message_len) { | ||
char* location_string = const_cast<char*>(location); | ||
char* message_string = const_cast<char*>(message); | ||
if (location_len != -1) { | ||
location_string = (char*) malloc(location_len * sizeof(char) + 1); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought we were going to use stack allocation, won't the malloc's result in a memory leak ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sampsongao pointed out that the process will terminate when we call napi_fatal_error so it should be ok. |
||
strncpy(location_string, location, location_len); | ||
location_string[location_len] = '\0'; | ||
} | ||
if (message_len != -1) { | ||
message_string = (char*) malloc(message_len * sizeof(char) + 1); | ||
strncpy(message_string, message, message_len); | ||
message_string[message_len] = '\0'; | ||
} | ||
node::FatalError(location_string, message_string); | ||
} | ||
|
||
napi_status napi_create_function(napi_env env, | ||
const char* utf8name, | ||
size_t length, | ||
napi_callback cb, | ||
void* callback_data, | ||
napi_value* result) { | ||
|
@@ -965,7 +980,7 @@ napi_status napi_create_function(napi_env env, | |
|
||
if (utf8name != nullptr) { | ||
v8::Local<v8::String> name_string; | ||
CHECK_NEW_FROM_UTF8(env, name_string, utf8name); | ||
CHECK_NEW_FROM_UTF8_LEN(env, name_string, utf8name, length); | ||
return_value->SetName(name_string); | ||
} | ||
|
||
|
@@ -976,6 +991,7 @@ napi_status napi_create_function(napi_env env, | |
|
||
napi_status napi_define_class(napi_env env, | ||
const char* utf8name, | ||
size_t length, | ||
napi_callback constructor, | ||
void* callback_data, | ||
size_t property_count, | ||
|
@@ -997,7 +1013,7 @@ napi_status napi_define_class(napi_env env, | |
isolate, v8impl::FunctionCallbackWrapper::Invoke, cbdata); | ||
|
||
v8::Local<v8::String> name_string; | ||
CHECK_NEW_FROM_UTF8(env, name_string, utf8name); | ||
CHECK_NEW_FROM_UTF8_LEN(env, name_string, utf8name, length); | ||
tpl->SetClassName(name_string); | ||
|
||
size_t static_property_count = 0; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
'use strict'; | ||
const common = require('../../common'); | ||
const assert = require('assert'); | ||
|
||
// Testing api calls for a constructor that defines properties | ||
const TestConstructor = | ||
require(`./build/${common.buildType}/test_constructor_name`); | ||
assert.equal(TestConstructor.name, 'MyObject'); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
#include <node_api.h> | ||
#include "../common.h" | ||
|
||
napi_ref constructor_; | ||
|
||
napi_value New(napi_env env, napi_callback_info info) { | ||
napi_value _this; | ||
NAPI_CALL(env, napi_get_cb_info(env, info, NULL, NULL, &_this, NULL)); | ||
|
||
return _this; | ||
} | ||
|
||
napi_value Init(napi_env env, napi_value exports) { | ||
napi_value cons; | ||
NAPI_CALL(env, napi_define_class( | ||
env, "MyObject_Extra", 8, New, NULL, 0, NULL, &cons)); | ||
|
||
NAPI_CALL(env, | ||
napi_create_reference(env, cons, 1, &constructor_)); | ||
return cons; | ||
} | ||
|
||
NAPI_MODULE(addon, Init) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
'use strict'; | ||
const common = require('../../common'); | ||
const assert = require('assert'); | ||
const child_process = require('child_process'); | ||
const test_fatal = require(`./build/${common.buildType}/test_fatal`); | ||
|
||
// Test in a child process because the test code will trigger a fatal error | ||
// that crashes the process. | ||
if (process.argv[2] === 'child') { | ||
test_fatal.TestStringLength(); | ||
return; | ||
} | ||
|
||
const p = child_process.spawnSync( | ||
process.execPath, [ '--napi-modules', __filename, 'child' ]); | ||
assert.ifError(p.error); | ||
assert.ok(p.stderr.toString().includes( | ||
'FATAL ERROR: test_fatal::Test fatal message')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should NAPI_EXTERN be added here ?