Expose 'size_t' type #3

Merged
1 commit merged into from Nov 18, 2010

Projects

None yet

2 participants

@wadey

A lot of the functions in libc have parameters of type size_t (an example is gethostname(3)), so it makes sense to expose this type.

@rbranson

This sounds like a good idea, but the code in the commit looks to be incomplete. GetSizeT and PutSizeT methods would need to be added to the C++ Pointer class.

@wadey

Is that necessary for the "non-specific types"? It looks like this line is setting up the mapping:

https://github.com/rbranson/node-ffi/blob/ddba83d/lib/pointer.js#L77

And my test case change that calls ptr.getSizeT and ptr.putSizeT is passing.

I did notice something strange though. I added a logging statement on that line I referenced to see what the type mappings are:

console.log("binding: ", method, " -> ", suffix);

And the output:

binding:  Byte  ->  UInt8
binding:  Char  ->  Int8
binding:  UChar  ->  Int8
binding:  Short  ->  Int16
binding:  UShort  ->  Int16
binding:  Int  ->  Int32
binding:  UInt  ->  Int32
binding:  Long  ->  Int64
binding:  ULong  ->  Int64
binding:  LongLong  ->  Int64
binding:  ULongLong  ->  Int64
binding:  SizeT  ->  Int64

It looks like a lot of the unsigned types are getting mapped to the signed equivalent, is this correct?

@wadey

I actually found the bug with mapping the unsigned methods, I will submit a different pull request for that fix.

@rbranson

Given the precision (or lack thereof) for large integers in V8, I think perhaps some kind of wrapper object should be used. If we just pass around the actual value of size_t, it will probably get screwed up by V8's use of floating point numbers internally for values >2^32.

@rbranson

So check it.. just pushed some code that changes the Int64 methods on Pointer to return strings. They can also now accept strings. This fixes any potential data loss issues for >2^32 values.

@rbranson

Also, you're right, since size_t is already in FFI.Bindings.TYPE_SIZE_MAP, it can be handled on the JS side. Let me merge and test these.

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