-
Notifications
You must be signed in to change notification settings - Fork 107
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
Add FromBytes::zero_and_get_bytes
#2369
base: v0.8.x
Are you sure you want to change the base?
Conversation
Closes #2318 gherrit-pr-id: I38a568578de67f067fab936d17ba57332c6cdfed
/// | ||
/// # Examples | ||
/// | ||
/// TODO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO
// - `data` is valid for reads and writes of `len` bytes: | ||
// - `data` refers to a block of `len` bytes within a single allocated | ||
// object by invariant on `&mut self` | ||
// - TODO: Others? https://doc.rust-lang.org/std/ptr/index.html#safety |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO
/// | ||
/// TODO | ||
#[inline] | ||
fn zero_and_get_bytes(&mut self) -> &mut [u8] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: Bikeshed name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thoughts on the name:
zero_and_get_bytes
is a good and clear name, though its length is right on that border of being verbose.mut
could be mentioned but is not necessary when the namezero
implies mutation.fn zero(&mut self)
onFromZeros
could yield&[u8]
of all zeros, and that could be given the same name. I struggle to consider a situation where it's practically useful to have access to an all-zeros&[u8]
with the same address as some&mut T
, so 🤷🏻♀️
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v0.8.x #2369 +/- ##
==========================================
- Coverage 87.47% 87.09% -0.39%
==========================================
Files 16 16
Lines 6182 6209 +27
==========================================
Hits 5408 5408
- Misses 774 801 +27 ☔ View full report in Codecov by Sentry. |
/// This permits accessing the bytes of a type which may not implement | ||
/// [`IntoBytes`] by first zeroing any padding bytes which would be unsound | ||
/// to expose via [`IntoBytes::as_bytes`]. | ||
/// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it's worth mentioning why this has to be on FromBytes
and can't be on FromZeros
? It's obvious to me but it could help solidify the concepts of the library to new users.
let data: *mut Self = self; | ||
let data = data.cast::<u8>(); | ||
|
||
// SAFETY: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps FromBytes::read_from_io
could be refactored to use this in this PR, keeping the total number of unsafe
the same and simplifying its implementation further. It still needs the one unsafe
at the end.
// duration of its lifetime. | ||
// - By invariant on `&mut self`, the referent is not larger than | ||
// `isize::MAX` bytes, and the referent does not wrap around the | ||
// address space |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: period consistency
/// | ||
/// TODO | ||
#[inline] | ||
fn zero_and_get_bytes(&mut self) -> &mut [u8] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thoughts on the name:
zero_and_get_bytes
is a good and clear name, though its length is right on that border of being verbose.mut
could be mentioned but is not necessary when the namezero
implies mutation.fn zero(&mut self)
onFromZeros
could yield&[u8]
of all zeros, and that could be given the same name. I struggle to consider a situation where it's practically useful to have access to an all-zeros&[u8]
with the same address as some&mut T
, so 🤷🏻♀️
Closes #2318
gherrit-pr-id: I38a568578de67f067fab936d17ba57332c6cdfed