core-foundation-rs icon indicating copy to clipboard operation
core-foundation-rs copied to clipboard

Improve CFData and add CFMutableData type

Open lqf96 opened this issue 5 years ago • 3 comments

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 avatar Nov 13 '19 06:11 lqf96

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.

lqf96 avatar Nov 17 '19 08:11 lqf96

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

bors-servo avatar Feb 15 '21 21:02 bors-servo

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

@lqf96 👈 poke

mikeumus avatar Nov 07 '21 18:11 mikeumus