From 657d660dc89606fe90fb78aaccf76adf689fc137 Mon Sep 17 00:00:00 2001 From: Sampson Gao Date: Wed, 20 Sep 2017 17:36:56 -0400 Subject: [PATCH 1/4] n-api: fix warning about size_t compare with int --- src/node_api.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/node_api.cc b/src/node_api.cc index 8f8c400cdfbb21..f4b19928e81129 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -920,13 +920,13 @@ NAPI_NO_RETURN void napi_fatal_error(const char* location, size_t message_len) { char* location_string = const_cast(location); char* message_string = const_cast(message); - if (location_len != -1) { + if (static_cast(location_len) != -1) { location_string = reinterpret_cast( malloc(location_len * sizeof(char) + 1)); strncpy(location_string, location, location_len); location_string[location_len] = '\0'; } - if (message_len != -1) { + if (static_cast(message_len) != -1) { message_string = reinterpret_cast( malloc(message_len * sizeof(char) + 1)); strncpy(message_string, message, message_len); From b1bf44a2220633d94c144149787c8135caf581e6 Mon Sep 17 00:00:00 2001 From: Sampson Gao Date: Thu, 21 Sep 2017 18:23:07 -0400 Subject: [PATCH 2/4] Define NAPI_AUTO_LENGTH for string length --- src/node_api.cc | 6 ++-- src/node_api.h | 2 ++ test/addons-napi/test_async/test_async.cc | 8 +++--- .../test_constructor/test_constructor.c | 2 +- .../test_conversions/test_conversions.c | 3 +- .../test_env_sharing/compare_env.c | 3 +- test/addons-napi/test_error/test_error.cc | 28 ++++++++++++------- test/addons-napi/test_fatal/test_fatal.c | 3 +- .../addons-napi/test_function/test_function.c | 4 +-- test/addons-napi/test_general/test_general.c | 26 +++++++++++------ .../addons-napi/test_make_callback/binding.cc | 6 ++-- .../test_make_callback_recurse/binding.cc | 3 +- .../test_properties/test_properties.c | 4 +-- 13 files changed, 61 insertions(+), 37 deletions(-) diff --git a/src/node_api.cc b/src/node_api.cc index f4b19928e81129..97ba80a4de3a95 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -126,7 +126,7 @@ struct napi_env__ { } while (0) #define CHECK_NEW_FROM_UTF8(env, result, str) \ - CHECK_NEW_FROM_UTF8_LEN((env), (result), (str), -1) + CHECK_NEW_FROM_UTF8_LEN((env), (result), (str), NAPI_AUTO_LENGTH) #define GET_RETURN_STATUS(env) \ (!try_catch.HasCaught() ? napi_ok \ @@ -920,13 +920,13 @@ NAPI_NO_RETURN void napi_fatal_error(const char* location, size_t message_len) { char* location_string = const_cast(location); char* message_string = const_cast(message); - if (static_cast(location_len) != -1) { + if (static_cast(location_len) != NAPI_AUTO_LENGTH) { location_string = reinterpret_cast( malloc(location_len * sizeof(char) + 1)); strncpy(location_string, location, location_len); location_string[location_len] = '\0'; } - if (static_cast(message_len) != -1) { + if (static_cast(message_len) != NAPI_AUTO_LENGTH) { message_string = reinterpret_cast( malloc(message_len * sizeof(char) + 1)); strncpy(message_string, message, message_len); diff --git a/src/node_api.h b/src/node_api.h index 82f2e1900e2fe4..a3a07a64673366 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -100,6 +100,8 @@ typedef struct { #define NAPI_MODULE(modname, regfunc) \ NAPI_MODULE_X(modname, regfunc, NULL, 0) +#define NAPI_AUTO_LENGTH SIZE_MAX + EXTERN_C_START NAPI_EXTERN void napi_module_register(napi_module* mod); diff --git a/test/addons-napi/test_async/test_async.cc b/test/addons-napi/test_async/test_async.cc index 251573828b9fe8..4d3e025097ba18 100644 --- a/test/addons-napi/test_async/test_async.cc +++ b/test/addons-napi/test_async/test_async.cc @@ -95,8 +95,8 @@ napi_value Test(napi_env env, napi_callback_info info) { NAPI_CALL(env, napi_create_reference(env, argv[2], 1, &the_carrier._callback)); - NAPI_CALL(env, - napi_create_string_utf8(env, "TestResource", -1, &resource_name)); + NAPI_CALL(env, napi_create_string_utf8( + env, "TestResource", NAPI_AUTO_LENGTH, &resource_name)); NAPI_CALL(env, napi_create_async_work(env, argv[1], resource_name, Execute, Complete, &the_carrier, &the_carrier._request)); NAPI_CALL(env, @@ -145,8 +145,8 @@ napi_value TestCancel(napi_env env, napi_callback_info info) { napi_value resource_name; void* data; - NAPI_CALL(env, - napi_create_string_utf8(env, "TestResource", -1, &resource_name)); + NAPI_CALL(env, napi_create_string_utf8( + env, "TestResource", NAPI_AUTO_LENGTH, &resource_name)); // make sure the work we are going to cancel will not be // able to start by using all the threads in the pool diff --git a/test/addons-napi/test_constructor/test_constructor.c b/test/addons-napi/test_constructor/test_constructor.c index d742f2bcd1707b..70f53ec5a92f55 100644 --- a/test/addons-napi/test_constructor/test_constructor.c +++ b/test/addons-napi/test_constructor/test_constructor.c @@ -77,7 +77,7 @@ napi_value Init(napi_env env, napi_value exports) { }; napi_value cons; - NAPI_CALL(env, napi_define_class(env, "MyObject", -1, New, + NAPI_CALL(env, napi_define_class(env, "MyObject", NAPI_AUTO_LENGTH, New, NULL, sizeof(properties)/sizeof(*properties), properties, &cons)); NAPI_CALL(env, napi_create_reference(env, cons, 1, &constructor_)); diff --git a/test/addons-napi/test_conversions/test_conversions.c b/test/addons-napi/test_conversions/test_conversions.c index eafe375d66eba7..4f92bafa35b79d 100644 --- a/test/addons-napi/test_conversions/test_conversions.c +++ b/test/addons-napi/test_conversions/test_conversions.c @@ -81,7 +81,8 @@ napi_value AsString(napi_env env, napi_callback_info info) { napi_get_value_string_utf8(env, args[0], value, sizeof(value), NULL)); napi_value output; - NAPI_CALL(env, napi_create_string_utf8(env, value, -1, &output)); + NAPI_CALL(env, napi_create_string_utf8( + env, value, NAPI_AUTO_LENGTH, &output)); return output; } diff --git a/test/addons-napi/test_env_sharing/compare_env.c b/test/addons-napi/test_env_sharing/compare_env.c index da984b68f53f3e..6a93ce52c64025 100644 --- a/test/addons-napi/test_env_sharing/compare_env.c +++ b/test/addons-napi/test_env_sharing/compare_env.c @@ -15,7 +15,8 @@ napi_value compare(napi_env env, napi_callback_info info) { } napi_value Init(napi_env env, napi_value exports) { - NAPI_CALL(env, napi_create_function(env, "exports", -1, compare, NULL, &exports)); + NAPI_CALL(env, napi_create_function( + env, "exports", NAPI_AUTO_LENGTH, compare, NULL, &exports)); return exports; } diff --git a/test/addons-napi/test_error/test_error.cc b/test/addons-napi/test_error/test_error.cc index bd6cf8c2fe4800..4ab20bd7522a3b 100644 --- a/test/addons-napi/test_error/test_error.cc +++ b/test/addons-napi/test_error/test_error.cc @@ -18,7 +18,8 @@ napi_value checkError(napi_env env, napi_callback_info info) { napi_value throwExistingError(napi_env env, napi_callback_info info) { napi_value message; napi_value error; - NAPI_CALL(env, napi_create_string_utf8(env, "existing error", -1, &message)); + NAPI_CALL(env, napi_create_string_utf8( + env, "existing error", NAPI_AUTO_LENGTH, &message)); NAPI_CALL(env, napi_create_error(env, nullptr, message, &error)); NAPI_CALL(env, napi_throw(env, error)); return nullptr; @@ -62,7 +63,8 @@ napi_value throwTypeErrorCode(napi_env env, napi_callback_info info) { napi_value createError(napi_env env, napi_callback_info info) { napi_value result; napi_value message; - NAPI_CALL(env, napi_create_string_utf8(env, "error", -1, &message)); + NAPI_CALL(env, napi_create_string_utf8( + env, "error", NAPI_AUTO_LENGTH, &message)); NAPI_CALL(env, napi_create_error(env, nullptr, message, &result)); return result; } @@ -70,7 +72,8 @@ napi_value createError(napi_env env, napi_callback_info info) { napi_value createRangeError(napi_env env, napi_callback_info info) { napi_value result; napi_value message; - NAPI_CALL(env, napi_create_string_utf8(env, "range error", -1, &message)); + NAPI_CALL(env, napi_create_string_utf8( + env, "range error", NAPI_AUTO_LENGTH, &message)); NAPI_CALL(env, napi_create_range_error(env, nullptr, message, &result)); return result; } @@ -78,7 +81,8 @@ napi_value createRangeError(napi_env env, napi_callback_info info) { napi_value createTypeError(napi_env env, napi_callback_info info) { napi_value result; napi_value message; - NAPI_CALL(env, napi_create_string_utf8(env, "type error", -1, &message)); + NAPI_CALL(env, napi_create_string_utf8( + env, "type error", NAPI_AUTO_LENGTH, &message)); NAPI_CALL(env, napi_create_type_error(env, nullptr, message, &result)); return result; } @@ -87,8 +91,10 @@ napi_value createErrorCode(napi_env env, napi_callback_info info) { napi_value result; napi_value message; napi_value code; - NAPI_CALL(env, napi_create_string_utf8(env, "Error [error]", -1, &message)); - NAPI_CALL(env, napi_create_string_utf8(env, "ERR_TEST_CODE", -1, &code)); + NAPI_CALL(env, napi_create_string_utf8( + env, "Error [error]", NAPI_AUTO_LENGTH, &message)); + NAPI_CALL(env, napi_create_string_utf8( + env, "ERR_TEST_CODE", NAPI_AUTO_LENGTH, &code)); NAPI_CALL(env, napi_create_error(env, code, message, &result)); return result; } @@ -99,9 +105,10 @@ napi_value createRangeErrorCode(napi_env env, napi_callback_info info) { napi_value code; NAPI_CALL(env, napi_create_string_utf8(env, "RangeError [range error]", - -1, + NAPI_AUTO_LENGTH, &message)); - NAPI_CALL(env, napi_create_string_utf8(env, "ERR_TEST_CODE", -1, &code)); + NAPI_CALL(env, napi_create_string_utf8( + env, "ERR_TEST_CODE", NAPI_AUTO_LENGTH, &code)); NAPI_CALL(env, napi_create_range_error(env, code, message, &result)); return result; } @@ -112,9 +119,10 @@ napi_value createTypeErrorCode(napi_env env, napi_callback_info info) { napi_value code; NAPI_CALL(env, napi_create_string_utf8(env, "TypeError [type error]", - -1, + NAPI_AUTO_LENGTH, &message)); - NAPI_CALL(env, napi_create_string_utf8(env, "ERR_TEST_CODE", -1, &code)); + NAPI_CALL(env, napi_create_string_utf8( + env, "ERR_TEST_CODE", NAPI_AUTO_LENGTH, &code)); NAPI_CALL(env, napi_create_type_error(env, code, message, &result)); return result; } diff --git a/test/addons-napi/test_fatal/test_fatal.c b/test/addons-napi/test_fatal/test_fatal.c index 6d8fa6ff8613f2..add8bebf3be7e3 100644 --- a/test/addons-napi/test_fatal/test_fatal.c +++ b/test/addons-napi/test_fatal/test_fatal.c @@ -2,7 +2,8 @@ #include "../common.h" napi_value Test(napi_env env, napi_callback_info info) { - napi_fatal_error("test_fatal::Test", -1, "fatal message", -1); + napi_fatal_error("test_fatal::Test", NAPI_AUTO_LENGTH, + "fatal message", NAPI_AUTO_LENGTH); return NULL; } diff --git a/test/addons-napi/test_function/test_function.c b/test/addons-napi/test_function/test_function.c index 5ce06d670609c1..b0350e6ee22876 100644 --- a/test/addons-napi/test_function/test_function.c +++ b/test/addons-napi/test_function/test_function.c @@ -33,11 +33,11 @@ napi_value TestFunctionName(napi_env env, napi_callback_info info) { napi_value Init(napi_env env, napi_value exports) { napi_value fn1; NAPI_CALL(env, napi_create_function( - env, NULL, -1, TestCallFunction, NULL, &fn1)); + env, NULL, NAPI_AUTO_LENGTH, TestCallFunction, NULL, &fn1)); napi_value fn2; NAPI_CALL(env, napi_create_function( - env, "Name", -1, TestFunctionName, NULL, &fn2)); + env, "Name", NAPI_AUTO_LENGTH, TestFunctionName, NULL, &fn2)); napi_value fn3; NAPI_CALL(env, napi_create_function( diff --git a/test/addons-napi/test_general/test_general.c b/test/addons-napi/test_general/test_general.c index 099c1315c071fe..05cdd76e3c1e64 100644 --- a/test/addons-napi/test_general/test_general.c +++ b/test/addons-napi/test_general/test_general.c @@ -43,7 +43,7 @@ napi_value testGetNodeVersion(napi_env env, napi_callback_info info) { NAPI_CALL(env, napi_create_uint32(env, node_version->patch, &patch)); NAPI_CALL(env, napi_create_string_utf8(env, node_version->release, - (size_t)-1, + NAPI_AUTO_LENGTH, &release)); NAPI_CALL(env, napi_create_array_with_length(env, 4, &result)); NAPI_CALL(env, napi_set_element(env, result, 0, major)); @@ -120,21 +120,29 @@ napi_value testNapiTypeof(napi_env env, napi_callback_info info) { napi_value result = NULL; if (argument_type == napi_number) { - NAPI_CALL(env, napi_create_string_utf8(env, "number", -1, &result)); + NAPI_CALL(env, napi_create_string_utf8( + env, "number", NAPI_AUTO_LENGTH, &result)); } else if (argument_type == napi_string) { - NAPI_CALL(env, napi_create_string_utf8(env, "string", -1, &result)); + NAPI_CALL(env, napi_create_string_utf8( + env, "string", NAPI_AUTO_LENGTH, &result)); } else if (argument_type == napi_function) { - NAPI_CALL(env, napi_create_string_utf8(env, "function", -1, &result)); + NAPI_CALL(env, napi_create_string_utf8( + env, "function", NAPI_AUTO_LENGTH, &result)); } else if (argument_type == napi_object) { - NAPI_CALL(env, napi_create_string_utf8(env, "object", -1, &result)); + NAPI_CALL(env, napi_create_string_utf8( + env, "object", NAPI_AUTO_LENGTH, &result)); } else if (argument_type == napi_boolean) { - NAPI_CALL(env, napi_create_string_utf8(env, "boolean", -1, &result)); + NAPI_CALL(env, napi_create_string_utf8( + env, "boolean", NAPI_AUTO_LENGTH, &result)); } else if (argument_type == napi_undefined) { - NAPI_CALL(env, napi_create_string_utf8(env, "undefined", -1, &result)); + NAPI_CALL(env, napi_create_string_utf8( + env, "undefined", NAPI_AUTO_LENGTH, &result)); } else if (argument_type == napi_symbol) { - NAPI_CALL(env, napi_create_string_utf8(env, "symbol", -1, &result)); + NAPI_CALL(env, napi_create_string_utf8( + env, "symbol", NAPI_AUTO_LENGTH, &result)); } else if (argument_type == napi_null) { - NAPI_CALL(env, napi_create_string_utf8(env, "null", -1, &result)); + NAPI_CALL(env, napi_create_string_utf8( + env, "null", NAPI_AUTO_LENGTH, &result)); } return result; } diff --git a/test/addons-napi/test_make_callback/binding.cc b/test/addons-napi/test_make_callback/binding.cc index 8a6d94985c1962..4b0537ca07cf17 100644 --- a/test/addons-napi/test_make_callback/binding.cc +++ b/test/addons-napi/test_make_callback/binding.cc @@ -25,7 +25,8 @@ napi_value MakeCallback(napi_env env, napi_callback_info info) { NAPI_CALL(env, napi_typeof(env, func, &func_type)); napi_value resource_name; - NAPI_CALL(env, napi_create_string_utf8(env, "test", -1, &resource_name)); + NAPI_CALL(env, napi_create_string_utf8( + env, "test", NAPI_AUTO_LENGTH, &resource_name)); napi_async_context context; NAPI_CALL(env, napi_async_init(env, func, resource_name, &context)); @@ -45,7 +46,8 @@ napi_value MakeCallback(napi_env env, napi_callback_info info) { napi_value Init(napi_env env, napi_value exports) { napi_value fn; - NAPI_CALL(env, napi_create_function(env, NULL, -1, MakeCallback, NULL, &fn)); + NAPI_CALL(env, napi_create_function( + env, NULL, NAPI_AUTO_LENGTH, MakeCallback, NULL, &fn)); NAPI_CALL(env, napi_set_named_property(env, exports, "makeCallback", fn)); return exports; } diff --git a/test/addons-napi/test_make_callback_recurse/binding.cc b/test/addons-napi/test_make_callback_recurse/binding.cc index 212c9ec4022fa3..714e44773de623 100644 --- a/test/addons-napi/test_make_callback_recurse/binding.cc +++ b/test/addons-napi/test_make_callback_recurse/binding.cc @@ -20,7 +20,8 @@ napi_value MakeCallback(napi_env env, napi_callback_info info) { napi_value Init(napi_env env, napi_value exports) { napi_value fn; - NAPI_CALL(env, napi_create_function(env, NULL, -1, MakeCallback, NULL, &fn)); + NAPI_CALL(env, napi_create_function( + env, NULL, NAPI_AUTO_LENGTH, MakeCallback, NULL, &fn)); NAPI_CALL(env, napi_set_named_property(env, exports, "makeCallback", fn)); return exports; } diff --git a/test/addons-napi/test_properties/test_properties.c b/test/addons-napi/test_properties/test_properties.c index 4c3394e0e0349e..a95c7013c2cabb 100644 --- a/test/addons-napi/test_properties/test_properties.c +++ b/test/addons-napi/test_properties/test_properties.c @@ -66,14 +66,14 @@ napi_value Init(napi_env env, napi_value exports) { napi_value name_value; NAPI_CALL(env, napi_create_string_utf8(env, "NameKeyValue", - -1, + NAPI_AUTO_LENGTH, &name_value)); napi_value symbol_description; napi_value name_symbol; NAPI_CALL(env, napi_create_string_utf8(env, "NameKeySymbol", - -1, + NAPI_AUTO_LENGTH, &symbol_description)); NAPI_CALL(env, napi_create_symbol(env, symbol_description, From a7ab6861958ea07831c0dd78d73c24b83c5c8c74 Mon Sep 17 00:00:00 2001 From: Sampson Gao Date: Thu, 21 Sep 2017 20:54:50 -0400 Subject: [PATCH 3/4] Use std::string for copying string --- src/node_api.cc | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/node_api.cc b/src/node_api.cc index 97ba80a4de3a95..a13f45672c5ea3 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -120,7 +120,8 @@ struct napi_env__ { #define CHECK_NEW_FROM_UTF8_LEN(env, result, str, len) \ do { \ auto str_maybe = v8::String::NewFromUtf8( \ - (env)->isolate, (str), v8::NewStringType::kInternalized, (len)); \ + (env)->isolate, (str), v8::NewStringType::kInternalized, \ + static_cast(len)); \ CHECK_MAYBE_EMPTY((env), str_maybe, napi_generic_failure); \ (result) = str_maybe.ToLocalChecked(); \ } while (0) @@ -918,21 +919,26 @@ NAPI_NO_RETURN void napi_fatal_error(const char* location, size_t location_len, const char* message, size_t message_len) { - char* location_string = const_cast(location); - char* message_string = const_cast(message); + std::string location_string; + std::string message_string; + if (static_cast(location_len) != NAPI_AUTO_LENGTH) { - location_string = reinterpret_cast( - malloc(location_len * sizeof(char) + 1)); - strncpy(location_string, location, location_len); - location_string[location_len] = '\0'; + location_string.assign( + const_cast(location), location_len); + } else { + location_string.assign( + const_cast(location), strlen(location)); } + if (static_cast(message_len) != NAPI_AUTO_LENGTH) { - message_string = reinterpret_cast( - malloc(message_len * sizeof(char) + 1)); - strncpy(message_string, message, message_len); - message_string[message_len] = '\0'; + message_string.assign( + const_cast(message), message_len); + } else { + message_string.assign( + const_cast(message), strlen(message)); } - node::FatalError(location_string, message_string); + + node::FatalError(location_string.c_str(), message_string.c_str()); } napi_status napi_create_function(napi_env env, From c4732e712c6a1e89d8fd7fce9e26c7c533888e2a Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Fri, 22 Sep 2017 15:30:05 -0400 Subject: [PATCH 4/4] squash: address latest comments Make checks directly against NAPI_AUTO_LENGTH Add assert Remove one check in unrelated napi function that was causing a warning and is not needed --- src/node_api.cc | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/node_api.cc b/src/node_api.cc index a13f45672c5ea3..fbf20878abd702 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -117,8 +117,14 @@ struct napi_env__ { CHECK_TO_TYPE((env), Boolean, (context), (result), (src), \ napi_boolean_expected) +// n-api defines NAPI_AUTO_LENGHTH as the indicator that a string +// is null terminated. For V8 the equivalent is -1. The assert +// validates that our cast of NAPI_AUTO_LENGTH results in -1 as +// needed by V8. #define CHECK_NEW_FROM_UTF8_LEN(env, result, str, len) \ do { \ + static_assert(static_cast(NAPI_AUTO_LENGTH) == -1, \ + "Casting NAPI_AUTO_LENGTH to int must result in -1"); \ auto str_maybe = v8::String::NewFromUtf8( \ (env)->isolate, (str), v8::NewStringType::kInternalized, \ static_cast(len)); \ @@ -922,7 +928,7 @@ NAPI_NO_RETURN void napi_fatal_error(const char* location, std::string location_string; std::string message_string; - if (static_cast(location_len) != NAPI_AUTO_LENGTH) { + if (location_len != NAPI_AUTO_LENGTH) { location_string.assign( const_cast(location), location_len); } else { @@ -930,7 +936,7 @@ NAPI_NO_RETURN void napi_fatal_error(const char* location, const_cast(location), strlen(location)); } - if (static_cast(message_len) != NAPI_AUTO_LENGTH) { + if (message_len != NAPI_AUTO_LENGTH) { message_string.assign( const_cast(message), message_len); } else { @@ -3291,7 +3297,6 @@ napi_status napi_adjust_external_memory(napi_env env, int64_t change_in_bytes, int64_t* adjusted_value) { CHECK_ENV(env); - CHECK_ARG(env, &change_in_bytes); CHECK_ARG(env, adjusted_value); *adjusted_value = env->isolate->AdjustAmountOfExternalAllocatedMemory(