-
Notifications
You must be signed in to change notification settings - Fork 685
Add info to external pointer free callback #4642
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
Add info to external pointer free callback #4642
Conversation
f4d813c to
cc44908
Compare
cc44908 to
bf69fa4
Compare
galpeter
left a comment
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.
Minor doc change requests.
| **Prototype** | ||
|
|
||
| ```c | ||
| typedef void (*jerry_value_free_callback_t) (void *native_p); |
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.
Was wondering if it would make sense to have a "type" information as second argument for this callback. What do you think? Could this be a useful feature? This "type" could/would indicate what kind of data should be freed.
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.
I am neutral. If others think this is a good idea we could do it in another patch (before release).
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.
That's true. We could even do a different WIP PR to see how it would look.
docs/02.API-REFERENCE.md
Outdated
| **Summary** | ||
|
|
||
| Native free callback of an object. It is used in `jerry_object_native_info_t` and for external Array buffers. | ||
| Native free callback of an object. It is part of the type information provided by `jerry_object_native_info_t`. |
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.
I'm not sure I understand what do you mean with the It is part of the type information provided by jerry_object_native_info_t. sentence.
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.
jerry_object_native_info_t structure represents the internal type information of an object, and jerry_object_native_free_callback_t is a mandatory part of this type info. Let me know if you would prefer another sentence.
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.
For me the new sentence does not provide extra info, maybe?: Native free callback of an object. In addition to the memory pointer the callback receives the native info pointer which was provided by the jerry_set_native_ptr... method.
bf69fa4 to
94049eb
Compare
|
Patch is updated, please check. |
galpeter
left a comment
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.
lgtm
94049eb to
1b15414
Compare
Furthermore reduce memory consumption when only one external pointer is assigned to an object. JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg zherczeg.u-szeged@partner.samsung.com
1b15414 to
85dd50c
Compare
Furthermore reduce memory consumption when only one external pointer is assigned to an object.