Structs #25

Merged
merged 36 commits into from Sep 6, 2011

Conversation

Projects
None yet
2 participants
Member

TooTallNate commented Sep 5, 2011

This BIG ASS pull request contains an almost complete reworking of the Struct implementation.

  • Firstly, there were bugs in the current alignment/offset logic, which were uncovered in the test case for Structs that I added.
  • Now, passing a Struct type as a Function argument or return type is supported when the function returns/accepts the Struct instance by value. This fixes #23 and #24.
  • Add a new public function to Struct instances: ref() which returns the pointer to the struct instance (as per described in #24). This should be used with functions that accept struct instances by reference, as most do. So adding a little syntax sugar I thought was in order.
  • Added a "struct" example to the "examples" dir. To show that this is all working as expected.

On a related note, all the struct-related integration with NodObjC started to just work after implementing these features (as I already had the code in place, assuming this was how the API worked, until I realized who the read culprit was).

Cheers!

TooTallNate added some commits Aug 18, 2011

@TooTallNate TooTallNate On "darwin", surround the sync `ffi_call` invocation with an Objectiv…
…e-C @try/@catch.

This patch will not affect builds for other OSs. I also did not add a @try/@catch
for the "async" function case, simply because I do not need it (at least not at
the moment).

I ran the test suite and it still passes.
f000c71
@TooTallNate TooTallNate Merge remote branch 'ttn/@try/@catch' ccda928
@TooTallNate TooTallNate Add explicit cast since apparently 'eio_custom' has changed it's sign…
…ature in node 0.5.5.
0f540a6
@TooTallNate TooTallNate I've been told this is good to have, for forwards compatibility. f5fc31f
@TooTallNate TooTallNate Add a (temporary) hack workaround for the bad alignment logic in the …
…Struct implementation.
3524836
@TooTallNate TooTallNate Add a .gitignore file. 7945c4c
@TooTallNate TooTallNate Add proof-of-concept Struct example. Not yet working... fc2f2af
@TooTallNate TooTallNate Make it so that Struct Pointer return values work. f737fe7
@TooTallNate TooTallNate Struct support for FFI.sizeOf() 0b609f3
@TooTallNate TooTallNate Add ref() helper for Struct instances. dd01a89
@TooTallNate TooTallNate Ignore compiled lib files. 14b1821
@TooTallNate TooTallNate Revert "Add a (temporary) hack workaround for the bad alignment logic…
… in the Struct implementation."

This reverts commit 3524836.
17d452e
@TooTallNate TooTallNate Add test case for structs (on darwin x64 at least). Currently failing. 8e9385c
@TooTallNate TooTallNate Add more to the struct test case. 9daae94
@TooTallNate TooTallNate Formatting. Remove unused import. f34b6ed
@TooTallNate TooTallNate Rewrite the Struct alignment/offset/size logic, test cases pass. 9768c14
@TooTallNate TooTallNate whitespace, comments. 02f53bd
@TooTallNate TooTallNate whitespace 0947ab1
@TooTallNate TooTallNate Fix reference error. 9834ef2
@TooTallNate TooTallNate Definte the ffi_type Struct in JS-land. 23359f8
@TooTallNate TooTallNate Some optimizations. Remove FFI.typeNameFor(). 04411f3
@TooTallNate TooTallNate Add Struct#_ffiType() function, which creates an ffi_type struct inst…
…ance based off the given struct.
8f2801f
@TooTallNate TooTallNate Don't use FFI.typeNameFor(). c9b9c0d
@TooTallNate TooTallNate Update regular test case for new Struct behavior. a53a94c
@TooTallNate TooTallNate Pass the type in properly in Struct#_ffiType(). cf3608c
@TooTallNate TooTallNate ws and formatting d89b38a
@TooTallNate TooTallNate Terminate the 'elements' array with a NULL pointer. 1220f3b
@TooTallNate TooTallNate Use the real pointer size. 0c188fb
@TooTallNate TooTallNate Various cleanup. fcc57f4
@TooTallNate TooTallNate Remove this._paramPtrs; wasn't being used. 10818e7
@TooTallNate TooTallNate Use forEach(), to force a closure. bcfdeec
@TooTallNate TooTallNate more whitespace. 1ed082c
@TooTallNate TooTallNate Rewrite and consolidate the putterFunction logic, mostly to accept st…
…ruct instances by value.
13fda0e
@TooTallNate TooTallNate Fix 'string' reading and writing on Structs. 3916284
@TooTallNate TooTallNate More to the struct example. e4c0222
@TooTallNate TooTallNate Don't print out the function's toString() in test/struct.js. d93a60d
Contributor

rbranson commented Sep 6, 2011

Not surprising the structs were buggy. This all looks very good. Nice work.

@rbranson rbranson added a commit that referenced this pull request Sep 6, 2011

@rbranson rbranson Merge pull request #25 from TooTallNate/structs
Major re-work (aka working code, no longer my giant mess) of the structs code by TooTallNate.
b5d96fd

@rbranson rbranson merged commit b5d96fd into node-ffi:master Sep 6, 2011

Member

TooTallNate commented Sep 6, 2011

Excellent! Thanks for the merge! Can I get a version bump and an npm publish?

Contributor

rbranson commented Sep 7, 2011

Working on a Linux test on latest stable right now.

On Tue, Sep 6, 2011 at 12:14 PM, TooTallNate
reply@reply.github.com
wrote:

Excellent! Thanks for the merge! Can I get a version bump and an npm publish?

Reply to this email directly or view it on GitHub:
rbranson#25 (comment)

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