node-gtk
node-gtk copied to clipboard
romgrk
fix(value): handle transfer-full, in arguments properly
This is done by making V8ToGIArgument() accept the transfer status and react accordingly. I does not make the new argument optional; this forces me into thinking about the appropriate value to put in for various callsites of this function.
Closes #303
@peat-psuwit sorry for the delay, was traveling a bit this summer.
Overall this looks good, one thing though. For all the changes it makes, the PR only has a single test. Is it really the only case affected by these changes?
For all the changes it makes, the PR only has a single test. Is it really the only case affected by these changes?
Well, the thing is that I can't find more APIs in GStreamer, GLib or GTK that would be affected by this. I would prefer to have at least an equivalent test but for GObject. I considered adding an introspectable library for testing, but I'm not sure if it can be done using node-gyp system
Alright. There is #269 open to add libgimarshallingtest which does that kind of stuff, we can add more tests once the lib is used for testing. I'll merge as soon as you fix the last two nitpicks :]
How is this one related to #302? Do you want to include the memory ownership thing here?
It's a different problem. This one try to prevent use-after-free for transfer-full, in argument, while #302 concerns about memory leakage. Both are memory problems, but of different kinds...
This one can be merged without the fix for #302.