Zero initialize Vec returned from mp4parse_fallible branch of allocate_read_buf. #173
Conversation
mp4parse/src/lib.rs
Outdated
| unsafe { | ||
| buf.set_len(size); | ||
| std::ptr::write_bytes(buf.as_mut_ptr(), 0, buf.len() * std::mem::size_of::<u8>()); | ||
| } |
There was a problem hiding this comment.
You can do buf.extend(std::iter::repeat(0).take(size)) to avoid unsafe code. A loop on 0..size with Vec::push calls would probably also compile down to the same thing.
There was a problem hiding this comment.
Possibly done the way it is to avoid needing to reserve more space? Let me know if I'm mistaken, but if we try_reserve(buf, n), buf.set_len(n), then zero the vec with a ptr, we don't trigger further reserves (set_len guarantees it won't modify buffers). If we extend or push we get will get an extra allocation when we push at len == cap.
Edit for further clarification: I believe the further reserve is an issue because it would result in a non-fallible allocation in the context where we only want fallible allocation (there is also performance, but that is a secondary concern).
There was a problem hiding this comment.
This is incorrect, the Vec API won’t reserve additional space in the case the iteration fits, and it does here.
There was a problem hiding this comment.
Note that Vec::push itself will never reallocate as long as it has space, too. In general, don't use unsafe code if you don't need to.
There was a problem hiding this comment.
Thanks for the correction. I was mistaken as to when the new reserves take place (I was under the impression it was at the time len hit cap).
…e_read_buf. Also delete vec_reserve, which is completely unused. Closes mozilla#172.
|
Thanks for the feedback from both of you. Updated to use extend. |
Zero initialize Vec returned from mp4parse_fallible branch of allocate_read_buf.
Also delete vec_reserve, which is completely unused.
Closes #172.
r? @SingingTree please