Skip to content

Commit

Permalink
Address review comments, iterate.
Browse files Browse the repository at this point in the history
  • Loading branch information
sunfishcode committed Jan 30, 2025
1 parent 0ff0e14 commit e455b94
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 76 deletions.
36 changes: 29 additions & 7 deletions examples/new_read.rs
Original file line number Diff line number Diff line change
@@ -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();
}
12 changes: 2 additions & 10 deletions src/backend/linux_raw/io/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
115 changes: 63 additions & 52 deletions src/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand Down Expand Up @@ -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::*;
Expand Down
7 changes: 1 addition & 6 deletions src/fs/inotify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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();
Expand Down
3 changes: 2 additions & 1 deletion src/io/read_write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)) }
}
Expand Down

0 comments on commit e455b94

Please sign in to comment.