From 65eff40ad4bb7a3fdaaae38b65b8bae1efcfde14 Mon Sep 17 00:00:00 2001 From: Luciano Martorella Date: Tue, 23 Oct 2018 09:43:47 +0200 Subject: [PATCH 1/3] - Fixed missing void*data usage in PropertyDescriptors --- napi-inl.h | 15 +++++++++++---- test/object/object.cc | 26 ++++++++++++++++++++++++++ test/object/object.js | 17 ++++++++++++++--- 3 files changed, 51 insertions(+), 7 deletions(-) diff --git a/napi-inl.h b/napi-inl.h index 6d8a021c1..5e8d57f9e 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -176,6 +176,7 @@ struct AccessorCallbackData { CallbackInfo callbackInfo(env, info); AccessorCallbackData* callbackData = static_cast(callbackInfo.Data()); + callbackInfo.SetData(callbackData->data); return callbackData->getterCallback(callbackInfo); }); } @@ -186,6 +187,7 @@ struct AccessorCallbackData { CallbackInfo callbackInfo(env, info); AccessorCallbackData* callbackData = static_cast(callbackInfo.Data()); + callbackInfo.SetData(callbackData->data); callbackData->setterCallback(callbackInfo); return nullptr; }); @@ -193,6 +195,7 @@ struct AccessorCallbackData { Getter getterCallback; Setter setterCallback; + void* data; }; } // namespace details @@ -2603,9 +2606,10 @@ PropertyDescriptor::Accessor(Napi::Env env, const char* utf8name, Getter getter, napi_property_attributes attributes, - void* /*data*/) { + void* data) { typedef details::CallbackData CbData; auto callbackData = new CbData({ getter, nullptr }); + callbackData->data = data; napi_status status = AttachData(env, object, callbackData); NAPI_THROW_IF_FAILED(env, status, napi_property_descriptor()); @@ -2638,9 +2642,10 @@ inline PropertyDescriptor PropertyDescriptor::Accessor(Napi::Env env, Name name, Getter getter, napi_property_attributes attributes, - void* /*data*/) { + void* data) { typedef details::CallbackData CbData; auto callbackData = new CbData({ getter, nullptr }); + callbackData->data = data; napi_status status = AttachData(env, object, callbackData); NAPI_THROW_IF_FAILED(env, status, napi_property_descriptor()); @@ -2664,9 +2669,10 @@ inline PropertyDescriptor PropertyDescriptor::Accessor(Napi::Env env, Getter getter, Setter setter, napi_property_attributes attributes, - void* /*data*/) { + void* data) { typedef details::AccessorCallbackData CbData; auto callbackData = new CbData({ getter, setter }); + callbackData->data = data; napi_status status = AttachData(env, object, callbackData); NAPI_THROW_IF_FAILED(env, status, napi_property_descriptor()); @@ -2701,9 +2707,10 @@ inline PropertyDescriptor PropertyDescriptor::Accessor(Napi::Env env, Getter getter, Setter setter, napi_property_attributes attributes, - void* /*data*/) { + void* data) { typedef details::AccessorCallbackData CbData; auto callbackData = new CbData({ getter, setter }); + callbackData->data = data; napi_status status = AttachData(env, object, callbackData); NAPI_THROW_IF_FAILED(env, status, napi_property_descriptor()); diff --git a/test/object/object.cc b/test/object/object.cc index 0dfc61818..1815debf1 100644 --- a/test/object/object.cc +++ b/test/object/object.cc @@ -33,6 +33,10 @@ Value HasPropertyWithCStyleString(const CallbackInfo& info); Value HasPropertyWithCppStyleString(const CallbackInfo& info); static bool testValue = true; +// Used to test void* Data() integrity +struct UserDataHolder { + int32_t value; +}; Value TestGetter(const CallbackInfo& info) { return Boolean::New(info.Env(), testValue); @@ -42,6 +46,16 @@ void TestSetter(const CallbackInfo& info) { testValue = info[0].As(); } +Value TestGetterWithUd(const CallbackInfo& info) { + const UserDataHolder* holder = reinterpret_cast(info.Data()); + return Number::New(info.Env(), holder->value); +} + +void TestSetterWithUd(const CallbackInfo& info) { + UserDataHolder* holder = reinterpret_cast(info.Data()); + holder->value = info[0].As().Int32Value(); +} + Value TestFunction(const CallbackInfo& info) { return Boolean::New(info.Env(), true); } @@ -58,11 +72,15 @@ void DefineProperties(const CallbackInfo& info) { Env env = info.Env(); Boolean trueValue = Boolean::New(env, true); + UserDataHolder* holder = new UserDataHolder(); + holder->value = 1234; if (nameType.Utf8Value() == "literal") { obj.DefineProperties({ PropertyDescriptor::Accessor(env, obj, "readonlyAccessor", TestGetter), PropertyDescriptor::Accessor(env, obj, "readwriteAccessor", TestGetter, TestSetter), + PropertyDescriptor::Accessor(env, obj, "readonlyAccessorWithUd", TestGetterWithUd, napi_property_attributes::napi_default, reinterpret_cast(holder)), + PropertyDescriptor::Accessor(env, obj, "readwriteAccessorWithUd", TestGetterWithUd, TestSetterWithUd, napi_property_attributes::napi_default, reinterpret_cast(holder)), PropertyDescriptor::Value("readonlyValue", trueValue), PropertyDescriptor::Value("readwriteValue", trueValue, napi_writable), PropertyDescriptor::Value("enumerableValue", trueValue, napi_enumerable), @@ -76,6 +94,8 @@ void DefineProperties(const CallbackInfo& info) { // work around the issue. std::string str1("readonlyAccessor"); std::string str2("readwriteAccessor"); + std::string str1a("readonlyAccessorWithUd"); + std::string str2a("readwriteAccessorWithUd"); std::string str3("readonlyValue"); std::string str4("readwriteValue"); std::string str5("enumerableValue"); @@ -85,6 +105,8 @@ void DefineProperties(const CallbackInfo& info) { obj.DefineProperties({ PropertyDescriptor::Accessor(env, obj, str1, TestGetter), PropertyDescriptor::Accessor(env, obj, str2, TestGetter, TestSetter), + PropertyDescriptor::Accessor(env, obj, str1a, TestGetterWithUd, napi_property_attributes::napi_default, reinterpret_cast(holder)), + PropertyDescriptor::Accessor(env, obj, str2a, TestGetterWithUd, TestSetterWithUd, napi_property_attributes::napi_default, reinterpret_cast(holder)), PropertyDescriptor::Value(str3, trueValue), PropertyDescriptor::Value(str4, trueValue, napi_writable), PropertyDescriptor::Value(str5, trueValue, napi_enumerable), @@ -97,6 +119,10 @@ void DefineProperties(const CallbackInfo& info) { Napi::String::New(env, "readonlyAccessor"), TestGetter), PropertyDescriptor::Accessor(env, obj, Napi::String::New(env, "readwriteAccessor"), TestGetter, TestSetter), + PropertyDescriptor::Accessor(env, obj, + Napi::String::New(env, "readonlyAccessorWithUd"), TestGetterWithUd, napi_property_attributes::napi_default, reinterpret_cast(holder)), + PropertyDescriptor::Accessor(env, obj, + Napi::String::New(env, "readwriteAccessorWithUd"), TestGetterWithUd, TestSetterWithUd, napi_property_attributes::napi_default, reinterpret_cast(holder)), PropertyDescriptor::Value( Napi::String::New(env, "readonlyValue"), trueValue), PropertyDescriptor::Value( diff --git a/test/object/object.js b/test/object/object.js index 6b5884519..7622bdd1f 100644 --- a/test/object/object.js +++ b/test/object/object.js @@ -12,11 +12,11 @@ function test(binding) { assert.ok(propDesc[attribute]); } - function assertPropertyIsNot(obj, key, attribute) { - const propDesc = Object.getOwnPropertyDescriptor(obj, key); + function assertPropertyIsNot(obj, key, attribute) { + const propDesc = Object.getOwnPropertyDescriptor(obj, key); assert.ok(propDesc); assert.ok(!propDesc[attribute]); - } + } function testDefineProperties(nameType) { const obj = {}; @@ -26,6 +26,10 @@ function test(binding) { assertPropertyIsNot(obj, 'readonlyAccessor', 'configurable'); assert.strictEqual(obj.readonlyAccessor, true); + assertPropertyIsNot(obj, 'readonlyAccessorWithUd', 'enumerable'); + assertPropertyIsNot(obj, 'readonlyAccessorWithUd', 'configurable'); + assert.strictEqual(obj.readonlyAccessorWithUd, 1234, nameType); + assertPropertyIsNot(obj, 'readwriteAccessor', 'enumerable'); assertPropertyIsNot(obj, 'readwriteAccessor', 'configurable'); obj.readwriteAccessor = false; @@ -33,6 +37,13 @@ function test(binding) { obj.readwriteAccessor = true; assert.strictEqual(obj.readwriteAccessor, true); + assertPropertyIsNot(obj, 'readwriteAccessorWithUd', 'enumerable'); + assertPropertyIsNot(obj, 'readwriteAccessorWithUd', 'configurable'); + obj.readwriteAccessorWithUd = 2; + assert.strictEqual(obj.readwriteAccessorWithUd, 2); + obj.readwriteAccessorWithUd = -14; + assert.strictEqual(obj.readwriteAccessorWithUd, -14); + assertPropertyIsNot(obj, 'readonlyValue', 'writable'); assertPropertyIsNot(obj, 'readonlyValue', 'enumerable'); assertPropertyIsNot(obj, 'readonlyValue', 'configurable'); From 7572cd813dc746315bc687ee5baecbf1f0f72bbd Mon Sep 17 00:00:00 2001 From: Luciano Martorella Date: Sat, 27 Oct 2018 14:49:46 +0200 Subject: [PATCH 2/3] WIP --- test/object/object.cc | 20 ++++++++++---------- test/object/object.js | 24 ++++++++++++------------ 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/test/object/object.cc b/test/object/object.cc index 1815debf1..c94a4215c 100644 --- a/test/object/object.cc +++ b/test/object/object.cc @@ -46,12 +46,12 @@ void TestSetter(const CallbackInfo& info) { testValue = info[0].As(); } -Value TestGetterWithUd(const CallbackInfo& info) { +Value TestGetterWithUserData(const CallbackInfo& info) { const UserDataHolder* holder = reinterpret_cast(info.Data()); return Number::New(info.Env(), holder->value); } -void TestSetterWithUd(const CallbackInfo& info) { +void TestSetterWithUserData(const CallbackInfo& info) { UserDataHolder* holder = reinterpret_cast(info.Data()); holder->value = info[0].As().Int32Value(); } @@ -79,8 +79,8 @@ void DefineProperties(const CallbackInfo& info) { obj.DefineProperties({ PropertyDescriptor::Accessor(env, obj, "readonlyAccessor", TestGetter), PropertyDescriptor::Accessor(env, obj, "readwriteAccessor", TestGetter, TestSetter), - PropertyDescriptor::Accessor(env, obj, "readonlyAccessorWithUd", TestGetterWithUd, napi_property_attributes::napi_default, reinterpret_cast(holder)), - PropertyDescriptor::Accessor(env, obj, "readwriteAccessorWithUd", TestGetterWithUd, TestSetterWithUd, napi_property_attributes::napi_default, reinterpret_cast(holder)), + PropertyDescriptor::Accessor(env, obj, "readonlyAccessorWithUserData", TestGetterWithUserData, napi_property_attributes::napi_default, reinterpret_cast(holder)), + PropertyDescriptor::Accessor(env, obj, "readwriteAccessorWithUserData", TestGetterWithUserData, TestSetterWithUserData, napi_property_attributes::napi_default, reinterpret_cast(holder)), PropertyDescriptor::Value("readonlyValue", trueValue), PropertyDescriptor::Value("readwriteValue", trueValue, napi_writable), PropertyDescriptor::Value("enumerableValue", trueValue, napi_enumerable), @@ -94,8 +94,8 @@ void DefineProperties(const CallbackInfo& info) { // work around the issue. std::string str1("readonlyAccessor"); std::string str2("readwriteAccessor"); - std::string str1a("readonlyAccessorWithUd"); - std::string str2a("readwriteAccessorWithUd"); + std::string str1a("readonlyAccessorWithUserData"); + std::string str2a("readwriteAccessorWithUserData"); std::string str3("readonlyValue"); std::string str4("readwriteValue"); std::string str5("enumerableValue"); @@ -105,8 +105,8 @@ void DefineProperties(const CallbackInfo& info) { obj.DefineProperties({ PropertyDescriptor::Accessor(env, obj, str1, TestGetter), PropertyDescriptor::Accessor(env, obj, str2, TestGetter, TestSetter), - PropertyDescriptor::Accessor(env, obj, str1a, TestGetterWithUd, napi_property_attributes::napi_default, reinterpret_cast(holder)), - PropertyDescriptor::Accessor(env, obj, str2a, TestGetterWithUd, TestSetterWithUd, napi_property_attributes::napi_default, reinterpret_cast(holder)), + PropertyDescriptor::Accessor(env, obj, str1a, TestGetterWithUserData, napi_property_attributes::napi_default, reinterpret_cast(holder)), + PropertyDescriptor::Accessor(env, obj, str2a, TestGetterWithUserData, TestSetterWithUserData, napi_property_attributes::napi_default, reinterpret_cast(holder)), PropertyDescriptor::Value(str3, trueValue), PropertyDescriptor::Value(str4, trueValue, napi_writable), PropertyDescriptor::Value(str5, trueValue, napi_enumerable), @@ -120,9 +120,9 @@ void DefineProperties(const CallbackInfo& info) { PropertyDescriptor::Accessor(env, obj, Napi::String::New(env, "readwriteAccessor"), TestGetter, TestSetter), PropertyDescriptor::Accessor(env, obj, - Napi::String::New(env, "readonlyAccessorWithUd"), TestGetterWithUd, napi_property_attributes::napi_default, reinterpret_cast(holder)), + Napi::String::New(env, "readonlyAccessorWithUserData"), TestGetterWithUserData, napi_property_attributes::napi_default, reinterpret_cast(holder)), PropertyDescriptor::Accessor(env, obj, - Napi::String::New(env, "readwriteAccessorWithUd"), TestGetterWithUd, TestSetterWithUd, napi_property_attributes::napi_default, reinterpret_cast(holder)), + Napi::String::New(env, "readwriteAccessorWithUserData"), TestGetterWithUserData, TestSetterWithUserData, napi_property_attributes::napi_default, reinterpret_cast(holder)), PropertyDescriptor::Value( Napi::String::New(env, "readonlyValue"), trueValue), PropertyDescriptor::Value( diff --git a/test/object/object.js b/test/object/object.js index 7622bdd1f..b4fe57dff 100644 --- a/test/object/object.js +++ b/test/object/object.js @@ -12,11 +12,11 @@ function test(binding) { assert.ok(propDesc[attribute]); } - function assertPropertyIsNot(obj, key, attribute) { - const propDesc = Object.getOwnPropertyDescriptor(obj, key); + function assertPropertyIsNot(obj, key, attribute) { + const propDesc = Object.getOwnPropertyDescriptor(obj, key); assert.ok(propDesc); assert.ok(!propDesc[attribute]); - } + } function testDefineProperties(nameType) { const obj = {}; @@ -26,9 +26,9 @@ function test(binding) { assertPropertyIsNot(obj, 'readonlyAccessor', 'configurable'); assert.strictEqual(obj.readonlyAccessor, true); - assertPropertyIsNot(obj, 'readonlyAccessorWithUd', 'enumerable'); - assertPropertyIsNot(obj, 'readonlyAccessorWithUd', 'configurable'); - assert.strictEqual(obj.readonlyAccessorWithUd, 1234, nameType); + assertPropertyIsNot(obj, 'readonlyAccessorWithUserData', 'enumerable'); + assertPropertyIsNot(obj, 'readonlyAccessorWithUserData', 'configurable'); + assert.strictEqual(obj.readonlyAccessorWithUserData, 1234, nameType); assertPropertyIsNot(obj, 'readwriteAccessor', 'enumerable'); assertPropertyIsNot(obj, 'readwriteAccessor', 'configurable'); @@ -37,12 +37,12 @@ function test(binding) { obj.readwriteAccessor = true; assert.strictEqual(obj.readwriteAccessor, true); - assertPropertyIsNot(obj, 'readwriteAccessorWithUd', 'enumerable'); - assertPropertyIsNot(obj, 'readwriteAccessorWithUd', 'configurable'); - obj.readwriteAccessorWithUd = 2; - assert.strictEqual(obj.readwriteAccessorWithUd, 2); - obj.readwriteAccessorWithUd = -14; - assert.strictEqual(obj.readwriteAccessorWithUd, -14); + assertPropertyIsNot(obj, 'readwriteAccessorWithUserData', 'enumerable'); + assertPropertyIsNot(obj, 'readwriteAccessorWithUserData', 'configurable'); + obj.readwriteAccessorWithUserData = 2; + assert.strictEqual(obj.readwriteAccessorWithUserData, 2); + obj.readwriteAccessorWithUserData = -14; + assert.strictEqual(obj.readwriteAccessorWithUserData, -14); assertPropertyIsNot(obj, 'readonlyValue', 'writable'); assertPropertyIsNot(obj, 'readonlyValue', 'enumerable'); From c01f306d1a8eada6c9664d2bb756b495caef6f39 Mon Sep 17 00:00:00 2001 From: Luciano Martorella Date: Fri, 21 Dec 2018 11:07:50 +0100 Subject: [PATCH 3/3] - Fixed "missing initializer" C++ strict compiler issue --- napi-inl.deprecated.h | 4 ++-- napi-inl.h | 12 ++++-------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/napi-inl.deprecated.h b/napi-inl.deprecated.h index d00174357..f19aca76b 100644 --- a/napi-inl.deprecated.h +++ b/napi-inl.deprecated.h @@ -73,7 +73,7 @@ inline PropertyDescriptor PropertyDescriptor::Accessor(const char* utf8name, void* /*data*/) { typedef details::AccessorCallbackData CbData; // TODO: Delete when the function is destroyed - auto callbackData = new CbData({ getter, setter }); + auto callbackData = new CbData({ getter, setter, nullptr }); return PropertyDescriptor({ utf8name, @@ -104,7 +104,7 @@ inline PropertyDescriptor PropertyDescriptor::Accessor(napi_value name, void* /*data*/) { typedef details::AccessorCallbackData CbData; // TODO: Delete when the function is destroyed - auto callbackData = new CbData({ getter, setter }); + auto callbackData = new CbData({ getter, setter, nullptr }); return PropertyDescriptor({ nullptr, diff --git a/napi-inl.h b/napi-inl.h index 5e8d57f9e..140ca4147 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -2608,8 +2608,7 @@ PropertyDescriptor::Accessor(Napi::Env env, napi_property_attributes attributes, void* data) { typedef details::CallbackData CbData; - auto callbackData = new CbData({ getter, nullptr }); - callbackData->data = data; + auto callbackData = new CbData({ getter, data }); napi_status status = AttachData(env, object, callbackData); NAPI_THROW_IF_FAILED(env, status, napi_property_descriptor()); @@ -2644,8 +2643,7 @@ inline PropertyDescriptor PropertyDescriptor::Accessor(Napi::Env env, napi_property_attributes attributes, void* data) { typedef details::CallbackData CbData; - auto callbackData = new CbData({ getter, nullptr }); - callbackData->data = data; + auto callbackData = new CbData({ getter, data }); napi_status status = AttachData(env, object, callbackData); NAPI_THROW_IF_FAILED(env, status, napi_property_descriptor()); @@ -2671,8 +2669,7 @@ inline PropertyDescriptor PropertyDescriptor::Accessor(Napi::Env env, napi_property_attributes attributes, void* data) { typedef details::AccessorCallbackData CbData; - auto callbackData = new CbData({ getter, setter }); - callbackData->data = data; + auto callbackData = new CbData({ getter, setter, data }); napi_status status = AttachData(env, object, callbackData); NAPI_THROW_IF_FAILED(env, status, napi_property_descriptor()); @@ -2709,8 +2706,7 @@ inline PropertyDescriptor PropertyDescriptor::Accessor(Napi::Env env, napi_property_attributes attributes, void* data) { typedef details::AccessorCallbackData CbData; - auto callbackData = new CbData({ getter, setter }); - callbackData->data = data; + auto callbackData = new CbData({ getter, setter, data }); napi_status status = AttachData(env, object, callbackData); NAPI_THROW_IF_FAILED(env, status, napi_property_descriptor());