Skip to content

Commit

Permalink
Remove BufMut::bytes_vectored_mut() (#430)
Browse files Browse the repository at this point in the history
There are issues with regard to uninitialized memory. We are avoiding
stabilizing this function for now.
  • Loading branch information
carllerche committed Oct 16, 2020
1 parent 422048e commit 447530b
Show file tree
Hide file tree
Showing 4 changed files with 0 additions and 121 deletions.
91 changes: 0 additions & 91 deletions src/buf/buf_mut.rs
Expand Up @@ -4,9 +4,6 @@ use core::{
ptr, usize,
};

#[cfg(feature = "std")]
use std::fmt;

use alloc::{boxed::Box, vec::Vec};

/// A trait for values that provide sequential write access to bytes.
Expand Down Expand Up @@ -168,48 +165,6 @@ pub trait BufMut {
/// return an empty slice.
fn bytes_mut(&mut self) -> &mut [MaybeUninit<u8>];

/// Fills `dst` with potentially multiple mutable slices starting at `self`'s
/// current position.
///
/// If the `BufMut` is backed by disjoint slices of bytes, `bytes_vectored_mut`
/// enables fetching more than one slice at once. `dst` is a slice of
/// mutable `IoSliceMut` references, enabling the slice to be directly used with
/// [`readv`] without any further conversion. The sum of the lengths of all
/// the buffers in `dst` will be less than or equal to
/// `Buf::remaining_mut()`.
///
/// The entries in `dst` will be overwritten, but the data **contained** by
/// the slices **will not** be modified. If `bytes_vectored_mut` does not fill every
/// entry in `dst`, then `dst` is guaranteed to contain all remaining slices
/// in `self.
///
/// This is a lower level function. Most operations are done with other
/// functions.
///
/// # Implementer notes
///
/// This function should never panic. Once the end of the buffer is reached,
/// i.e., `BufMut::remaining_mut` returns 0, calls to `bytes_vectored_mut` must
/// return 0 without mutating `dst`.
///
/// Implementations should also take care to properly handle being called
/// with `dst` being a zero length slice.
///
/// [`readv`]: http://man7.org/linux/man-pages/man2/readv.2.html
#[cfg(feature = "std")]
fn bytes_vectored_mut<'a>(&'a mut self, dst: &mut [IoSliceMut<'a>]) -> usize {
if dst.is_empty() {
return 0;
}

if self.has_remaining_mut() {
dst[0] = IoSliceMut::from(self.bytes_mut());
1
} else {
0
}
}

/// Transfer bytes into `self` from `src` and advance the cursor by the
/// number of bytes written.
///
Expand Down Expand Up @@ -890,11 +845,6 @@ macro_rules! deref_forward_bufmut {
(**self).bytes_mut()
}

#[cfg(feature = "std")]
fn bytes_vectored_mut<'b>(&'b mut self, dst: &mut [IoSliceMut<'b>]) -> usize {
(**self).bytes_vectored_mut(dst)
}

unsafe fn advance_mut(&mut self, cnt: usize) {
(**self).advance_mut(cnt)
}
Expand Down Expand Up @@ -1057,44 +1007,3 @@ impl BufMut for Vec<u8> {
// The existence of this function makes the compiler catch if the BufMut
// trait is "object-safe" or not.
fn _assert_trait_object(_b: &dyn BufMut) {}

// ===== impl IoSliceMut =====

/// A buffer type used for `readv`.
///
/// This is a wrapper around an `std::io::IoSliceMut`, but does not expose
/// the inner bytes in a safe API, as they may point at uninitialized memory.
///
/// This is `repr(transparent)` of the `std::io::IoSliceMut`, so it is valid to
/// transmute them. However, as the memory might be uninitialized, care must be
/// taken to not *read* the internal bytes, only *write* to them.
#[repr(transparent)]
#[cfg(feature = "std")]
pub struct IoSliceMut<'a>(std::io::IoSliceMut<'a>);

#[cfg(feature = "std")]
impl fmt::Debug for IoSliceMut<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("IoSliceMut")
.field("len", &self.0.len())
.finish()
}
}

#[cfg(feature = "std")]
impl<'a> From<&'a mut [u8]> for IoSliceMut<'a> {
fn from(buf: &'a mut [u8]) -> IoSliceMut<'a> {
IoSliceMut(std::io::IoSliceMut::new(buf))
}
}

#[cfg(feature = "std")]
impl<'a> From<&'a mut [MaybeUninit<u8>]> for IoSliceMut<'a> {
fn from(buf: &'a mut [MaybeUninit<u8>]) -> IoSliceMut<'a> {
IoSliceMut(std::io::IoSliceMut::new(unsafe {
// We don't look at the contents, and `std::io::IoSliceMut`
// doesn't either.
mem::transmute::<&'a mut [MaybeUninit<u8>], &'a mut [u8]>(buf)
}))
}
}
9 changes: 0 additions & 9 deletions src/buf/ext/chain.rs
Expand Up @@ -3,8 +3,6 @@ use crate::{Buf, BufMut};

use core::mem::MaybeUninit;

#[cfg(feature = "std")]
use crate::buf::IoSliceMut;
#[cfg(feature = "std")]
use std::io::IoSlice;

Expand Down Expand Up @@ -210,13 +208,6 @@ where

self.b.advance_mut(cnt);
}

#[cfg(feature = "std")]
fn bytes_vectored_mut<'a>(&'a mut self, dst: &mut [IoSliceMut<'a>]) -> usize {
let mut n = self.a.bytes_vectored_mut(dst);
n += self.b.bytes_vectored_mut(&mut dst[n..]);
n
}
}

impl<T, U> IntoIterator for Chain<T, U>
Expand Down
2 changes: 0 additions & 2 deletions src/buf/mod.rs
Expand Up @@ -24,7 +24,5 @@ mod vec_deque;

pub use self::buf_impl::Buf;
pub use self::buf_mut::BufMut;
#[cfg(feature = "std")]
pub use self::buf_mut::IoSliceMut;
pub use self::ext::{BufExt, BufMutExt};
pub use self::iter::IntoIter;
19 changes: 0 additions & 19 deletions tests/test_buf_mut.rs
@@ -1,7 +1,5 @@
#![warn(rust_2018_idioms)]

#[cfg(feature = "std")]
use bytes::buf::IoSliceMut;
use bytes::{BufMut, BytesMut};
use core::fmt::Write;
use core::usize;
Expand Down Expand Up @@ -66,23 +64,6 @@ fn test_clone() {
assert!(buf != buf2);
}

#[cfg(feature = "std")]
#[test]
fn test_bufs_vec_mut() {
let b1: &mut [u8] = &mut [];
let b2: &mut [u8] = &mut [];
let mut dst = [IoSliceMut::from(b1), IoSliceMut::from(b2)];

// with no capacity
let mut buf = BytesMut::new();
assert_eq!(buf.capacity(), 0);
assert_eq!(1, buf.bytes_vectored_mut(&mut dst[..]));

// with capacity
let mut buf = BytesMut::with_capacity(64);
assert_eq!(1, buf.bytes_vectored_mut(&mut dst[..]));
}

#[test]
fn test_mut_slice() {
let mut v = vec![0, 0, 0, 0];
Expand Down

0 comments on commit 447530b

Please sign in to comment.