Rollup merge of #138832 - ChrisDenton:with_native_path, r=joboet

Start using `with_native_path` in `std::sys::fs`

Ideally, each platform should use their own native path type internally. This will, for example, allow passing a `CStr` directly to `std::fs::File::open` and therefore avoid the need for allocating a new null-terminated C string.

However, doing that for every function and platform all at once makes for a large PR that is way too prone to breaking. So this PR does some minimal refactoring which should help progress towards that goal. The changes are Unix-only and even then I avoided functions that require more changes so that this PR is just moving things around.

r? joboet

1 files changed, 79 insertions, 100 deletions

diff --git a/library/std/src/sys/fs/unix.rs b/library/std/src/sys/fs/unix.rs
index 60e6559fa6b..87865be0387 100644
--- a/library/std/src/sys/fs/unix.rs
+++ b/library/std/src/sys/fs/unix.rs
@@ -926,7 +926,7 @@ impl DirEntry {
         miri
     ))]
     pub fn metadata(&self) -> io::Result<FileAttr> {
-        lstat(&self.path())
+        run_path_with_cstr(&self.path(), &lstat)
     }
 
     #[cfg(any(
@@ -1657,7 +1657,7 @@ impl fmt::Debug for File {
         fn get_path(fd: c_int) -> Option<PathBuf> {
             let mut p = PathBuf::from("/proc/self/fd");
             p.push(&fd.to_string());
-            readlink(&p).ok()
+            run_path_with_cstr(&p, &readlink).ok()
         }
 
         #[cfg(any(target_vendor = "apple", target_os = "netbsd"))]
@@ -1675,7 +1675,7 @@ impl fmt::Debug for File {
                         // fallback to procfs as last resort
                         let mut p = PathBuf::from("/proc/self/fd");
                         p.push(&fd.to_string());
-                        return readlink(&p).ok();
+                        return run_path_with_cstr(&p, &readlink).ok()
                     } else {
                         return None;
                     }
@@ -1830,127 +1830,106 @@ pub fn readdir(path: &Path) -> io::Result<ReadDir> {
     }
 }
 
-pub fn unlink(p: &Path) -> io::Result<()> {
-    run_path_with_cstr(p, &|p| cvt(unsafe { libc::unlink(p.as_ptr()) }).map(|_| ()))
+pub fn unlink(p: &CStr) -> io::Result<()> {
+    cvt(unsafe { libc::unlink(p.as_ptr()) }).map(|_| ())
 }
 
-pub fn rename(old: &Path, new: &Path) -> io::Result<()> {
-    run_path_with_cstr(old, &|old| {
-        run_path_with_cstr(new, &|new| {
-            cvt(unsafe { libc::rename(old.as_ptr(), new.as_ptr()) }).map(|_| ())
-        })
-    })
+pub fn rename(old: &CStr, new: &CStr) -> io::Result<()> {
+    cvt(unsafe { libc::rename(old.as_ptr(), new.as_ptr()) }).map(|_| ())
 }
 
-pub fn set_perm(p: &Path, perm: FilePermissions) -> io::Result<()> {
-    run_path_with_cstr(p, &|p| cvt_r(|| unsafe { libc::chmod(p.as_ptr(), perm.mode) }).map(|_| ()))
+pub fn set_perm(p: &CStr, perm: FilePermissions) -> io::Result<()> {
+    cvt_r(|| unsafe { libc::chmod(p.as_ptr(), perm.mode) }).map(|_| ())
 }
 
-pub fn rmdir(p: &Path) -> io::Result<()> {
-    run_path_with_cstr(p, &|p| cvt(unsafe { libc::rmdir(p.as_ptr()) }).map(|_| ()))
+pub fn rmdir(p: &CStr) -> io::Result<()> {
+    cvt(unsafe { libc::rmdir(p.as_ptr()) }).map(|_| ())
 }
 
-pub fn readlink(p: &Path) -> io::Result<PathBuf> {
-    run_path_with_cstr(p, &|c_path| {
-        let p = c_path.as_ptr();
-
-        let mut buf = Vec::with_capacity(256);
+pub fn readlink(c_path: &CStr) -> io::Result<PathBuf> {
+    let p = c_path.as_ptr();
 
-        loop {
-            let buf_read =
-                cvt(unsafe { libc::readlink(p, buf.as_mut_ptr() as *mut _, buf.capacity()) })?
-                    as usize;
+    let mut buf = Vec::with_capacity(256);
 
-            unsafe {
-                buf.set_len(buf_read);
-            }
+    loop {
+        let buf_read =
+            cvt(unsafe { libc::readlink(p, buf.as_mut_ptr() as *mut _, buf.capacity()) })? as usize;
 
-            if buf_read != buf.capacity() {
-                buf.shrink_to_fit();
+        unsafe {
+            buf.set_len(buf_read);
+        }
 
-                return Ok(PathBuf::from(OsString::from_vec(buf)));
-            }
+        if buf_read != buf.capacity() {
+            buf.shrink_to_fit();
 
-            // Trigger the internal buffer resizing logic of `Vec` by requiring
-            // more space than the current capacity. The length is guaranteed to be
-            // the same as the capacity due to the if statement above.
-            buf.reserve(1);
+            return Ok(PathBuf::from(OsString::from_vec(buf)));
         }
-    })
+
+        // Trigger the internal buffer resizing logic of `Vec` by requiring
+        // more space than the current capacity. The length is guaranteed to be
+        // the same as the capacity due to the if statement above.
+        buf.reserve(1);
+    }
 }
 
-pub fn symlink(original: &Path, link: &Path) -> io::Result<()> {
-    run_path_with_cstr(original, &|original| {
-        run_path_with_cstr(link, &|link| {
-            cvt(unsafe { libc::symlink(original.as_ptr(), link.as_ptr()) }).map(|_| ())
-        })
-    })
+pub fn symlink(original: &CStr, link: &CStr) -> io::Result<()> {
+    cvt(unsafe { libc::symlink(original.as_ptr(), link.as_ptr()) }).map(|_| ())
 }
 
-pub fn link(original: &Path, link: &Path) -> io::Result<()> {
-    run_path_with_cstr(original, &|original| {
-        run_path_with_cstr(link, &|link| {
-            cfg_if::cfg_if! {
-                if #[cfg(any(target_os = "vxworks", target_os = "redox", target_os = "android", target_os = "espidf", target_os = "horizon", target_os = "vita", target_env = "nto70"))] {
-                    // VxWorks, Redox and ESP-IDF lack `linkat`, so use `link` instead. POSIX leaves
-                    // it implementation-defined whether `link` follows symlinks, so rely on the
-                    // `symlink_hard_link` test in library/std/src/fs/tests.rs to check the behavior.
-                    // Android has `linkat` on newer versions, but we happen to know `link`
-                    // always has the correct behavior, so it's here as well.
-                    cvt(unsafe { libc::link(original.as_ptr(), link.as_ptr()) })?;
-                } else {
-                    // Where we can, use `linkat` instead of `link`; see the comment above
-                    // this one for details on why.
-                    cvt(unsafe { libc::linkat(libc::AT_FDCWD, original.as_ptr(), libc::AT_FDCWD, link.as_ptr(), 0) })?;
-                }
-            }
-            Ok(())
-        })
-    })
+pub fn link(original: &CStr, link: &CStr) -> io::Result<()> {
+    cfg_if::cfg_if! {
+        if #[cfg(any(target_os = "vxworks", target_os = "redox", target_os = "android", target_os = "espidf", target_os = "horizon", target_os = "vita", target_env = "nto70"))] {
+            // VxWorks, Redox and ESP-IDF lack `linkat`, so use `link` instead. POSIX leaves
+            // it implementation-defined whether `link` follows symlinks, so rely on the
+            // `symlink_hard_link` test in library/std/src/fs/tests.rs to check the behavior.
+            // Android has `linkat` on newer versions, but we happen to know `link`
+            // always has the correct behavior, so it's here as well.
+            cvt(unsafe { libc::link(original.as_ptr(), link.as_ptr()) })?;
+        } else {
+            // Where we can, use `linkat` instead of `link`; see the comment above
+            // this one for details on why.
+            cvt(unsafe { libc::linkat(libc::AT_FDCWD, original.as_ptr(), libc::AT_FDCWD, link.as_ptr(), 0) })?;
+        }
+    }
+    Ok(())
 }
 
-pub fn stat(p: &Path) -> io::Result<FileAttr> {
-    run_path_with_cstr(p, &|p| {
-        cfg_has_statx! {
-            if let Some(ret) = unsafe { try_statx(
-                libc::AT_FDCWD,
-                p.as_ptr(),
-                libc::AT_STATX_SYNC_AS_STAT,
-                libc::STATX_BASIC_STATS | libc::STATX_BTIME,
-            ) } {
-                return ret;
-            }
+pub fn stat(p: &CStr) -> io::Result<FileAttr> {
+    cfg_has_statx! {
+        if let Some(ret) = unsafe { try_statx(
+            libc::AT_FDCWD,
+            p.as_ptr(),
+            libc::AT_STATX_SYNC_AS_STAT,
+            libc::STATX_BASIC_STATS | libc::STATX_BTIME,
+        ) } {
+            return ret;
         }
+    }
 
-        let mut stat: stat64 = unsafe { mem::zeroed() };
-        cvt(unsafe { stat64(p.as_ptr(), &mut stat) })?;
-        Ok(FileAttr::from_stat64(stat))
-    })
+    let mut stat: stat64 = unsafe { mem::zeroed() };
+    cvt(unsafe { stat64(p.as_ptr(), &mut stat) })?;
+    Ok(FileAttr::from_stat64(stat))
 }
 
-pub fn lstat(p: &Path) -> io::Result<FileAttr> {
-    run_path_with_cstr(p, &|p| {
-        cfg_has_statx! {
-            if let Some(ret) = unsafe { try_statx(
-                libc::AT_FDCWD,
-                p.as_ptr(),
-                libc::AT_SYMLINK_NOFOLLOW | libc::AT_STATX_SYNC_AS_STAT,
-                libc::STATX_BASIC_STATS | libc::STATX_BTIME,
-            ) } {
-                return ret;
-            }
+pub fn lstat(p: &CStr) -> io::Result<FileAttr> {
+    cfg_has_statx! {
+        if let Some(ret) = unsafe { try_statx(
+            libc::AT_FDCWD,
+            p.as_ptr(),
+            libc::AT_SYMLINK_NOFOLLOW | libc::AT_STATX_SYNC_AS_STAT,
+            libc::STATX_BASIC_STATS | libc::STATX_BTIME,
+        ) } {
+            return ret;
         }
+    }
 
-        let mut stat: stat64 = unsafe { mem::zeroed() };
-        cvt(unsafe { lstat64(p.as_ptr(), &mut stat) })?;
-        Ok(FileAttr::from_stat64(stat))
-    })
+    let mut stat: stat64 = unsafe { mem::zeroed() };
+    cvt(unsafe { lstat64(p.as_ptr(), &mut stat) })?;
+    Ok(FileAttr::from_stat64(stat))
 }
 
-pub fn canonicalize(p: &Path) -> io::Result<PathBuf> {
-    let r = run_path_with_cstr(p, &|path| unsafe {
-        Ok(libc::realpath(path.as_ptr(), ptr::null_mut()))
-    })?;
+pub fn canonicalize(path: &CStr) -> io::Result<PathBuf> {
+    let r = unsafe { libc::realpath(path.as_ptr(), ptr::null_mut()) };
     if r.is_null() {
         return Err(io::Error::last_os_error());
     }
@@ -2328,19 +2307,19 @@ mod remove_dir_impl {
         Ok(())
     }
 
-    fn remove_dir_all_modern(p: &Path) -> io::Result<()> {
+    fn remove_dir_all_modern(p: &CStr) -> io::Result<()> {
         // We cannot just call remove_dir_all_recursive() here because that would not delete a passed
         // symlink. No need to worry about races, because remove_dir_all_recursive() does not recurse
         // into symlinks.
         let attr = lstat(p)?;
         if attr.file_type().is_symlink() {
-            crate::fs::remove_file(p)
+            super::unlink(p)
         } else {
-            run_path_with_cstr(p, &|p| remove_dir_all_recursive(None, &p))
+            remove_dir_all_recursive(None, &p)
         }
     }
 
     pub fn remove_dir_all(p: &Path) -> io::Result<()> {
-        remove_dir_all_modern(p)
+        run_path_with_cstr(p, &remove_dir_all_modern)
     }
 }