Convert readlinkat_raw to use Buffer. (#1362)

Author: sunfishcode

CHANGES.md

@@ -288,10 +288,10 @@ a `[MaybeUninit<u8>]` instead of a `[u8]`.
 [`SendAncillaryBuffer`]: https://docs.rs/rustix/1.0.0/rustix/net/struct.SendAncillaryBuffer.html
 [`RecvAncillaryBuffer`]: https://docs.rs/rustix/1.0.0/rustix/net/struct.RecvAncillaryBuffer.html
 
-[`read`], [`pread`], [`recv`], [`recvfrom`], [`getrandom`], [`epoll::wait`],
-[`kevent`], [`port::getn`], [`getxattr`], [`lgetxattr`], [`fgetxattr`],
-[`listxattr`], [`llistxattr`], and [`flistxattr`] now use the new
-[`Buffer` trait].
+[`read`], [`pread`], [`recv`], [`recvfrom`], [`getrandom`], [`readlinkat_raw`],
+[`epoll::wait`], [`kevent`], [`port::getn`], [`getxattr`], [`lgetxattr`],
+[`fgetxattr`], [`listxattr`], [`llistxattr`], and [`flistxattr`] now use the
+new [`Buffer` trait].
 
 This replaces `read_uninit`, `pread_uninit`, `recv_uninit`, `recvfrom_uninit`,
 and `getrandom_uninit`, as the `Buffer` trait supports reading into
@@ -308,6 +308,7 @@ not cleared first. Consider clearing the vector before calling `epoll::wait`,
 [`recv`]: https://docs.rs/rustix/1.0.0/rustix/net/fn.recv.html
 [`recvfrom`]: https://docs.rs/rustix/1.0.0/rustix/net/fn.recvfrom.html
 [`getrandom`]: https://docs.rs/rustix/1.0.0/rustix/rand/fn.getrandom.html
+[`readlinkat_raw`]: https://docs.rs/rustix/1.0.0/rustix/fs/fn.readlinkat_raw.html
 [`epoll::wait`]: https://docs.rs/rustix/1.0.0/rustix/event/epoll/fn.wait.html
 [`getxattr`]: https://docs.rs/rustix/1.0.0/rustix/fs/fn.getxattr.html
 [`lgetxattr`]: https://docs.rs/rustix/1.0.0/rustix/fs/fn.lgetxattr.html

src/backend/libc/fs/syscalls.rs

@@ -289,19 +289,12 @@ pub(crate) fn readlink(path: &CStr, buf: &mut [u8]) -> io::Result<usize> {
 
 #[cfg(not(target_os = "redox"))]
 #[inline]
-pub(crate) fn readlinkat(
+pub(crate) unsafe fn readlinkat(
     dirfd: BorrowedFd<'_>,
     path: &CStr,
-    buf: &mut [MaybeUninit<u8>],
+    buf: (*mut u8, usize),
 ) -> io::Result<usize> {
-    unsafe {
-        ret_usize(c::readlinkat(
-            borrowed_fd(dirfd),
-            c_str(path),
-            buf.as_mut_ptr().cast(),
-            buf.len(),
-        ) as isize)
-    }
+    ret_usize(c::readlinkat(borrowed_fd(dirfd), c_str(path), buf.0.cast(), buf.1) as isize)
 }
 
 pub(crate) fn mkdir(path: &CStr, mode: Mode) -> io::Result<()> {

src/backend/linux_raw/fs/syscalls.rs

@@ -976,21 +976,18 @@ pub(crate) fn readlink(path: &CStr, buf: &mut [u8]) -> io::Result<usize> {
 }
 
 #[inline]
-pub(crate) fn readlinkat(
+pub(crate) unsafe fn readlinkat(
     dirfd: BorrowedFd<'_>,
     path: &CStr,
-    buf: &mut [MaybeUninit<u8>],
+    buf: (*mut u8, usize),
 ) -> io::Result<usize> {
-    let (buf_addr_mut, buf_len) = slice_mut(buf);
-    unsafe {
-        ret_usize(syscall!(
-            __NR_readlinkat,
-            dirfd,
-            path,
-            buf_addr_mut,
-            buf_len
-        ))
-    }
+    ret_usize(syscall!(
+        __NR_readlinkat,
+        dirfd,
+        path,
+        buf.0,
+        pass_usize(buf.1)
+    ))
 }
 
 #[inline]

src/backend/linux_raw/termios/syscalls.rs

@@ -390,30 +390,36 @@ pub(crate) fn ttyname(fd: BorrowedFd<'_>, buf: &mut [MaybeUninit<u8>]) -> io::Re
     // SAFETY: We just wrote a valid C String.
     let proc_self_fd_path = unsafe { CStr::from_ptr(proc_self_fd_buf.as_ptr().cast()) };
 
-    // Gather the ttyname by reading the "fd" file inside `proc_self_fd`.
-    let r = crate::backend::fs::syscalls::readlinkat(crate::fs::CWD, proc_self_fd_path, buf)?;
+    let ptr = buf.as_mut_ptr();
+    let len = {
+        // Gather the ttyname by reading the "fd" file inside `proc_self_fd`.
+        let (init, uninit) = crate::fs::readlinkat_raw(crate::fs::CWD, proc_self_fd_path, buf)?;
+
+        // If the number of bytes is equal to the buffer length, truncation may
+        // have occurred. This check also ensures that we have enough space for
+        // adding a NUL terminator.
+        if uninit.is_empty() {
+            return Err(io::Errno::RANGE);
+        }
 
-    // If the number of bytes is equal to the buffer length, truncation may
-    // have occurred. This check also ensures that we have enough space for
-    // adding a NUL terminator.
-    if r == buf.len() {
-        return Err(io::Errno::RANGE);
-    }
+        // `readlinkat` returns the number of bytes placed in the buffer.
+        // NUL-terminate the string at that offset.
+        uninit[0].write(b'\0');
 
-    // `readlinkat` returns the number of bytes placed in the buffer.
-    // NUL-terminate the string at that offset.
-    buf[r].write(b'\0');
+        init.len()
+    };
 
     // Check that the path we read refers to the same file as `fd`.
     {
-        // SAFETY: We just wrote the NUL byte above
-        let path = unsafe { CStr::from_ptr(buf.as_ptr().cast()) };
+        // SAFETY: We just wrote the NUL byte above.
+        let path = unsafe { CStr::from_ptr(ptr.cast()) };
 
         let path_stat = crate::backend::fs::syscalls::stat(path)?;
         if path_stat.st_dev != fd_stat.st_dev || path_stat.st_ino != fd_stat.st_ino {
             return Err(io::Errno::NODEV);
         }
     }
 
-    Ok(r)
+    // Return the length, excluding the NUL terminator.
+    Ok(len)
 }

src/fs/at.rs

@@ -6,8 +6,10 @@
 //! [`CWD`]: crate::fs::CWD
 //! [`ABS`]: crate::fs::ABS
 
+#![allow(unsafe_code)]
+
+use crate::buffer::Buffer;
 use crate::fd::OwnedFd;
-use crate::ffi::CStr;
 #[cfg(not(any(target_os = "espidf", target_os = "horizon", target_os = "vita")))]
 use crate::fs::Access;
 #[cfg(not(target_os = "espidf"))]
@@ -24,11 +26,14 @@ use crate::fs::{Dev, FileType};
 use crate::fs::{Gid, Uid};
 use crate::fs::{Mode, OFlags};
 use crate::{backend, io, path};
-use backend::fd::{AsFd, BorrowedFd};
-use core::mem::MaybeUninit;
-use core::slice;
+use backend::fd::AsFd;
 #[cfg(feature = "alloc")]
-use {crate::ffi::CString, crate::path::SMALL_PATH_BUFFER_SIZE, alloc::vec::Vec};
+use {
+    crate::ffi::{CStr, CString},
+    crate::path::SMALL_PATH_BUFFER_SIZE,
+    alloc::vec::Vec,
+    backend::fd::BorrowedFd,
+};
 #[cfg(not(any(target_os = "espidf", target_os = "vita")))]
 use {crate::fs::Timestamps, crate::timespec::Nsecs};
 
@@ -107,8 +112,16 @@ fn _readlinkat(dirfd: BorrowedFd<'_>, path: &CStr, mut buffer: Vec<u8>) -> io::R
     buffer.reserve(SMALL_PATH_BUFFER_SIZE);
 
     loop {
-        let nread =
-            backend::fs::syscalls::readlinkat(dirfd.as_fd(), path, buffer.spare_capacity_mut())?;
+        let buf = buffer.spare_capacity_mut();
+
+        // SAFETY: `readlinkat` behaves.
+        let nread = unsafe {
+            backend::fs::syscalls::readlinkat(
+                dirfd.as_fd(),
+                path,
+                (buf.as_mut_ptr().cast(), buf.len()),
+            )?
+        };
 
         debug_assert!(nread <= buffer.capacity());
         if nread < buffer.capacity() {
@@ -146,14 +159,9 @@ fn _readlinkat(dirfd: BorrowedFd<'_>, path: &CStr, mut buffer: Vec<u8>) -> io::R
 /// `readlinkat(fd, path)`—Reads the contents of a symlink, without
 /// allocating.
 ///
-/// This is the "raw" version which avoids allocating, but which is
-/// significantly trickier to use; most users should use plain [`readlinkat`].
-///
-/// This version writes bytes into the buffer and returns two slices, one
-/// containing the written bytes, and one containing the remaining
-/// uninitialized space. If the number of written bytes is equal to the length
-/// of the buffer, it means the buffer wasn't big enough to hold the full
-/// string, and callers should try again with a bigger buffer.
+/// This is the "raw" version which avoids allocating, but which truncates the
+/// string if it doesn't fit in the provided buffer, and doesn't NUL-terminate
+/// the string.
 ///
 /// # References
 ///  - [POSIX]
@@ -162,27 +170,17 @@ fn _readlinkat(dirfd: BorrowedFd<'_>, path: &CStr, mut buffer: Vec<u8>) -> io::R
 /// [POSIX]: https://pubs.opengroup.org/onlinepubs/9799919799/functions/readlinkat.html
 /// [Linux]: https://man7.org/linux/man-pages/man2/readlinkat.2.html
 #[inline]
-pub fn readlinkat_raw<P: path::Arg, Fd: AsFd>(
+pub fn readlinkat_raw<P: path::Arg, Fd: AsFd, Buf: Buffer<u8>>(
     dirfd: Fd,
     path: P,
-    buf: &mut [MaybeUninit<u8>],
-) -> io::Result<(&mut [u8], &mut [MaybeUninit<u8>])> {
-    path.into_with_c_str(|path| _readlinkat_raw(dirfd.as_fd(), path, buf))
-}
-
-#[allow(unsafe_code)]
-fn _readlinkat_raw<'a>(
-    dirfd: BorrowedFd<'_>,
-    path: &CStr,
-    buf: &'a mut [MaybeUninit<u8>],
-) -> io::Result<(&'a mut [u8], &'a mut [MaybeUninit<u8>])> {
-    let n = backend::fs::syscalls::readlinkat(dirfd.as_fd(), path, buf)?;
-    unsafe {
-        Ok((
-            slice::from_raw_parts_mut(buf.as_mut_ptr().cast::<u8>(), n),
-            &mut buf[n..],
-        ))
-    }
+    mut buf: Buf,
+) -> io::Result<Buf::Output> {
+    // SAFETY: `readlinkat` behaves.
+    let len = path.into_with_c_str(|path| unsafe {
+        backend::fs::syscalls::readlinkat(dirfd.as_fd(), path, buf.parts_mut())
+    })?;
+    // SAFETY: `readlinkat` behaves.
+    unsafe { Ok(buf.assume_init(len)) }
 }
 
 /// `mkdirat(fd, path, mode)`—Creates a directory.