Skip to content

Conversation

@lqf96
Copy link
Contributor

@lqf96 lqf96 commented Nov 13, 2019

This pull request includes:

  • Low-level CoreFoundation bindings for CFMutableData.
  • Implementation of CFMutableData type.
  • Implementation of IntoIterator trait for CFData.
  • Implementation of Deref, DerefMut, IntoIterator, Extend and FromIterator trait for CFMutableData.
  • Tests for CFData and CFMutableData.

Potential future improvements:

  • When invalid CFRange is detected, should we return a Result::Err instead of panicking?
  • data::IntoIter can be made more efficient. Currently data::IntoIter calls CFDataGetBytePtr on every iteration. We can instead call CFDataGetBytePtr only once and cache the data pointer for future iterations.

@lqf96
Copy link
Contributor Author

lqf96 commented Nov 17, 2019

Also the CoreFoundation documents and source code indicates that there are two kinds of CFMutableData: fixed (with limited capacity) and unlimited. An unlimited CFMutableData can grow to arbitrary large size, just like Rust's Vec<u8>. A fixed CFMutableData, however, cannot grow beyond the initial capacity specified. Any mutations that cause the buffer to grow beyond the capacity will result in UB (program termination through SIGILL in reality).

Hence, to build a safe wrapper around CFMutableData, we should check if the buffer is fixed or not and if mutating it's buffer (by setting a new length or appending data) will cause the buffer to go beyond it's capacity. The problem is that CoreFoundation does not provide any API for retrieving the growability and capacity of CFData, so I'm still thinking about a safe API design that can avoid such UB.

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #446) made this pull request unmergeable. Please resolve the merge conflicts.

@mikeumus
Copy link

mikeumus commented Nov 7, 2021

I think @bors-servo just likes to say things. He's on every PR saying the same thing.

@lqf96 👈 poke

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.

3 participants