-
Notifications
You must be signed in to change notification settings - Fork 172
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
Solving issues in https://github.com/saghul/txiki.js/issues/653 #654
base: master
Are you sure you want to change the base?
Conversation
@lal12 mine taking a look? |
I don't think it is that big of an issue. It is not quite obvious what happens, but that if a C function expects the memory to be existent for a while you have to take special care, that is just an inherent thing with memory unsafe programming languages. It also isn't quite "unreasonable" to expect such a behavior, since strings are primitives in JS and therefore effectively call-by-value. This PR would be additional overhead and less comfortable for most use cases. There can an argument be made for the automatic behavior of the parser though, since it cannot really decide what type to use (advanced buffer, string or simple ptr). I never really intended it to be a full C prototype parser, just for an easier way for people to use ffi without having to learn an API, so you can control it by using |
I guess allowing multiple things a string, a buffer, a pointer (or even a dedicated ffi string class) to be passed, would be an option though. |
While I agree in principle, as this would apply to pointers and buffers alike, for string it is just made worse by the attempt of a cleaner syntax. |
Some information about this should obviously go into the documentation. I am not fundamentally in disagreement, I guess it really depends on the use case, what in the end is more practical. But I just noticed another issue in the current implementation and the PR. If you get a char* pointer as a return and need it later (e.g. for passing it to a free function), it also would be lost. |
What do other ffi APIs do here? For example Python's cffi? Perhaps we could borrow some inspiration from there. |
This solves some issues I found out while debugging #653.
The current implementation of strings as an advanced type is extremely problematic.
What is really passed onto the C function is a clone of the original object with a shorter lifetime due to its scoping. This causes issues when that object is expected to be consumed later once the reference counter already collected it.
The new syntax is not as compact or nice, but there is no reasonable way to make the original better from what I can tell (without hacking the js syntax).
I already ported these changes to the test-suite and verified it worked.
There is one missing bit, at the moment
fromBuffer
for CString is very wasteful as it forces to decode and reencode.There is surely a better way to bind the buffer directly, but I am not very familiar with the intrinsics provided in txiki.