Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix as_slice and as_mut_slice for empty Buffers and ArrayBuffers #681

Merged
merged 1 commit into from
Feb 10, 2021

Conversation

jrose-signal
Copy link
Contributor

These buffers may have NULL as their address, which slice::from_raw_parts does not allow.

These buffers may have NULL as their address, which
slice::from_raw_parts does not allow.
@kjvalencik
Copy link
Member

kjvalencik commented Feb 10, 2021

@jrose-signal In what situations will Node have a null pointer for the backing buffer? I rolled back the changes to src/types/binary.rs and the tests still passed on v14.15.4.

Is this something that is "working" in some cases but, unsound and potentially not working in others (e.g., if an enum is relying on a niche).

@kjvalencik
Copy link
Member

Hmm, I guess it doesn't matter if we can reproduce or not. In either case it's clearly relying on UB. From the N-API docs:

If length is 0, this may be NULL or any other pointer value.

@kjvalencik kjvalencik merged commit eabe090 into neon-bindings:main Feb 10, 2021
@jrose-signal jrose-signal deleted the empty-buffers branch February 10, 2021 20:45
@jrose-signal
Copy link
Contributor Author

The newly-added tests do fail for me with Node v12.13.0 (at least, that's what I have as which node). I suspect there's also an optimization aspect to this; immediately to_vec() on an empty buffer's slice worked fine for me but doing more complicated work caused problems.

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.

2 participants