Address review comments, iterate.

Author: sunfishcode

examples/new_read.rs

@@ -1,28 +1,50 @@
 use rustix::io::read;
 use rustix::stdio::stdin;
+use std::mem::MaybeUninit;
 
 fn main() {
-    let buf = Vec::new();
+    let buf = vec![0_u8; 3];
     let _x: Vec<u8> = read(stdin(), buf).unwrap();
 
-    let mut buf = Vec::new();
+    let mut buf = vec![0_u8; 3];
     let _x: usize = read(stdin(), &mut buf).unwrap();
+    let _x: usize = read(stdin(), &mut *buf).unwrap();
 
     let mut buf = [0, 0, 0];
     let _x: usize = read(stdin(), &mut buf).unwrap();
 
-    // Why doesn't this work? This is reduced from src/fs/inotify.rs line 177.
+    let mut buf = [0, 0, 0];
+    let _x: usize = read(stdin(), &mut buf[..]).unwrap();
+
+    let mut buf = [
+        MaybeUninit::uninit(),
+        MaybeUninit::uninit(),
+        MaybeUninit::uninit(),
+    ];
+    let _x: (&mut [u8], &mut [MaybeUninit<u8>]) = read(stdin(), &mut buf).unwrap();
+
+    let mut buf = [
+        MaybeUninit::uninit(),
+        MaybeUninit::uninit(),
+        MaybeUninit::uninit(),
+    ];
+    let _x: (&mut [u8], &mut [MaybeUninit<u8>]) = read(stdin(), &mut buf[..]).unwrap();
+
+    // This is reduced from src/fs/inotify.rs line 177.
     struct Wrapper<'a>(&'a mut [u8]);
     impl<'a> Wrapper<'a> {
         fn read(&mut self) {
-            let _x: usize = read(stdin(), self.0).unwrap();
+            // Ideally we'd write this.
+            //let _x: usize = read(stdin(), self.0).unwrap();
+            // But we need to write this instead.
+            let _x: usize = read(stdin(), &mut *self.0).unwrap();
         }
     }
-    let mut buf = Vec::new();
+    let mut buf = vec![0_u8; 3];
     let mut wrapper = Wrapper(&mut buf);
     wrapper.read();
 
     // Why does this get two error messages?
-    let mut buf = [0, 0, 0];
-    let _x = read(stdin(), buf).unwrap();
+    //let mut buf = [0, 0, 0];
+    //let _x = read(stdin(), buf).unwrap();
 }

src/backend/linux_raw/io/syscalls.rs

@@ -24,19 +24,11 @@ use crate::fd::{AsFd, BorrowedFd, OwnedFd, RawFd};
 use crate::io::{self, DupFlags, FdFlags, IoSlice, IoSliceMut, ReadWriteFlags};
 use crate::ioctl::{IoctlOutput, RawOpcode};
 use core::cmp;
-use core::mem::MaybeUninit;
 use linux_raw_sys::general::{F_DUPFD_CLOEXEC, F_GETFD, F_SETFD};
 
 #[inline]
-pub(crate) fn read(fd: BorrowedFd<'_>, buf: &mut [MaybeUninit<u8>]) -> io::Result<usize> {
-    unsafe {
-        ret_usize(syscall!(
-            __NR_read,
-            fd,
-            buf.as_mut_ptr(),
-            pass_usize(buf.len())
-        ))
-    }
+pub(crate) unsafe fn read(fd: BorrowedFd<'_>, buf: (*mut u8, usize)) -> io::Result<usize> {
+    ret_usize(syscall!(__NR_read, fd, buf.0, pass_usize(buf.1)))
 }
 
 #[inline]

src/buffer.rs

@@ -8,32 +8,22 @@ use core::mem::MaybeUninit;
 use core::slice;
 
 /// A memory buffer that may be uninitialized.
-pub trait Buffer<T> {
-    /// The result of the process operation.
-    type Result;
-
-    /// Convert this buffer into a maybe-unitiailized view.
-    fn as_maybe_uninitialized(&mut self) -> &mut [MaybeUninit<T>];
-
-    /// Convert a finished buffer pointer into its result.
-    ///
-    /// # Safety
-    ///
-    /// At least `len` bytes of the buffer must now be initialized.
-    unsafe fn finish(self, len: usize) -> Self::Result;
-}
+pub trait Buffer<T>: private::Sealed<T> {}
 
-/// Implements [`Buffer`] around the a slice of bytes.
-///
-/// `Result` is a `usize` indicating how many bytes were written.
-impl<T> Buffer<T> for &mut [T] {
+// Implement `Buffer` for all the types that implement `Sealed`.
+impl<T> Buffer<T> for &mut [T] {}
+impl<T, const N: usize> Buffer<T> for &mut [T; N] {}
+impl<T> Buffer<T> for &mut Vec<T> {}
+impl<'a, T> Buffer<T> for &'a mut [MaybeUninit<T>] {}
+impl<'a, T, const N: usize> Buffer<T> for &'a mut [MaybeUninit<T>; N] {}
+impl<T> Buffer<T> for Vec<T> {}
+
+impl<T> private::Sealed<T> for &mut [T] {
     type Result = usize;
 
     #[inline]
-    fn as_maybe_uninitialized(&mut self) -> &mut [MaybeUninit<T>] {
-        // SAFETY: This just casts away the knowledge that the elements are
-        // initialized.
-        unsafe { core::mem::transmute::<&mut [T], &mut [MaybeUninit<T>]>(self) }
+    fn as_raw_parts_mut(&mut self) -> (*mut T, usize) {
+        (self.as_mut_ptr(), self.len())
     }
 
     #[inline]
@@ -42,17 +32,12 @@ impl<T> Buffer<T> for &mut [T] {
     }
 }
 
-/// Implements [`Buffer`] around the a slice of bytes.
-///
-/// `Result` is a `usize` indicating how many bytes were written.
-impl<T, const N: usize> Buffer<T> for &mut [T; N] {
+impl<T, const N: usize> private::Sealed<T> for &mut [T; N] {
     type Result = usize;
 
     #[inline]
-    fn as_maybe_uninitialized(&mut self) -> &mut [MaybeUninit<T>] {
-        // SAFETY: This just casts away the knowledge that the elements are
-        // initialized.
-        unsafe { core::mem::transmute::<&mut [T], &mut [MaybeUninit<T>]>(*self) }
+    fn as_raw_parts_mut(&mut self) -> (*mut T, usize) {
+        (self.as_mut_ptr(), N)
     }
 
     #[inline]
@@ -61,17 +46,15 @@ impl<T, const N: usize> Buffer<T> for &mut [T; N] {
     }
 }
 
-/// Implements [`Buffer`] around the a slice of bytes.
-///
-/// `Result` is a `usize` indicating how many bytes were written.
-impl<T> Buffer<T> for &mut Vec<T> {
+// `Vec` implements `DerefMut` to `&mut [T]`, however it doesn't get
+// auto-derefed in a `impl Buffer<u8>`, so we add this `impl` so that our users
+// don't have to add an extra `*` in these situations.
+impl<T> private::Sealed<T> for &mut Vec<T> {
     type Result = usize;
 
     #[inline]
-    fn as_maybe_uninitialized(&mut self) -> &mut [MaybeUninit<T>] {
-        // SAFETY: This just casts away the knowledge that the elements are
-        // initialized.
-        unsafe { core::mem::transmute::<&mut [T], &mut [MaybeUninit<T>]>(self) }
+    fn as_raw_parts_mut(&mut self) -> (*mut T, usize) {
+        (self.as_mut_ptr(), self.len())
     }
 
     #[inline]
@@ -80,16 +63,31 @@ impl<T> Buffer<T> for &mut Vec<T> {
     }
 }
 
-/// Implements [`Buffer`] around the a slice of uninitialized bytes.
-///
-/// `Result` is a pair of slices giving the initialized and uninitialized
-/// subslices after the new data is written.
-impl<'a, T> Buffer<T> for &'a mut [MaybeUninit<T>] {
+impl<'a, T> private::Sealed<T> for &'a mut [MaybeUninit<T>] {
     type Result = (&'a mut [T], &'a mut [MaybeUninit<T>]);
 
     #[inline]
-    fn as_maybe_uninitialized(&mut self) -> &mut [MaybeUninit<T>] {
-        self
+    fn as_raw_parts_mut(&mut self) -> (*mut T, usize) {
+        (self.as_mut_ptr().cast(), self.len())
+    }
+
+    #[inline]
+    unsafe fn finish(self, len: usize) -> Self::Result {
+        let (init, uninit) = self.split_at_mut(len);
+
+        // SAFETY: The user asserts that the slice is now initialized.
+        let init = slice::from_raw_parts_mut(init.as_mut_ptr().cast::<T>(), init.len());
+
+        (init, uninit)
+    }
+}
+
+impl<'a, T, const N: usize> private::Sealed<T> for &'a mut [MaybeUninit<T>; N] {
+    type Result = (&'a mut [T], &'a mut [MaybeUninit<T>]);
+
+    #[inline]
+    fn as_raw_parts_mut(&mut self) -> (*mut T, usize) {
+        (self.as_mut_ptr().cast(), self.len())
     }
 
     #[inline]
@@ -103,18 +101,14 @@ impl<'a, T> Buffer<T> for &'a mut [MaybeUninit<T>] {
     }
 }
 
-/// Implements [`Buffer`] around the `Vec` type.
-///
-/// This implementation fills the buffer, overwriting any previous data, with
-/// the new data data and sets the length.
 #[cfg(feature = "alloc")]
-impl<T> Buffer<T> for Vec<T> {
+impl<T> private::Sealed<T> for Vec<T> {
     type Result = Vec<T>;
 
     #[inline]
-    fn as_maybe_uninitialized(&mut self) -> &mut [MaybeUninit<T>] {
+    fn as_raw_parts_mut(&mut self) -> (*mut T, usize) {
         self.clear();
-        self.spare_capacity_mut()
+        (self.as_mut_ptr(), self.capacity())
     }
 
     #[inline]
@@ -143,6 +137,23 @@ pub(super) unsafe fn split_init(
     (init, uninit)
 }
 
+mod private {
+    pub trait Sealed<T> {
+        /// The result of the process operation.
+        type Result;
+
+        /// Return a pointer and length for this buffer.
+        fn as_raw_parts_mut(&mut self) -> (*mut T, usize);
+
+        /// Convert a finished buffer pointer into its result.
+        ///
+        /// # Safety
+        ///
+        /// At least `len` bytes of the buffer must now be initialized.
+        unsafe fn finish(self, len: usize) -> Self::Result;
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

src/fs/inotify.rs

@@ -40,8 +40,6 @@
 //! # }
 
 #![allow(unused_qualifications)]
-#![allow(dead_code)] // FIXME
-#![allow(unused_imports)] // FIXME
 
 use super::inotify;
 pub use crate::backend::fs::inotify::{CreateFlags, ReadFlags, WatchFlags};
@@ -176,17 +174,14 @@ impl<'buf, Fd: AsFd> Reader<'buf, Fd> {
     #[allow(clippy::should_implement_trait)]
     pub fn next(&mut self) -> io::Result<Event<'_>> {
         if self.is_buffer_empty() {
-            todo!("FIXME: see \"Why doesn't this work?\" in examples/new_read.rs");
-            /*
-            match read(self.fd.as_fd(), self.buf).map(|(init, _)| init.len()) {
+            match read(self.fd.as_fd(), &mut *self.buf).map(|(init, _)| init.len()) {
                 Ok(0) => return Err(Errno::INVAL),
                 Ok(bytes_read) => {
                     self.initialized = bytes_read;
                     self.offset = 0;
                 }
                 Err(e) => return Err(e),
             }
-            */
         }
 
         let ptr = self.buf[self.offset..].as_ptr();

src/io/read_write.rs

@@ -38,7 +38,8 @@ pub use backend::io::types::ReadWriteFlags;
 /// [glibc]: https://sourceware.org/glibc/manual/latest/html_node/I_002fO-Primitives.html#index-reading-from-a-file-descriptor
 #[inline]
 pub fn read<Fd: AsFd, Buf: Buffer<u8>>(fd: Fd, mut buf: Buf) -> io::Result<Buf::Result> {
-    let len = backend::io::syscalls::read(fd.as_fd(), buf.as_maybe_uninitialized())?;
+    // SAFETY: `read` behaves.
+    let len = unsafe { backend::io::syscalls::read(fd.as_fd(), buf.as_raw_parts_mut())? };
     // SAFETY: `read` works.
     unsafe { Ok(buf.finish(len)) }
 }