Add `read_buf` equivalents for positioned reads #140459

niklasf · 2025-04-29T13:54:42Z

Adds the following items under the ~~read_buf (#78485)~~ read_buf_at (#140771) feature:

std::os::unix::fs::FileExt::read_buf_at
std::os::unix::fs::FileExt::read_buf_exact_at
std::os::windows::fs::FileExt::seek_read_buf

try-job: x86_64-msvc*
try-job: test-various*
try-job: dist-various*

rustbot · 2025-04-29T13:54:54Z

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

tgross35 · 2025-05-01T00:54:44Z

Have these changes been discussed with libs-api at all? Usually changes to unstable API need to be proposed at https://github.com/rust-lang/libs-team/ with the ACP issue template.

This needs some tests as well.

tgross35 · 2025-05-01T01:42:49Z

For the above,
@rustbot author

rustbot · 2025-05-01T01:42:54Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

Kivooeo · 2025-05-08T04:28:04Z

For consistency and clarity, could we include safety comments on each unsafe block? Even brief notes are helpful to understand the assumptions being made

niklasf · 2025-05-12T23:13:31Z

Thank you both.

Opened an APC that was accepted, but items to be added under a new feature instead of read_buf. Moved to read_buf_at with new tracking issue Tracking Issue for read_buf_at #140771.
Added safety comments also for FFI calls.
Added tests.

@rustbot ready

library/std/src/sys/fd/unix.rs

library/std/src/os/unix/fs.rs

tgross35 · 2025-05-28T22:37:50Z

library/std/src/sys/fd/unix.rs

+#[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;


Since this is getting moved anyway, mind simplifying this with cfg_select!?

Sure. Did I pick the right place for #![feature(cfg_select)]?

Thanks! I would put it at the top with the off64_t import, some of that config might be sharable.

tgross35 · 2025-05-28T22:38:03Z

@bors try

Add `read_buf` equivalents for positioned reads Adds the following items under the ~~`read_buf` (#78485)~~ `read_buf_at` (#140771) feature: - `std::os::unix::fs::FileExt::read_buf_at` - `std::os::unix::fs::FileExt::read_buf_exact_at` - `std::os::windows::fs::FileExt::seek_read_buf` try-job: x86_64-msvc-1 try-job: x86_64-msvc-2

bors · 2025-05-28T23:08:34Z

⌛ Trying commit 85e1da1 with merge f769285...

bors · 2025-05-29T01:03:19Z

💔 Test failed - checks-actions

Adds the following items under the `read_buf_at` feature: - `std::os::unix::fs::FileExt::read_buf_at` - `std::os::unix::fs::FileExt::read_buf_exact_at` - `std::os::windows::fs::FileExt::seek_read_buf`

niklasf · 2025-05-29T18:20:14Z

Ready for the next round of review.

The failed test on Windows was an assertion that I had (blindly) added about the seek position after seek_read_buf() with an empty buffer. Turns out it's a noop, but neither the existing seek_read() nor the current underlying Windows API guarantees it (afaict), so I just removed the assertion.

@rustbot ready

tgross35 · 2025-06-10T04:12:12Z

library/std/src/sys/pal/windows/handle.rs

+        // SAFETY: `read` bytes were written to the initialized portion of the buffer
+        unsafe {
+            cursor.advance_unchecked(read);
+        }


Do we need the same handling for ErrorKind::BrokenPipe as with read_buf?

If so, these functions are pretty identical. Maybe we should only have read_buf_at(self, cursor, offset: Option<u64>), and instead of read_buf call read_buf_at(..., None)?

As always, cc @ChrisDenton for Windows changes

tgross35 · 2025-06-10T04:15:03Z

library/std/src/os/unix/fs.rs

+    /// 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`.


Small style nit, docs can wrap at 100 chars

tgross35 · 2025-06-10T04:16:39Z

library/std/src/fs/tests.rs

@@ -490,6 +490,47 @@ fn file_test_io_read_write_at() {
    check!(fs::remove_file(&filename));
 }

+#[test]
+#[cfg(unix)]
+fn test_read_buf_at() {


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?

tgross35 · 2025-06-10T04:17:56Z

Running another try, including a few other unix platforms

@bors2 try

rust-bors · 2025-06-10T04:18:00Z

⌛ Trying commit e8a35d4 with merge 2e522c2…

To cancel the try build, run the command @bors2 try cancel.

Add `read_buf` equivalents for positioned reads Adds the following items under the ~~`read_buf` (#78485)~~ `read_buf_at` (#140771) feature: - `std::os::unix::fs::FileExt::read_buf_at` - `std::os::unix::fs::FileExt::read_buf_exact_at` - `std::os::windows::fs::FileExt::seek_read_buf` try-job: `x86_64-msvc*` try-job: `test-various*` try-job: `dist-various*`

tgross35 · 2025-06-10T04:20:56Z

library/std/src/sys/fd/unix.rs

+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;
+    }
+}


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.

tgross35 · 2025-06-10T05:07:11Z

@rustbot author

rust-bors · 2025-06-10T06:47:27Z

☀️ Try build successful (CI)
Build commit: 2e522c2 (2e522c2e8590b7b074d7a35dea51085165d83c67)

rustbot assigned tgross35 Apr 29, 2025

rustbot added O-unix Operating system: Unix-like O-windows Operating system: Windows S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 29, 2025

This comment has been minimized.

Sign in to view

niklasf closed this Apr 29, 2025

niklasf force-pushed the feature/read-buf-at branch from 74e016c to 2ccb45f Compare April 29, 2025 14:07

niklasf reopened this Apr 29, 2025

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 1, 2025

niklasf mentioned this pull request May 7, 2025

Tracking Issue for read_buf_at #140771

Open

4 tasks

niklasf force-pushed the feature/read-buf-at branch from 2ccb45f to 09ac65a Compare May 12, 2025 21:45

This comment has been minimized.

Sign in to view

niklasf force-pushed the feature/read-buf-at branch from 09ac65a to a22d2b4 Compare May 12, 2025 22:06

This comment has been minimized.

Sign in to view

niklasf force-pushed the feature/read-buf-at branch from a22d2b4 to 59cb58e Compare May 12, 2025 22:30

This comment has been minimized.

Sign in to view

niklasf force-pushed the feature/read-buf-at branch from 59cb58e to 85e1da1 Compare May 12, 2025 22:41

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 12, 2025

tgross35 reviewed May 28, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 29, 2025

niklasf force-pushed the feature/read-buf-at branch from 85e1da1 to 05dcef0 Compare May 29, 2025 18:08

This comment has been minimized.

Sign in to view

Add read_buf equivalents for positioned reads

e8a35d4

Adds the following items under the `read_buf_at` feature: - `std::os::unix::fs::FileExt::read_buf_at` - `std::os::unix::fs::FileExt::read_buf_exact_at` - `std::os::windows::fs::FileExt::seek_read_buf`

niklasf force-pushed the feature/read-buf-at branch from 05dcef0 to e8a35d4 Compare May 29, 2025 18:09

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 29, 2025

tgross35 reviewed Jun 10, 2025

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 10, 2025

Add read_buf equivalents for positioned reads #140459

Are you sure you want to change the base?

Add read_buf equivalents for positioned reads #140459

Uh oh!

Conversation

niklasf commented Apr 29, 2025 • edited by tgross35 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Apr 29, 2025

Uh oh!

This comment has been minimized.

tgross35 commented May 1, 2025

Uh oh!

tgross35 commented May 1, 2025

Uh oh!

rustbot commented May 1, 2025

Uh oh!

Kivooeo commented May 8, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

niklasf commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgross35 commented May 28, 2025

Uh oh!

bors commented May 28, 2025

Uh oh!

This comment has been minimized.

bors commented May 29, 2025

Uh oh!

This comment has been minimized.

niklasf commented May 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgross35 commented Jun 10, 2025

Uh oh!

rust-bors bot commented Jun 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgross35 commented Jun 10, 2025

Uh oh!

rust-bors bot commented Jun 10, 2025

Uh oh!

Uh oh!

Add `read_buf` equivalents for positioned reads #140459

Add `read_buf` equivalents for positioned reads #140459

niklasf commented Apr 29, 2025 •

edited by tgross35

Loading

niklasf commented May 12, 2025 •

edited

Loading