-
Notifications
You must be signed in to change notification settings - Fork 13.6k
std: sys: io: io_slice: Add UEFI types #144350
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
base: master
Are you sure you want to change the base?
Conversation
5ee308f
to
17cd61d
Compare
// that `IoSlice` and `IoSliceMut` are compatible with the vectored types for all the | ||
// networking protocols. | ||
|
||
fn to_slice<T>(val: &T) -> &[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.
Even though this is just test code, the function is unsafe and should be marked as such.
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.
Done
#[inline] | ||
pub fn advance(&mut self, n: usize) { | ||
let count: u32 = n.try_into().unwrap(); | ||
self.len += count; |
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.
Shouldn't this be subtracting, not adding?
A couple other notes: I think this should use checked arithmetic since advance
is supposed to panic "when trying to advance beyond the end of the slice". Also the Unix and Windows impls also have a nice "advancing IoSlice beyond its length" panic message which would be good to include here.
Something like:
self.len = u32::try_from(n)
.ok()
.and_then(|n| self.len.checked_sub(n))
.expect("advancing IoSlice beyond its length");
(Ditto for the mut advance
impl)
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.
Fixed
]; | ||
|
||
assert_eq!(to_slice(&lhs), to_slice(&rhs)); | ||
} |
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.
Would be good to add a test or two for advance/as_slice/into_slice/as_mut_slice
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.
Added
UEFI networking APIs do support vectored read/write. While the types for UDP4, UDP6, TCP4 and TCP6 are defined separately, they are essentially the same C struct. So we can map IoSlice and IoSliceMut to have the same binary representation. Since all UEFI networking types for read/write are DSTs, `IoSlice` and `IoSliceMut` will need to be copied to the end of the transmit/receive structures. So having the same binary representation just allows us to do a single memcpy instead of having to loop and set the DST. Signed-off-by: Ayush Singh <[email protected]>
UEFI networking APIs do support vectored read/write. While the types for UDP4, UDP6, TCP4 and TCP6 are defined separately, they are essentially the same C struct. So we can map IoSlice and IoSliceMut to have the same binary representation.
Since all UEFI networking types for read/write are DSTs,
IoSlice
andIoSliceMut
will need to be copied to the end of the transmit/receive structures. So having the same binary representation just allows us to do a single memcpy instead of having to loop and set the DST.cc @nicholasbishop