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

n-api: refactoring a previous commit #28848

Closed
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -0,0 +1,49 @@
#include <js_native_api.h>
#include "common.h"

#include <stdio.h>

void add_returned_status(napi_env env,
const char* key,
napi_value object,
napi_status expected_status,
This conversation was marked as resolved by gabrielschulhof

This comment has been minimized.

Copy link
@gabrielschulhof

gabrielschulhof Jul 24, 2019

Contributor

You need a char* expected_message here because the expected_status is not necessarily napi_invalid_arg. By omitting the char* expected_message you are essentially causing this function to output "Invalid argument" no matter what expected_status is, as long as it agrees with actual_status.

Basically, if expected_status agrees with actual_status, output expected_message, which could be "Invalid argument", but could be some other string corresponding to expected_status, and if they don't agree, do the snprintf().

This comment has been minimized.

Copy link
@gabrielschulhof

gabrielschulhof Jul 24, 2019

Contributor

Another way to look at it is that, since you cannot access the association between napi_invalid_arg and "Invalid argument", because env is NULL and thus you have no access to napi_get_last_error_info(), you must establish the association by passing the napi_status expected_status, const char* expected_message pair in the parameters by hand. It's not ideal, but it's the only thing you can do when env is NULL.

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");

This comment has been minimized.

Copy link
@addaleax

addaleax Jul 24, 2019

Member
Suggested change
snprintf(p_napi_message, 99, "Invalid argument");
snprintf(p_napi_message, sizeof(p_napi_message), "Invalid argument");

(Side note: why the p_? Do you know that?)

This comment has been minimized.

Copy link
@octaviansoldea

octaviansoldea Jul 24, 2019

Author Contributor

Hi @addaleax

Thank you for your message. Usually I use p_ for a name that indicates a pointer. Could you please suggest a better name?

Thank you in advance,
@octaviansoldea

This comment has been minimized.

Copy link
@addaleax

addaleax Jul 24, 2019

Member

Usually I use p_ for a name that indicates a pointer.

Okay, that makes sense, but we generally don’t to this elsewhere 🙂. How about just napi_message, or if you want to be explicit about the type, napi_message_string?

} else {
snprintf(p_napi_message, 99, "Invalid status [%d]", actual_status);

This comment has been minimized.

Copy link
@addaleax

addaleax Jul 24, 2019

Member
Suggested change
snprintf(p_napi_message, 99, "Invalid status [%d]", actual_status);
snprintf(p_napi_message, sizeof(p_napi_message), "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));
}
@@ -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);
@@ -3,6 +3,7 @@
{
"target_name": "test_constructor",
"sources": [
"../common.c",
"../entry_point.c",
"test_constructor.c"
]
@@ -1,35 +1,13 @@
#include <js_native_api.h>
#include "../common.h"

#include <stdio.h>

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;
}
@@ -3,6 +3,7 @@
{
"target_name": "test_object",
"sources": [
"../common.c",
"../entry_point.c",
"test_object.c"
]
@@ -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'
});
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.