Skip to content

Add read_buf equivalents for positioned reads #140459

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions library/std/src/fs/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,47 @@ fn file_test_io_read_write_at() {
check!(fs::remove_file(&filename));
}

#[test]
#[cfg(unix)]
fn test_read_buf_at() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test should get a couple of calls for read_buf_at (not exact) that aren't just eof. Or maybe split test_read_buf_at and read_buf_exact_at?

use crate::os::unix::fs::FileExt;

let tmpdir = tmpdir();
let filename = tmpdir.join("file_rt_io_file_test_read_buf_at.txt");
{
let oo = OpenOptions::new().create_new(true).write(true).read(true).clone();
let mut file = check!(oo.open(&filename));
check!(file.write_all(b"0123456789"));
}
{
let mut file = check!(File::open(&filename));
let mut buf: [MaybeUninit<u8>; 5] = [MaybeUninit::uninit(); 5];
let mut buf = BorrowedBuf::from(buf.as_mut_slice());

check!(file.read_buf_exact_at(buf.unfilled(), 2));
assert_eq!(buf.filled(), b"23456");

// Already full
check!(file.read_buf_exact_at(buf.unfilled(), 3));
check!(file.read_buf_exact_at(buf.unfilled(), 10));
assert_eq!(buf.filled(), b"23456");
assert_eq!(check!(file.stream_position()), 0);

// Exact read past eof fails
let err = file.read_buf_exact_at(buf.clear().unfilled(), 6).unwrap_err();
assert_eq!(err.kind(), ErrorKind::UnexpectedEof);
assert_eq!(check!(file.stream_position()), 0);

// Read past eof is noop
check!(file.read_buf_at(buf.clear().unfilled(), 10));
assert_eq!(buf.filled(), b"");
check!(file.read_buf_at(buf.clear().unfilled(), 11));
assert_eq!(buf.filled(), b"");
assert_eq!(check!(file.stream_position()), 0);
}
check!(fs::remove_file(&filename));
}

#[test]
#[cfg(unix)]
fn set_get_unix_permissions() {
Expand Down Expand Up @@ -566,6 +607,39 @@ fn file_test_io_seek_read_write() {
check!(fs::remove_file(&filename));
}

#[test]
#[cfg(windows)]
fn test_seek_read_buf() {
use crate::os::windows::fs::FileExt;

let tmpdir = tmpdir();
let filename = tmpdir.join("file_rt_io_file_test_seek_read_buf.txt");
{
let oo = OpenOptions::new().create_new(true).write(true).read(true).clone();
let mut file = check!(oo.open(&filename));
check!(file.write_all(b"0123456789"));
}
{
let mut file = check!(File::open(&filename));
let mut buf: [MaybeUninit<u8>; 1] = [MaybeUninit::uninit()];
let mut buf = BorrowedBuf::from(buf.as_mut_slice());

// Seek read
check!(file.seek_read_buf(buf.unfilled(), 8));
assert_eq!(buf.filled(), b"8");
assert_eq!(check!(file.stream_position()), 9);

// Empty seek read
check!(file.seek_read_buf(buf.unfilled(), 0));
assert_eq!(buf.filled(), b"8");

// Seek read past eof
check!(file.seek_read_buf(buf.clear().unfilled(), 10));
assert_eq!(buf.filled(), b"");
}
check!(fs::remove_file(&filename));
}

#[test]
fn file_test_read_buf() {
let tmpdir = tmpdir();
Expand Down
1 change: 1 addition & 0 deletions library/std/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@
#![feature(array_chunks)]
#![feature(bstr)]
#![feature(bstr_internals)]
#![feature(cfg_select)]
#![feature(char_internals)]
#![feature(clone_to_uninit)]
#![feature(core_intrinsics)]
Expand Down
92 changes: 92 additions & 0 deletions library/std/src/os/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use super::platform::fs::MetadataExt as _;
// Used for `File::read` on intra-doc links
use crate::ffi::OsStr;
use crate::fs::{self, OpenOptions, Permissions};
use crate::io::BorrowedCursor;
use crate::os::unix::io::{AsFd, AsRawFd};
use crate::path::Path;
use crate::sealed::Sealed;
Expand Down Expand Up @@ -130,6 +131,94 @@ pub trait FileExt {
if !buf.is_empty() { Err(io::Error::READ_EXACT_EOF) } else { Ok(()) }
}

/// Reads some bytes starting from a given offset into the buffer.
///
/// This equivalent to the [`read_at`](FileExt::read_at) method,
/// except that it is passed a [`BorrowedCursor`] rather than `&mut [u8]` to allow
/// use with uninitialized buffers. The new data will be appended to any
/// existing contents of `buf`.
Comment on lines +136 to +139
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small style nit, docs can wrap at 100 chars

///
/// # Examples
///
/// ```no_run
/// #![feature(core_io_borrowed_buf)]
/// #![feature(read_buf_at)]
///
/// use std::io;
/// use std::io::BorrowedBuf;
/// use std::fs::File;
/// use std::mem::MaybeUninit;
/// use std::os::unix::prelude::*;
///
/// fn main() -> io::Result<()> {
/// let mut file = File::open("pi.txt")?;
///
/// // Read some bytes starting from offset 2
/// let mut buf: [MaybeUninit<u8>; 10] = [MaybeUninit::uninit(); 10];
/// let mut buf = BorrowedBuf::from(buf.as_mut_slice());
/// file.read_buf_at(buf.unfilled(), 2)?;
///
/// assert!(buf.filled().starts_with(b"1"));
///
/// Ok(())
/// }
/// ```
#[unstable(feature = "read_buf_at", issue = "140771")]
fn read_buf_at(&self, buf: BorrowedCursor<'_>, offset: u64) -> io::Result<()> {
io::default_read_buf(|b| self.read_at(b, offset), buf)
}

/// Reads the exact number of bytes required to fill the buffer from a given
/// offset.
///
/// This is equivalent to the [`read_exact_at`](FileExt::read_exact_at) method,
/// except that it is passed a [`BorrowedCursor`] rather than `&mut [u8]` to allow
/// use with uninitialized buffers. The new data will be appended to any
/// existing contents of `buf`.
///
/// # Examples
///
/// ```no_run
/// #![feature(core_io_borrowed_buf)]
/// #![feature(read_buf_at)]
///
/// use std::io;
/// use std::io::BorrowedBuf;
/// use std::fs::File;
/// use std::mem::MaybeUninit;
/// use std::os::unix::prelude::*;
///
/// fn main() -> io::Result<()> {
/// let mut file = File::open("pi.txt")?;
///
/// // Read exactly 10 bytes starting from offset 2
/// let mut buf: [MaybeUninit<u8>; 10] = [MaybeUninit::uninit(); 10];
/// let mut buf = BorrowedBuf::from(buf.as_mut_slice());
/// file.read_buf_exact_at(buf.unfilled(), 2)?;
///
/// assert_eq!(buf.filled(), b"1415926535");
///
/// Ok(())
/// }
/// ```
#[unstable(feature = "read_buf_at", issue = "140771")]
fn read_buf_exact_at(&self, mut buf: BorrowedCursor<'_>, mut offset: u64) -> io::Result<()> {
while buf.capacity() > 0 {
let prev_written = buf.written();
match self.read_buf_at(buf.reborrow(), offset) {
Ok(()) => {}
Err(e) if e.is_interrupted() => {}
Err(e) => return Err(e),
}
let n = buf.written() - prev_written;
offset += n as u64;
if n == 0 {
return Err(io::Error::READ_EXACT_EOF);
}
}
Ok(())
}

/// Writes a number of bytes starting from a given offset.
///
/// Returns the number of bytes written.
Expand Down Expand Up @@ -264,6 +353,9 @@ impl FileExt for fs::File {
fn read_at(&self, buf: &mut [u8], offset: u64) -> io::Result<usize> {
self.as_inner().read_at(buf, offset)
}
fn read_buf_at(&self, buf: BorrowedCursor<'_>, offset: u64) -> io::Result<()> {
self.as_inner().read_buf_at(buf, offset)
}
fn read_vectored_at(&self, bufs: &mut [io::IoSliceMut<'_>], offset: u64) -> io::Result<usize> {
self.as_inner().read_vectored_at(bufs, offset)
}
Expand Down
45 changes: 45 additions & 0 deletions library/std/src/os/windows/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#![stable(feature = "rust1", since = "1.0.0")]

use crate::fs::{self, Metadata, OpenOptions};
use crate::io::BorrowedCursor;
use crate::path::Path;
use crate::sealed::Sealed;
use crate::sys_common::{AsInner, AsInnerMut, IntoInner};
Expand Down Expand Up @@ -49,6 +50,46 @@ pub trait FileExt {
#[stable(feature = "file_offset", since = "1.15.0")]
fn seek_read(&self, buf: &mut [u8], offset: u64) -> io::Result<usize>;

/// Seeks to a given position and reads some bytes into the buffer.
///
/// This is equivalent to the [`seek_read`](FileExt::seek_read) method, except
/// that it is passed a [`BorrowedCursor`] rather than `&mut [u8]` to allow use
/// with uninitialized buffers. The new data will be appended to any existing
/// contents of `buf`.
///
/// Reading beyond the end of the file will always succeed without reading
/// any bytes.
///
/// # Examples
///
/// ```no_run
/// #![feature(core_io_borrowed_buf)]
/// #![feature(read_buf_at)]
///
/// use std::io;
/// use std::io::BorrowedBuf;
/// use std::fs::File;
/// use std::mem::MaybeUninit;
/// use std::os::windows::prelude::*;
///
/// fn main() -> io::Result<()> {
/// let mut file = File::open("pi.txt")?;
///
/// // Read some bytes starting from offset 2
/// let mut buf: [MaybeUninit<u8>; 10] = [MaybeUninit::uninit(); 10];
/// let mut buf = BorrowedBuf::from(buf.as_mut_slice());
/// file.seek_read_buf(buf.unfilled(), 2)?;
///
/// assert!(buf.filled().starts_with(b"1"));
///
/// Ok(())
/// }
/// ```
#[unstable(feature = "read_buf_at", issue = "140771")]
fn seek_read_buf(&self, buf: BorrowedCursor<'_>, offset: u64) -> io::Result<()> {
io::default_read_buf(|b| self.seek_read(b, offset), buf)
}

/// Seeks to a given position and writes a number of bytes.
///
/// Returns the number of bytes written.
Expand Down Expand Up @@ -89,6 +130,10 @@ impl FileExt for fs::File {
self.as_inner().read_at(buf, offset)
}

fn seek_read_buf(&self, buf: BorrowedCursor<'_>, offset: u64) -> io::Result<()> {
self.as_inner().read_buf_at(buf, offset)
}

fn seek_write(&self, buf: &[u8], offset: u64) -> io::Result<usize> {
self.as_inner().write_at(buf, offset)
}
Expand Down
61 changes: 39 additions & 22 deletions library/std/src/sys/fd/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,19 @@ const fn max_iov() -> usize {
16 // The minimum value required by POSIX.
}

crate::cfg_select! {
any(
all(target_os = "linux", not(target_env = "musl")),
target_os = "android",
target_os = "hurd",
) => {
use libc::pread64;
}
_ => {
use libc::pread as pread64;
}
}
Comment on lines +90 to +101
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should move to the top with the other imports (mentioned this in another comment), but actually do we need this remapping? From what I can gather at https://pubs.opengroup.org/onlinepubs/7908799/xsh/read.html and https://linux.die.net/man/2/pread64, pread is the POSIX name and Linux just changed its syscalls.


impl FileDesc {
#[inline]
pub fn try_clone(&self) -> io::Result<Self> {
Expand Down Expand Up @@ -146,42 +159,47 @@ impl FileDesc {
(&mut me).read_to_end(buf)
}

#[cfg_attr(target_os = "vxworks", allow(unused_unsafe))]
pub fn read_at(&self, buf: &mut [u8], offset: u64) -> io::Result<usize> {
#[cfg(not(any(
all(target_os = "linux", not(target_env = "musl")),
target_os = "android",
target_os = "hurd"
)))]
use libc::pread as pread64;
#[cfg(any(
all(target_os = "linux", not(target_env = "musl")),
target_os = "android",
target_os = "hurd"
))]
use libc::pread64;

unsafe {
cvt(pread64(
cvt(unsafe {
pread64(
self.as_raw_fd(),
buf.as_mut_ptr() as *mut libc::c_void,
cmp::min(buf.len(), READ_LIMIT),
offset as off64_t,
))
.map(|n| n as usize)
}
)
})
.map(|n| n as usize)
}

pub fn read_buf(&self, mut cursor: BorrowedCursor<'_>) -> io::Result<()> {
// SAFETY: `cursor.as_mut()` starts with `cursor.capacity()` writable bytes
let ret = cvt(unsafe {
libc::read(
self.as_raw_fd(),
cursor.as_mut().as_mut_ptr() as *mut libc::c_void,
cursor.as_mut().as_mut_ptr().cast::<libc::c_void>(),
cmp::min(cursor.capacity(), READ_LIMIT),
)
})?;

// Safety: `ret` bytes were written to the initialized portion of the buffer
// SAFETY: `ret` bytes were written to the initialized portion of the buffer
unsafe {
cursor.advance_unchecked(ret as usize);
}
Ok(())
}

pub fn read_buf_at(&self, mut cursor: BorrowedCursor<'_>, offset: u64) -> io::Result<()> {
// SAFETY: `cursor.as_mut()` starts with `cursor.capacity()` writable bytes
let ret = cvt(unsafe {
pread64(
self.as_raw_fd(),
cursor.as_mut().as_mut_ptr().cast::<libc::c_void>(),
cmp::min(cursor.capacity(), READ_LIMIT),
offset as off64_t,
)
})?;

// SAFETY: `ret` bytes were written to the initialized portion of the buffer
unsafe {
cursor.advance_unchecked(ret as usize);
}
Expand Down Expand Up @@ -369,7 +387,6 @@ impl FileDesc {
)))
}

#[cfg_attr(target_os = "vxworks", allow(unused_unsafe))]
pub fn write_at(&self, buf: &[u8], offset: u64) -> io::Result<usize> {
#[cfg(not(any(
all(target_os = "linux", not(target_env = "musl")),
Expand Down
4 changes: 4 additions & 0 deletions library/std/src/sys/fs/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1422,6 +1422,10 @@ impl File {
self.0.read_buf(cursor)
}

pub fn read_buf_at(&self, cursor: BorrowedCursor<'_>, offset: u64) -> io::Result<()> {
self.0.read_buf_at(cursor, offset)
}

pub fn read_vectored_at(&self, bufs: &mut [IoSliceMut<'_>], offset: u64) -> io::Result<usize> {
self.0.read_vectored_at(bufs, offset)
}
Expand Down
4 changes: 4 additions & 0 deletions library/std/src/sys/fs/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,10 @@ impl File {
self.handle.read_buf(cursor)
}

pub fn read_buf_at(&self, cursor: BorrowedCursor<'_>, offset: u64) -> io::Result<()> {
self.handle.read_buf_at(cursor, offset)
}

pub fn write(&self, buf: &[u8]) -> io::Result<usize> {
self.handle.write(buf)
}
Expand Down
Loading
Loading