search

creationix / seaduk

An implementation of nucleus-js using duktape and libuv implemented in C
Other
34 stars 5 forks source link

Accept buffer objects in addition to plain buffers #15

Open svaarala opened 7 years ago

svaarala commented 7 years ago

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: https://github.com/svaarala/duktape/pull/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.

creationix commented 7 years ago

Great work! I approve of this effort.

Let me know if you need help with any of my code.

creationix commented 7 years ago

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.)

svaarala commented 7 years ago

I'll release 1.7.0 in a few days and after that you could just remove the ifdefs.

svaarala commented 7 years ago

I mean, I can do that in this pull of course :)

creationix commented 7 years ago

Sounds great. I hadn't realized that 1.7.0 wasn't out yet.