Accept buffer objects in addition to plain buffers#15
Open
svaarala wants to merge 1 commit into
Open
Conversation
Reliably checking for buffer objects needs duk_is_buffer_data() which is available in Duktape master and will be backported to Duktape 1.7.0 maintenance release.
Owner
|
Great work! I approve of this effort. Let me know if you need help with any of my code. |
Owner
|
Should we just update the duktape submodule to the latest version so that we don't need the ifdefs? I don't think this project has enough users yet to worry about backwards compatibility. (If there are any, I've not heard of them.) |
Author
|
I'll release 1.7.0 in a few days and after that you could just remove the ifdefs. |
Author
|
I mean, I can do that in this pull of course :) |
Owner
|
Sounds great. I hadn't realized that 1.7.0 wasn't out yet. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is still a work in progress so don't merge yet!
Extend the script->C buffer data validation so that buffer objects are accepted in addition to plain buffers. This allows application code to create buffer data using e.g.
new Uint8Array(256)and then write it to a native binding like a socket. To validate buffer objects reliably (handling a zero-size fixed buffer correctly),duk_is_buffer_data()is needed from Duktape master or eventual Duktape 1.7.0 maintenance release: svaarala/duktape#1221.Going forward it may be good to promote the use of standard buffer objects (e.g. Uint8Array) rather than Duktape custom types. In Duktape 2.x plain buffers behave like Uint8Arrays with some limitations like not having a property table so the current approach of using plain buffers for data buffer passing is relatively clean for scripts. However, it'd be nice to allow scripts to create buffers using e.g.
new Uint8Array(256)and pass them into write() calls.Longer term plan for Duktape is to simplify the buffer data model so that script and C code would see as standard objects as possible. The plain buffer type may be eliminated at some point, and effort made to ensure standard buffer objects have more memory optimized representations. This should simplify buffer handling overall. For now it's good to accept plain buffers and buffer objects wherever applicable.
There's no need to change how plain buffers are used for handles and such as far as I can see, at least for now.