Skip to content

Conversation

@Place1
Copy link
Owner

@Place1 Place1 commented Jul 9, 2018

No description provided.

@Place1
Copy link
Owner Author

Place1 commented Jul 9, 2018

@romgrk I'm struggling a little bit with INOUT arguments. You can see my approach in this PR.

I'm trying to encapsule the logic and memory management of IN/OUT args in a Parameter class similar to your approach in node-gtk. I'd love some tips of maybe even a breakdown of how it should work to get me over the last bump.

Local<Array> js_array = Nan::New<Array>();
for (int i = 0; i < length; i++) {
// void** pointer = static_cast<void**>(static_cast<ulong>(native_array) + i * element_size);
void **pointer = static_cast<void **>(native_array + i * element_size);
Copy link

@romgrk romgrk Jul 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome if it works for you, but on my side, arithemics on pointers are byte based. E.g. if I add (void*)0x2000 + (int)1, I get (void*)0x2008. That's the reason for casting to a ulong.

@romgrk
Copy link

romgrk commented Jul 9, 2018

Do you have more specific questions? I see the general flow of the thing, but I'd need to understand the whole relevant parts of the codebase to spot specific bugs/issues.

General observations:

  • INOUT-arguments are similar to OUT-arguments in that you always need to add a level of indirection to them. In node-gtk, we first fill the GIArgument as for an IN-argument, then store that value in a pointer on the stack, then point the GIArgument to that stack-pointer.
  • You will eventually need a FreeGIArgument function, to deallocate any resources created during the function call. (for reference, here is PyGObject version: https://gitlab.gnome.org/GNOME/pygobject/blob/master/gi/pygi-argument.c#L1005-1271)
  • You're allocating your Out/In/InOutParameter instances on the heap. Function calling is a "hot" part of the bindings, if it's possible to do so you should use the stack only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants