search

TooTallNate / ref

Turn Buffer instances into "pointers"
http://tootallnate.github.com/ref
453 stars 141 forks source link

Possible fix for node-ffi issue #244 #42

Closed saneki closed 8 years ago

saneki commented 8 years ago

Not sure if this will fix all cases, but causes my unit tests to pass instead of erroring with Assertion '(obj_data) != (nullptr)' failed after updating to node.js 4.

unbornchikken commented 8 years ago

Yes, that'd be da fix. Thanks for this PR, we're really low on time nowadays. @TooTallNate, do we merge?

unbornchikken commented 8 years ago

Side note, there is that WrapPointer method exists at ffi code base, so that gotta be fixed eventually. But AFAIK we're not mapping nulls with that.

TooTallNate commented 8 years ago

I think this is the proper fix, but can you add a regression test? Basically attempting to read a null pointer with > 0 bytes length and asserting that the resulting buffer.length === 0.

saneki commented 8 years ago

Done, but make sure I implemented it correctly

TooTallNate commented 8 years ago

Perfect! Landed in b151fd4af266c08f2c506fe9915570d2288b0377.

I'm still curious as to what caused the regression in the first place, but this seems like a proper fix (I see no reason to ever have a length on a NULL pointer, since that would cause a segfault), so not 100% necessary.