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/helper: Fix dealloc_c_utf8_alloced_from_rust #3

Merged

Conversation

vinipsmaker
Copy link

Previous implementation assumed Vec::shrink_to_fit would always make
Vec's length equal to its capacity. However, there was no such guarantee
and undefined behaviour could be triggered.

Currently Rust implementation assume this as well. A proper fix would
use boxed slices so we don't need to store capacity.

We may want to revisit this as the following Rust issue gets more
attention: rust-lang/rust#36284

Previous implementation assumed `Vec::shrink_to_fit` would always make
Vec's length equal to its capacity. However, there was no such guarantee
and undefined behaviour could be triggered.

Currently Rust implementation assume this as well. A proper fix would
use boxed slices so we don't need to store capacity.

We may want to revisit this as the following Rust issue gets more
attention: rust-lang/rust#36284
The previous approach was being used out of concern over the FFI
barrier. However, `size_t` is broadly available and can be used safely.
`low_level_api::misc::misc_u8_ptr_free` should be enough.
@ustulation ustulation merged commit 687d94e into ustulation:rfc-0041-low-level-api Sep 8, 2016
@vinipsmaker vinipsmaker deleted the fix-utf8-alloc2 branch September 8, 2016 15:28
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