From 77233338017f39ba0fab94d7047144912acca12f Mon Sep 17 00:00:00 2001 From: Octavian Soldea Date: Wed, 17 Jul 2019 23:28:37 -0700 Subject: [PATCH 1/2] n-api: refactoring a previous commit This is a refactoring of PR-URL: https://github.com/nodejs/node/issues/27628 following PR-URL: https://github.com/nodejs/node/pull/28505 --- test/js-native-api/common.c | 49 +++++ test/js-native-api/common.h | 8 + .../test_constructor/binding.gyp | 1 + .../test_constructor/test_constructor.c | 49 +---- test/js-native-api/test_object/binding.gyp | 1 + test/js-native-api/test_object/test.js | 24 +-- test/js-native-api/test_object/test_object.c | 198 +++++------------- 7 files changed, 129 insertions(+), 201 deletions(-) create mode 100644 test/js-native-api/common.c diff --git a/test/js-native-api/common.c b/test/js-native-api/common.c new file mode 100644 index 00000000000000..8fc508ddbcf3ec --- /dev/null +++ b/test/js-native-api/common.c @@ -0,0 +1,49 @@ +#include +#include "common.h" + +#include + +void add_returned_status(napi_env env, + const char* key, + napi_value object, + napi_status expected_status, + napi_status actual_status) { + + char p_napi_message[100] = ""; + napi_value prop_value; + + if (actual_status == expected_status) { + snprintf(p_napi_message, 99, "Invalid argument"); + } else { + snprintf(p_napi_message, 99, "Invalid status [%d]", actual_status); + } + + NAPI_CALL_RETURN_VOID(env, + napi_create_string_utf8(env, + p_napi_message, + NAPI_AUTO_LENGTH, + &prop_value)); + NAPI_CALL_RETURN_VOID(env, + napi_set_named_property(env, + object, + key, + prop_value)); +} + +void add_last_status(napi_env env, const char* key, napi_value return_value) { + napi_value prop_value; + const napi_extended_error_info* p_last_error; + NAPI_CALL_RETURN_VOID(env, napi_get_last_error_info(env, &p_last_error)); + + NAPI_CALL_RETURN_VOID(env, + napi_create_string_utf8(env, + (p_last_error->error_message == NULL ? + "napi_ok" : + p_last_error->error_message), + NAPI_AUTO_LENGTH, + &prop_value)); + NAPI_CALL_RETURN_VOID(env, napi_set_named_property(env, + return_value, + key, + prop_value)); +} diff --git a/test/js-native-api/common.h b/test/js-native-api/common.h index b16f944771c1df..0ab22f96029b89 100644 --- a/test/js-native-api/common.h +++ b/test/js-native-api/common.h @@ -58,3 +58,11 @@ #define DECLARE_NAPI_GETTER(name, func) \ { (name), NULL, NULL, (func), NULL, NULL, napi_default, NULL } + +void add_returned_status(napi_env env, + const char* key, + napi_value object, + napi_status expected_status, + napi_status actual_status); + +void add_last_status(napi_env env, const char* key, napi_value return_value); diff --git a/test/js-native-api/test_constructor/binding.gyp b/test/js-native-api/test_constructor/binding.gyp index 0b256fd234a693..160ff436341950 100644 --- a/test/js-native-api/test_constructor/binding.gyp +++ b/test/js-native-api/test_constructor/binding.gyp @@ -3,6 +3,7 @@ { "target_name": "test_constructor", "sources": [ + "../common.c", "../entry_point.c", "test_constructor.c" ] diff --git a/test/js-native-api/test_constructor/test_constructor.c b/test/js-native-api/test_constructor/test_constructor.c index 19e00fbf1ece25..3ef3e978b66635 100644 --- a/test/js-native-api/test_constructor/test_constructor.c +++ b/test/js-native-api/test_constructor/test_constructor.c @@ -1,35 +1,13 @@ #include #include "../common.h" -#include - static double value_ = 1; static double static_value_ = 10; -static void -add_named_status(napi_env env, const char* key, napi_value return_value) { - napi_value prop_value; - const napi_extended_error_info* p_last_error; - NAPI_CALL_RETURN_VOID(env, napi_get_last_error_info(env, &p_last_error)); - - NAPI_CALL_RETURN_VOID(env, - napi_create_string_utf8(env, - (p_last_error->error_message == NULL ? - "napi_ok" : - p_last_error->error_message), - NAPI_AUTO_LENGTH, - &prop_value)); - NAPI_CALL_RETURN_VOID(env, napi_set_named_property(env, - return_value, - key, - prop_value)); -} - static napi_value TestDefineClass(napi_env env, napi_callback_info info) { napi_status status; - napi_value result, return_value, prop_value; - char p_napi_message[100] = ""; + napi_value result, return_value; napi_property_descriptor property_descriptor = { "TestDefineClass", @@ -52,20 +30,7 @@ static napi_value TestDefineClass(napi_env env, &property_descriptor, &result); - if (status == napi_invalid_arg) { - snprintf(p_napi_message, 99, "Invalid argument"); - } else { - snprintf(p_napi_message, 99, "Invalid status [%d]", status); - } - - NAPI_CALL(env, napi_create_string_utf8(env, - p_napi_message, - NAPI_AUTO_LENGTH, - &prop_value)); - NAPI_CALL(env, napi_set_named_property(env, - return_value, - "envIsNull", - prop_value)); + add_returned_status(env, "envIsNull", return_value, napi_invalid_arg, status); napi_define_class(env, NULL, @@ -76,7 +41,7 @@ static napi_value TestDefineClass(napi_env env, &property_descriptor, &result); - add_named_status(env, "nameIsNull", return_value); + add_last_status(env, "nameIsNull", return_value); napi_define_class(env, "TrackedFunction", @@ -87,7 +52,7 @@ static napi_value TestDefineClass(napi_env env, &property_descriptor, &result); - add_named_status(env, "cbIsNull", return_value); + add_last_status(env, "cbIsNull", return_value); napi_define_class(env, "TrackedFunction", @@ -98,7 +63,7 @@ static napi_value TestDefineClass(napi_env env, &property_descriptor, &result); - add_named_status(env, "cbDataIsNull", return_value); + add_last_status(env, "cbDataIsNull", return_value); napi_define_class(env, "TrackedFunction", @@ -109,7 +74,7 @@ static napi_value TestDefineClass(napi_env env, NULL, &result); - add_named_status(env, "propertiesIsNull", return_value); + add_last_status(env, "propertiesIsNull", return_value); napi_define_class(env, @@ -121,7 +86,7 @@ static napi_value TestDefineClass(napi_env env, &property_descriptor, NULL); - add_named_status(env, "resultIsNull", return_value); + add_last_status(env, "resultIsNull", return_value); return return_value; } diff --git a/test/js-native-api/test_object/binding.gyp b/test/js-native-api/test_object/binding.gyp index 94a4cd72fb6d22..1cb35d974dece9 100644 --- a/test/js-native-api/test_object/binding.gyp +++ b/test/js-native-api/test_object/binding.gyp @@ -3,6 +3,7 @@ { "target_name": "test_object", "sources": [ + "../common.c", "../entry_point.c", "test_object.c" ] diff --git a/test/js-native-api/test_object/test.js b/test/js-native-api/test_object/test.js index b49cb04f326a94..33ab49bb6fb5c5 100644 --- a/test/js-native-api/test_object/test.js +++ b/test/js-native-api/test_object/test.js @@ -229,26 +229,26 @@ assert.strictEqual(newObject.test_string, 'test string'); // Verify that passing NULL to napi_set_property() results in the correct // error. assert.deepStrictEqual(test_object.TestSetProperty(), { - envIsNull: 'pass', - objectIsNull: 'pass', - keyIsNull: 'pass', - valueIsNull: 'pass' + envIsNull: 'Invalid argument', + objectIsNull: 'Invalid argument', + keyIsNull: 'Invalid argument', + valueIsNull: 'Invalid argument' }); // Verify that passing NULL to napi_has_property() results in the correct // error. assert.deepStrictEqual(test_object.TestHasProperty(), { - envIsNull: 'pass', - objectIsNull: 'pass', - keyIsNull: 'pass', - resultIsNull: 'pass' + envIsNull: 'Invalid argument', + objectIsNull: 'Invalid argument', + keyIsNull: 'Invalid argument', + resultIsNull: 'Invalid argument' }); // Verify that passing NULL to napi_get_property() results in the correct // error. assert.deepStrictEqual(test_object.TestGetProperty(), { - envIsNull: 'pass', - objectIsNull: 'pass', - keyIsNull: 'pass', - resultIsNull: 'pass' + envIsNull: 'Invalid argument', + objectIsNull: 'Invalid argument', + keyIsNull: 'Invalid argument', + resultIsNull: 'Invalid argument' }); diff --git a/test/js-native-api/test_object/test_object.c b/test/js-native-api/test_object/test_object.c index e3d21156005b36..815baf460e527a 100644 --- a/test/js-native-api/test_object/test_object.c +++ b/test/js-native-api/test_object/test_object.c @@ -341,8 +341,8 @@ static napi_value Unwrap(napi_env env, napi_callback_info info) { static napi_value TestSetProperty(napi_env env, napi_callback_info info) { - napi_status ret[4]; - napi_value object, key, value, prop_value; + napi_status status; + napi_value object, key, value; NAPI_CALL(env, napi_create_object(env, &object)); @@ -350,122 +350,58 @@ static napi_value TestSetProperty(napi_env env, NAPI_CALL(env, napi_create_object(env, &value)); - ret[0] = napi_set_property(NULL, object, key, value); - - ret[1] = napi_set_property(env, NULL, key, value); - - ret[2] = napi_set_property(env, object, NULL, value); - - ret[3] = napi_set_property(env, object, key, NULL); - - NAPI_CALL(env, napi_create_string_utf8(env, - (ret[0] == napi_invalid_arg ? - "pass" : "fail"), - NAPI_AUTO_LENGTH, - &prop_value)); - NAPI_CALL(env, napi_set_named_property(env, - object, - "envIsNull", - prop_value)); - - NAPI_CALL(env, napi_create_string_utf8(env, - (ret[1] == napi_invalid_arg ? - "pass" : "fail"), - NAPI_AUTO_LENGTH, - &prop_value)); - NAPI_CALL(env, napi_set_named_property(env, - object, - "objectIsNull", - prop_value)); - - NAPI_CALL(env, napi_create_string_utf8(env, - (ret[2] == napi_invalid_arg ? - "pass" : "fail"), - NAPI_AUTO_LENGTH, - &prop_value)); - NAPI_CALL(env, napi_set_named_property(env, - object, - "keyIsNull", - prop_value)); - - NAPI_CALL(env, napi_create_string_utf8(env, - (ret[3] == napi_invalid_arg ? - "pass" : "fail"), - NAPI_AUTO_LENGTH, - &prop_value)); - NAPI_CALL(env, napi_set_named_property(env, - object, - "valueIsNull", - prop_value)); + status = napi_set_property(NULL, object, key, value); + + add_returned_status(env, "envIsNull", object, napi_invalid_arg, status); + + napi_set_property(env, NULL, key, value); + + add_last_status(env, "objectIsNull", object); + + napi_set_property(env, object, NULL, value); + + add_last_status(env, "keyIsNull", object); + + napi_set_property(env, object, key, NULL); + + add_last_status(env, "valueIsNull", object); return object; } static napi_value TestHasProperty(napi_env env, napi_callback_info info) { - napi_status ret[4]; - napi_value object, key, prop_result; + napi_status status; + napi_value object, key; bool result; NAPI_CALL(env, napi_create_object(env, &object)); NAPI_CALL(env, napi_create_string_utf8(env, "", NAPI_AUTO_LENGTH, &key)); - ret[0] = napi_has_property(NULL, object, key, &result); - - ret[1] = napi_has_property(env, NULL, key, &result); - - ret[2] = napi_has_property(env, object, NULL, &result); - - ret[3] = napi_has_property(env, object, key, NULL); - - NAPI_CALL(env, napi_create_string_utf8(env, - (ret[0] == napi_invalid_arg ? - "pass" : "fail"), - NAPI_AUTO_LENGTH, - &prop_result)); - NAPI_CALL(env, napi_set_named_property(env, - object, - "envIsNull", - prop_result)); - - NAPI_CALL(env, napi_create_string_utf8(env, - (ret[1] == napi_invalid_arg ? - "pass" : "fail"), - NAPI_AUTO_LENGTH, - &prop_result)); - NAPI_CALL(env, napi_set_named_property(env, - object, - "objectIsNull", - prop_result)); - - NAPI_CALL(env, napi_create_string_utf8(env, - (ret[2] == napi_invalid_arg ? - "pass" : "fail"), - NAPI_AUTO_LENGTH, - &prop_result)); - NAPI_CALL(env, napi_set_named_property(env, - object, - "keyIsNull", - prop_result)); - - NAPI_CALL(env, napi_create_string_utf8(env, - (ret[3] == napi_invalid_arg ? - "pass" : "fail"), - NAPI_AUTO_LENGTH, - &prop_result)); - NAPI_CALL(env, napi_set_named_property(env, - object, - "resultIsNull", - prop_result)); + status = napi_has_property(NULL, object, key, &result); + + add_returned_status(env, "envIsNull", object, napi_invalid_arg, status); + + napi_has_property(env, NULL, key, &result); + + add_last_status(env, "objectIsNull", object); + + napi_has_property(env, object, NULL, &result); + + add_last_status(env, "keyIsNull", object); + + napi_has_property(env, object, key, NULL); + + add_last_status(env, "resultIsNull", object); return object; } static napi_value TestGetProperty(napi_env env, napi_callback_info info) { - napi_status ret[4]; - napi_value object, key, result, prop_result; + napi_status status; + napi_value object, key, result; NAPI_CALL(env, napi_create_object(env, &object)); @@ -473,53 +409,21 @@ static napi_value TestGetProperty(napi_env env, NAPI_CALL(env, napi_create_object(env, &result)); - ret[0] = napi_get_property(NULL, object, key, &result); - - ret[1] = napi_get_property(env, NULL, key, &result); - - ret[2] = napi_get_property(env, object, NULL, &result); - - ret[3] = napi_get_property(env, object, key, NULL); - - NAPI_CALL(env, napi_create_string_utf8(env, - (ret[0] == napi_invalid_arg ? - "pass" : "fail"), - NAPI_AUTO_LENGTH, - &prop_result)); - NAPI_CALL(env, napi_set_named_property(env, - object, - "envIsNull", - prop_result)); - - NAPI_CALL(env, napi_create_string_utf8(env, - (ret[1] == napi_invalid_arg ? - "pass" : "fail"), - NAPI_AUTO_LENGTH, - &prop_result)); - NAPI_CALL(env, napi_set_named_property(env, - object, - "objectIsNull", - prop_result)); - - NAPI_CALL(env, napi_create_string_utf8(env, - (ret[2] == napi_invalid_arg ? - "pass" : "fail"), - NAPI_AUTO_LENGTH, - &prop_result)); - NAPI_CALL(env, napi_set_named_property(env, - object, - "keyIsNull", - prop_result)); - - NAPI_CALL(env, napi_create_string_utf8(env, - (ret[3] == napi_invalid_arg ? - "pass" : "fail"), - NAPI_AUTO_LENGTH, - &prop_result)); - NAPI_CALL(env, napi_set_named_property(env, - object, - "resultIsNull", - prop_result)); + status = napi_get_property(NULL, object, key, &result); + + add_returned_status(env, "envIsNull", object, napi_invalid_arg, status); + + napi_get_property(env, NULL, key, &result); + + add_last_status(env, "objectIsNull", object); + + napi_get_property(env, object, NULL, &result); + + add_last_status(env, "keyIsNull", object); + + napi_get_property(env, object, key, NULL); + + add_last_status(env, "resultIsNull", object); return object; } From 34a8e1026e55bf668179cbec937c0d0cdf0d1cb2 Mon Sep 17 00:00:00 2001 From: Octavian Soldea Date: Wed, 24 Jul 2019 16:22:57 -0700 Subject: [PATCH 2/2] n-api: refactoring a previous commit - answer 1 --- test/js-native-api/common.c | 20 ++++++++++-------- test/js-native-api/common.h | 3 +++ .../test_constructor/test_constructor.c | 7 ++++++- test/js-native-api/test_object/test_object.c | 21 ++++++++++++++++--- 4 files changed, 38 insertions(+), 13 deletions(-) diff --git a/test/js-native-api/common.c b/test/js-native-api/common.c index 8fc508ddbcf3ec..a6de256147c22a 100644 --- a/test/js-native-api/common.c +++ b/test/js-native-api/common.c @@ -6,23 +6,25 @@ void add_returned_status(napi_env env, const char* key, napi_value object, + char* expected_message, napi_status expected_status, napi_status actual_status) { - char p_napi_message[100] = ""; + char napi_message_string[100] = ""; napi_value prop_value; - if (actual_status == expected_status) { - snprintf(p_napi_message, 99, "Invalid argument"); - } else { - snprintf(p_napi_message, 99, "Invalid status [%d]", actual_status); + if (actual_status != expected_status) { + snprintf(napi_message_string, sizeof(napi_message_string), "Invalid status [%d]", actual_status); } NAPI_CALL_RETURN_VOID(env, - napi_create_string_utf8(env, - p_napi_message, - NAPI_AUTO_LENGTH, - &prop_value)); + napi_create_string_utf8( + env, + (actual_status == expected_status ? + expected_message : + napi_message_string), + NAPI_AUTO_LENGTH, + &prop_value)); NAPI_CALL_RETURN_VOID(env, napi_set_named_property(env, object, diff --git a/test/js-native-api/common.h b/test/js-native-api/common.h index 0ab22f96029b89..a524e8c7a75182 100644 --- a/test/js-native-api/common.h +++ b/test/js-native-api/common.h @@ -1,3 +1,5 @@ +#include + // Empty value so that macros here are able to return NULL or void #define NAPI_RETVAL_NOTHING // Intentionally blank #define @@ -62,6 +64,7 @@ void add_returned_status(napi_env env, const char* key, napi_value object, + char* expected_message, napi_status expected_status, napi_status actual_status); diff --git a/test/js-native-api/test_constructor/test_constructor.c b/test/js-native-api/test_constructor/test_constructor.c index 3ef3e978b66635..6c14e39fe2e624 100644 --- a/test/js-native-api/test_constructor/test_constructor.c +++ b/test/js-native-api/test_constructor/test_constructor.c @@ -30,7 +30,12 @@ static napi_value TestDefineClass(napi_env env, &property_descriptor, &result); - add_returned_status(env, "envIsNull", return_value, napi_invalid_arg, status); + add_returned_status(env, + "envIsNull", + return_value, + "Invalid argument", + napi_invalid_arg, + status); napi_define_class(env, NULL, diff --git a/test/js-native-api/test_object/test_object.c b/test/js-native-api/test_object/test_object.c index 815baf460e527a..db2d30c64a7cf4 100644 --- a/test/js-native-api/test_object/test_object.c +++ b/test/js-native-api/test_object/test_object.c @@ -352,7 +352,12 @@ static napi_value TestSetProperty(napi_env env, status = napi_set_property(NULL, object, key, value); - add_returned_status(env, "envIsNull", object, napi_invalid_arg, status); + add_returned_status(env, + "envIsNull", + object, + "Invalid argument", + napi_invalid_arg, + status); napi_set_property(env, NULL, key, value); @@ -381,7 +386,12 @@ static napi_value TestHasProperty(napi_env env, status = napi_has_property(NULL, object, key, &result); - add_returned_status(env, "envIsNull", object, napi_invalid_arg, status); + add_returned_status(env, + "envIsNull", + object, + "Invalid argument", + napi_invalid_arg, + status); napi_has_property(env, NULL, key, &result); @@ -411,7 +421,12 @@ static napi_value TestGetProperty(napi_env env, status = napi_get_property(NULL, object, key, &result); - add_returned_status(env, "envIsNull", object, napi_invalid_arg, status); + add_returned_status(env, + "envIsNull", + object, + "Invalid argument", + napi_invalid_arg, + status); napi_get_property(env, NULL, key, &result);