Store the path in io::Error without extra allocations.

We need to allocate for doing a syscall on a path anyway, so this just
keeps that allocation around in the io::Error if the syscall fails.

Author: m-ou-se

library/std/src/io/error.rs

@@ -1,7 +1,9 @@
+#![allow(unreachable_patterns)] // For empty OsWithPath on some platforms.
+
 #[cfg(test)]
 mod tests;
 
-use crate::convert::From;
+use crate::convert::{From, TryInto};
 use crate::error;
 use crate::fmt;
 use crate::result;
@@ -68,9 +70,19 @@ impl fmt::Debug for Error {
 
 enum Repr {
     Os(i32),
+
+    // i16 to make sure the whole Repr remains two words in size.
+    // All known error codes fit in a i16.
+    // If for some reason it wouldn't fit, we just don't add the path and use
+    // Os(code) instead.
+    #[allow(dead_code)]
+    OsWithPath(i16, sys::fs::OsPathBuf),
+
     Simple(ErrorKind),
+
     // &str is a fat pointer, but &&str is a thin pointer.
     SimpleMessage(ErrorKind, &'static &'static str),
+
     Custom(Box<Custom>),
 }
 
@@ -337,6 +349,16 @@ impl Error {
         Error { repr: Repr::Os(code) }
     }
 
+    #[allow(dead_code)]
+    pub(crate) fn with_path(self, path: sys::fs::OsPathBuf) -> Self {
+        if let Repr::Os(code) = self.repr {
+            if let Ok(code) = code.try_into() {
+                return Self { repr: Repr::OsWithPath(code, path) };
+            }
+        }
+        self
+    }
+
     /// Returns the OS error that this error represents (if any).
     ///
     /// If this [`Error`] was constructed via [`last_os_error`] or
@@ -371,6 +393,7 @@ impl Error {
     pub fn raw_os_error(&self) -> Option<i32> {
         match self.repr {
             Repr::Os(i) => Some(i),
+            Repr::OsWithPath(i, _) => Some(i.into()),
             Repr::Custom(..) => None,
             Repr::Simple(..) => None,
             Repr::SimpleMessage(..) => None,
@@ -409,6 +432,7 @@ impl Error {
     pub fn get_ref(&self) -> Option<&(dyn error::Error + Send + Sync + 'static)> {
         match self.repr {
             Repr::Os(..) => None,
+            Repr::OsWithPath(..) => None,
             Repr::Simple(..) => None,
             Repr::SimpleMessage(..) => None,
             Repr::Custom(ref c) => Some(&*c.error),
@@ -482,6 +506,7 @@ impl Error {
     pub fn get_mut(&mut self) -> Option<&mut (dyn error::Error + Send + Sync + 'static)> {
         match self.repr {
             Repr::Os(..) => None,
+            Repr::OsWithPath(..) => None,
             Repr::Simple(..) => None,
             Repr::SimpleMessage(..) => None,
             Repr::Custom(ref mut c) => Some(&mut *c.error),
@@ -520,6 +545,7 @@ impl Error {
     pub fn into_inner(self) -> Option<Box<dyn error::Error + Send + Sync>> {
         match self.repr {
             Repr::Os(..) => None,
+            Repr::OsWithPath(..) => None,
             Repr::Simple(..) => None,
             Repr::SimpleMessage(..) => None,
             Repr::Custom(c) => Some(c.error),
@@ -549,6 +575,7 @@ impl Error {
     pub fn kind(&self) -> ErrorKind {
         match self.repr {
             Repr::Os(code) => sys::decode_error_kind(code),
+            Repr::OsWithPath(code, _) => sys::decode_error_kind(code.into()),
             Repr::Custom(ref c) => c.kind,
             Repr::Simple(kind) => kind,
             Repr::SimpleMessage(kind, _) => kind,
@@ -565,6 +592,13 @@ impl fmt::Debug for Repr {
                 .field("kind", &sys::decode_error_kind(code))
                 .field("message", &sys::os::error_string(code))
                 .finish(),
+            Repr::OsWithPath(code, ref path) => fmt
+                .debug_struct("Os")
+                .field("code", &code)
+                .field("kind", &sys::decode_error_kind(code.into()))
+                .field("message", &sys::os::error_string(code.into()))
+                .field("path", path)
+                .finish(),
             Repr::Custom(ref c) => fmt::Debug::fmt(&c, fmt),
             Repr::Simple(kind) => fmt.debug_tuple("Kind").field(&kind).finish(),
             Repr::SimpleMessage(kind, &message) => {
@@ -582,6 +616,9 @@ impl fmt::Display for Error {
                 let detail = sys::os::error_string(code);
                 write!(fmt, "{} (os error {})", detail, code)
             }
+            Repr::OsWithPath(code, _) => {
+                fmt::Display::fmt(&Self::from_raw_os_error(code.into()), fmt)
+            }
             Repr::Custom(ref c) => c.error.fmt(fmt),
             Repr::Simple(kind) => write!(fmt, "{}", kind.as_str()),
             Repr::SimpleMessage(_, &msg) => msg.fmt(fmt),
@@ -594,7 +631,7 @@ impl error::Error for Error {
     #[allow(deprecated, deprecated_in_future)]
     fn description(&self) -> &str {
         match self.repr {
-            Repr::Os(..) | Repr::Simple(..) => self.kind().as_str(),
+            Repr::Os(..) | Repr::OsWithPath(..) | Repr::Simple(..) => self.kind().as_str(),
             Repr::SimpleMessage(_, &msg) => msg,
             Repr::Custom(ref c) => c.error.description(),
         }
@@ -604,6 +641,7 @@ impl error::Error for Error {
     fn cause(&self) -> Option<&dyn error::Error> {
         match self.repr {
             Repr::Os(..) => None,
+            Repr::OsWithPath(..) => None,
             Repr::Simple(..) => None,
             Repr::SimpleMessage(..) => None,
             Repr::Custom(ref c) => c.error.cause(),
@@ -613,6 +651,7 @@ impl error::Error for Error {
     fn source(&self) -> Option<&(dyn error::Error + 'static)> {
         match self.repr {
             Repr::Os(..) => None,
+            Repr::OsWithPath(..) => None,
             Repr::Simple(..) => None,
             Repr::SimpleMessage(..) => None,
             Repr::Custom(ref c) => c.error.source(),

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

@@ -1,3 +1,5 @@
+mod ospathbuf;
+
 use crate::os::unix::prelude::*;
 
 use crate::ffi::{CStr, CString, OsStr, OsString};
@@ -50,6 +52,8 @@ use libc::{
 
 pub use crate::sys_common::fs::{remove_dir_all, try_exists};
 
+pub use ospathbuf::OsPathBuf;
+
 pub struct File(FileDesc);
 
 // FIXME: This should be available on Linux with all `target_env`.
@@ -729,8 +733,8 @@ impl OpenOptions {
 
 impl File {
     pub fn open(path: &Path, opts: &OpenOptions) -> io::Result<File> {
-        let path = cstr(path)?;
-        File::open_c(&path, opts)
+        let path = OsPathBuf::new(path)?;
+        File::open_c(&path, opts).map_err(|e| e.with_path(path))
     }
 
     pub fn open_c(path: &CStr, opts: &OpenOptions) -> io::Result<File> {
@@ -898,8 +902,8 @@ impl DirBuilder {
     }
 
     pub fn mkdir(&self, p: &Path) -> io::Result<()> {
-        let p = cstr(p)?;
-        cvt(unsafe { libc::mkdir(p.as_ptr(), self.mode) })?;
+        let p = OsPathBuf::new(p)?;
+        cvt(unsafe { libc::mkdir(p.as_ptr(), self.mode) }).map_err(|e| e.with_path(p))?;
         Ok(())
     }
 
@@ -908,10 +912,6 @@ impl DirBuilder {
     }
 }
 
-fn cstr(path: &Path) -> io::Result<CString> {
-    Ok(CString::new(path.as_os_str().as_bytes())?)
-}
-
 impl FromInner<c_int> for File {
     fn from_inner(fd: c_int) -> File {
         File(FileDesc::new(fd))
@@ -998,11 +998,11 @@ impl fmt::Debug for File {
 
 pub fn readdir(p: &Path) -> io::Result<ReadDir> {
     let root = p.to_path_buf();
-    let p = cstr(p)?;
+    let p = OsPathBuf::new(p)?;
     unsafe {
         let ptr = libc::opendir(p.as_ptr());
         if ptr.is_null() {
-            Err(Error::last_os_error())
+            Err(Error::last_os_error().with_path(p))
         } else {
             let inner = InnerReadDir { dirp: Dir(ptr), root };
             Ok(ReadDir {
@@ -1020,39 +1020,42 @@ pub fn readdir(p: &Path) -> io::Result<ReadDir> {
 }
 
 pub fn unlink(p: &Path) -> io::Result<()> {
-    let p = cstr(p)?;
-    cvt(unsafe { libc::unlink(p.as_ptr()) })?;
+    let p = OsPathBuf::new(p)?;
+    cvt(unsafe { libc::unlink(p.as_ptr()) }).map_err(|e| e.with_path(p))?;
     Ok(())
 }
 
 pub fn rename(old: &Path, new: &Path) -> io::Result<()> {
-    let old = cstr(old)?;
-    let new = cstr(new)?;
+    let old = OsPathBuf::new(old)?;
+    let new = OsPathBuf::new(new)?;
     cvt(unsafe { libc::rename(old.as_ptr(), new.as_ptr()) })?;
     Ok(())
 }
 
 pub fn set_perm(p: &Path, perm: FilePermissions) -> io::Result<()> {
-    let p = cstr(p)?;
-    cvt_r(|| unsafe { libc::chmod(p.as_ptr(), perm.mode) })?;
+    let p = OsPathBuf::new(p)?;
+    cvt_r(|| unsafe { libc::chmod(p.as_ptr(), perm.mode) }).map_err(|e| e.with_path(p))?;
     Ok(())
 }
 
 pub fn rmdir(p: &Path) -> io::Result<()> {
-    let p = cstr(p)?;
-    cvt(unsafe { libc::rmdir(p.as_ptr()) })?;
+    let p = OsPathBuf::new(p)?;
+    cvt(unsafe { libc::rmdir(p.as_ptr()) }).map_err(|e| e.with_path(p))?;
     Ok(())
 }
 
 pub fn readlink(p: &Path) -> io::Result<PathBuf> {
-    let c_path = cstr(p)?;
-    let p = c_path.as_ptr();
+    let p = OsPathBuf::new(p)?;
 
     let mut buf = Vec::with_capacity(256);
 
     loop {
-        let buf_read =
-            cvt(unsafe { libc::readlink(p, buf.as_mut_ptr() as *mut _, buf.capacity()) })? as usize;
+        let buf_read = match cvt(unsafe {
+            libc::readlink(p.as_ptr(), buf.as_mut_ptr() as *mut _, buf.capacity())
+        }) {
+            Ok(r) => r as usize,
+            Err(e) => return Err(e.with_path(p)),
+        };
 
         unsafe {
             buf.set_len(buf_read);
@@ -1072,15 +1075,15 @@ pub fn readlink(p: &Path) -> io::Result<PathBuf> {
 }
 
 pub fn symlink(original: &Path, link: &Path) -> io::Result<()> {
-    let original = cstr(original)?;
-    let link = cstr(link)?;
+    let original = OsPathBuf::new(original)?;
+    let link = OsPathBuf::new(link)?;
     cvt(unsafe { libc::symlink(original.as_ptr(), link.as_ptr()) })?;
     Ok(())
 }
 
 pub fn link(original: &Path, link: &Path) -> io::Result<()> {
-    let original = cstr(original)?;
-    let link = cstr(link)?;
+    let original = OsPathBuf::new(original)?;
+    let link = OsPathBuf::new(link)?;
     cfg_if::cfg_if! {
         if #[cfg(any(target_os = "vxworks", target_os = "redox", target_os = "android"))] {
             // VxWorks, Redox, and old versions of Android lack `linkat`, so use
@@ -1099,7 +1102,7 @@ pub fn link(original: &Path, link: &Path) -> io::Result<()> {
 }
 
 pub fn stat(p: &Path) -> io::Result<FileAttr> {
-    let p = cstr(p)?;
+    let p = OsPathBuf::new(p)?;
 
     cfg_has_statx! {
         if let Some(ret) = unsafe { try_statx(
@@ -1108,17 +1111,17 @@ pub fn stat(p: &Path) -> io::Result<FileAttr> {
             libc::AT_STATX_SYNC_AS_STAT,
             libc::STATX_ALL,
         ) } {
-            return ret;
+            return ret.map_err(|e| e.with_path(p));
         }
     }
 
     let mut stat: stat64 = unsafe { mem::zeroed() };
-    cvt(unsafe { stat64(p.as_ptr(), &mut stat) })?;
+    cvt(unsafe { stat64(p.as_ptr(), &mut stat) }).map_err(|e| e.with_path(p))?;
     Ok(FileAttr::from_stat64(stat))
 }
 
 pub fn lstat(p: &Path) -> io::Result<FileAttr> {
-    let p = cstr(p)?;
+    let p = OsPathBuf::new(p)?;
 
     cfg_has_statx! {
         if let Some(ret) = unsafe { try_statx(
@@ -1127,12 +1130,12 @@ pub fn lstat(p: &Path) -> io::Result<FileAttr> {
             libc::AT_SYMLINK_NOFOLLOW | libc::AT_STATX_SYNC_AS_STAT,
             libc::STATX_ALL,
         ) } {
-            return ret;
+            return ret.map_err(|e| e.with_path(p));
         }
     }
 
     let mut stat: stat64 = unsafe { mem::zeroed() };
-    cvt(unsafe { lstat64(p.as_ptr(), &mut stat) })?;
+    cvt(unsafe { lstat64(p.as_ptr(), &mut stat) }).map_err(|e| e.with_path(p))?;
     Ok(FileAttr::from_stat64(stat))
 }
 
@@ -1284,7 +1287,7 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
     // Opportunistically attempt to create a copy-on-write clone of `from`
     // using `fclonefileat`.
     if HAS_FCLONEFILEAT.load(Ordering::Relaxed) {
-        let to = cstr(to)?;
+        let to = OsPathBuf::new(to)?;
         let clonefile_result =
             cvt(unsafe { fclonefileat(reader.as_raw_fd(), libc::AT_FDCWD, to.as_ptr(), 0) });
         match clonefile_result {
@@ -1331,7 +1334,7 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
 
 #[cfg(not(any(target_os = "fuchsia", target_os = "vxworks")))]
 pub fn chroot(dir: &Path) -> io::Result<()> {
-    let dir = cstr(dir)?;
-    cvt(unsafe { libc::chroot(dir.as_ptr()) })?;
+    let dir = OsPathBuf::new(dir)?;
+    cvt(unsafe { libc::chroot(dir.as_ptr()) }).map_err(|e| e.with_path(dir))?;
     Ok(())
 }