Start using with_native_path in std::sys::fs

Author: ChrisDenton

library/std/src/fs.rs

@@ -2362,7 +2362,7 @@ impl AsInner<fs_imp::DirEntry> for DirEntry {
 #[doc(alias = "rm", alias = "unlink", alias = "DeleteFile")]
 #[stable(feature = "rust1", since = "1.0.0")]
 pub fn remove_file<P: AsRef<Path>>(path: P) -> io::Result<()> {
-    fs_imp::unlink(path.as_ref())
+    fs_imp::remove_file(path.as_ref())
 }
 
 /// Given a path, queries the file system to get information about a file,
@@ -2401,7 +2401,7 @@ pub fn remove_file<P: AsRef<Path>>(path: P) -> io::Result<()> {
 #[doc(alias = "stat")]
 #[stable(feature = "rust1", since = "1.0.0")]
 pub fn metadata<P: AsRef<Path>>(path: P) -> io::Result<Metadata> {
-    fs_imp::stat(path.as_ref()).map(Metadata)
+    fs_imp::metadata(path.as_ref()).map(Metadata)
 }
 
 /// Queries the metadata about a file without following symlinks.
@@ -2436,7 +2436,7 @@ pub fn metadata<P: AsRef<Path>>(path: P) -> io::Result<Metadata> {
 #[doc(alias = "lstat")]
 #[stable(feature = "symlink_metadata", since = "1.1.0")]
 pub fn symlink_metadata<P: AsRef<Path>>(path: P) -> io::Result<Metadata> {
-    fs_imp::lstat(path.as_ref()).map(Metadata)
+    fs_imp::symlink_metadata(path.as_ref()).map(Metadata)
 }
 
 /// Renames a file or directory to a new name, replacing the original file if
@@ -2590,7 +2590,7 @@ pub fn copy<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) -> io::Result<u64> {
 #[doc(alias = "CreateHardLink", alias = "linkat")]
 #[stable(feature = "rust1", since = "1.0.0")]
 pub fn hard_link<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) -> io::Result<()> {
-    fs_imp::link(original.as_ref(), link.as_ref())
+    fs_imp::hard_link(original.as_ref(), link.as_ref())
 }
 
 /// Creates a new symbolic link on the filesystem.
@@ -2622,7 +2622,7 @@ pub fn hard_link<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) -> io::Re
             std::os::windows::fs::{symlink_file, symlink_dir}"
 )]
 pub fn soft_link<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) -> io::Result<()> {
-    fs_imp::symlink(original.as_ref(), link.as_ref())
+    fs_imp::soft_link(original.as_ref(), link.as_ref())
 }
 
 /// Reads a symbolic link, returning the file that the link points to.
@@ -2656,7 +2656,7 @@ pub fn soft_link<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) -> io::Re
 /// ```
 #[stable(feature = "rust1", since = "1.0.0")]
 pub fn read_link<P: AsRef<Path>>(path: P) -> io::Result<PathBuf> {
-    fs_imp::readlink(path.as_ref())
+    fs_imp::read_link(path.as_ref())
 }
 
 /// Returns the canonical, absolute form of a path with all intermediate
@@ -2832,7 +2832,7 @@ pub fn create_dir_all<P: AsRef<Path>>(path: P) -> io::Result<()> {
 #[doc(alias = "rmdir", alias = "RemoveDirectory")]
 #[stable(feature = "rust1", since = "1.0.0")]
 pub fn remove_dir<P: AsRef<Path>>(path: P) -> io::Result<()> {
-    fs_imp::rmdir(path.as_ref())
+    fs_imp::remove_dir(path.as_ref())
 }
 
 /// Removes a directory at this path, after removing all its contents. Use
@@ -2959,7 +2959,7 @@ pub fn remove_dir_all<P: AsRef<Path>>(path: P) -> io::Result<()> {
 #[doc(alias = "ls", alias = "opendir", alias = "FindFirstFile", alias = "FindNextFile")]
 #[stable(feature = "rust1", since = "1.0.0")]
 pub fn read_dir<P: AsRef<Path>>(path: P) -> io::Result<ReadDir> {
-    fs_imp::readdir(path.as_ref()).map(ReadDir)
+    fs_imp::read_dir(path.as_ref()).map(ReadDir)
 }
 
 /// Changes the permissions found on a file or a directory.
@@ -2995,7 +2995,7 @@ pub fn read_dir<P: AsRef<Path>>(path: P) -> io::Result<ReadDir> {
 #[doc(alias = "chmod", alias = "SetFileAttributes")]
 #[stable(feature = "set_permissions", since = "1.1.0")]
 pub fn set_permissions<P: AsRef<Path>>(path: P, perm: Permissions) -> io::Result<()> {
-    fs_imp::set_perm(path.as_ref(), perm.0)
+    fs_imp::set_permissions(path.as_ref(), perm.0)
 }
 
 impl DirBuilder {

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

@@ -975,7 +975,7 @@ impl DirEntryExt2 for fs::DirEntry {
 /// ```
 #[stable(feature = "symlink", since = "1.1.0")]
 pub fn symlink<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) -> io::Result<()> {
-    sys::fs::symlink(original.as_ref(), link.as_ref())
+    sys::fs::soft_link(original.as_ref(), link.as_ref())
 }
 
 /// Unix-specific extensions to [`fs::DirBuilder`].

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

@@ -1,28 +1,112 @@
 #![deny(unsafe_op_in_unsafe_fn)]
 
+use crate::io;
+use crate::path::{Path, PathBuf};
+
 pub mod common;
 
 cfg_if::cfg_if! {
     if #[cfg(target_family = "unix")] {
         mod unix;
-        pub use unix::*;
+        use unix as imp;
+        pub use unix::{chown, fchown, lchown, chroot};
+        pub(crate) use unix::debug_assert_fd_is_open;
+        #[cfg(any(target_os = "linux", target_os = "android"))]
+        pub(crate) use unix::CachedFileMetadata;
+        use crate::sys::common::small_c_string::run_path_with_cstr as with_native_path;
     } else if #[cfg(target_os = "windows")] {
         mod windows;
-        pub use windows::*;
+        use windows as imp;
+        pub use windows::{symlink_inner, junction_point};
     } else if #[cfg(target_os = "hermit")] {
         mod hermit;
-        pub use hermit::*;
+        use hermit as imp;
     } else if #[cfg(target_os = "solid_asp3")] {
         mod solid;
-        pub use solid::*;
+        use solid as imp;
     } else if #[cfg(target_os = "uefi")] {
         mod uefi;
-        pub use uefi::*;
+        use uefi as imp;
     } else if #[cfg(target_os = "wasi")] {
         mod wasi;
-        pub use wasi::*;
+        use wasi as imp;
     } else {
         mod unsupported;
-        pub use unsupported::*;
+        use unsupported as imp;
     }
 }
+
+// FIXME: Replace this with platform-specific path conversion functions.
+#[cfg(not(target_family = "unix"))]
+#[inline]
+pub fn with_native_path<T>(path: &Path, f: &dyn Fn(&Path) -> io::Result<T>) -> io::Result<T> {
+    f(path)
+}
+
+pub use imp::{
+    DirBuilder, DirEntry, File, FileAttr, FilePermissions, FileTimes, FileType, OpenOptions,
+    ReadDir,
+};
+
+pub fn read_dir(path: &Path) -> io::Result<ReadDir> {
+    // FIXME: use with_native_path
+    imp::readdir(path)
+}
+
+pub fn remove_file(path: &Path) -> io::Result<()> {
+    with_native_path(path, &imp::unlink)
+}
+
+pub fn rename(old: &Path, new: &Path) -> io::Result<()> {
+    with_native_path(old, &|old| with_native_path(new, &|new| imp::rename(old, new)))
+}
+
+pub fn remove_dir(path: &Path) -> io::Result<()> {
+    with_native_path(path, &imp::rmdir)
+}
+
+pub fn remove_dir_all(path: &Path) -> io::Result<()> {
+    with_native_path(path, &imp::remove_dir_all)
+}
+
+pub fn read_link(path: &Path) -> io::Result<PathBuf> {
+    with_native_path(path, &imp::readlink)
+}
+
+pub fn soft_link(original: &Path, link: &Path) -> io::Result<()> {
+    with_native_path(original, &|original| {
+        with_native_path(link, &|link| imp::symlink(original, link))
+    })
+}
+
+pub fn hard_link(original: &Path, link: &Path) -> io::Result<()> {
+    with_native_path(original, &|original| {
+        with_native_path(link, &|link| imp::link(original, link))
+    })
+}
+
+pub fn metadata(path: &Path) -> io::Result<FileAttr> {
+    with_native_path(path, &imp::stat)
+}
+
+pub fn symlink_metadata(path: &Path) -> io::Result<FileAttr> {
+    with_native_path(path, &imp::lstat)
+}
+
+pub fn set_permissions(path: &Path, perm: FilePermissions) -> io::Result<()> {
+    with_native_path(path, &|path| imp::set_perm(path, perm.clone()))
+}
+
+pub fn canonicalize(path: &Path) -> io::Result<PathBuf> {
+    with_native_path(path, &imp::canonicalize)
+}
+
+pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
+    // FIXME: use with_native_path
+    imp::copy(from, to)
+}
+
+pub fn exists(path: &Path) -> io::Result<bool> {
+    // FIXME: use with_native_path
+    imp::exists(path)
+}

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(
@@ -1653,7 +1653,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"))]
@@ -1826,127 +1826,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());
     }
@@ -2202,8 +2181,7 @@ mod remove_dir_impl {
     use crate::io;
     use crate::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd};
     use crate::os::unix::prelude::{OwnedFd, RawFd};
-    use crate::path::{Path, PathBuf};
-    use crate::sys::common::small_c_string::run_path_with_cstr;
+    use crate::path::PathBuf;
     use crate::sys::{cvt, cvt_r};
     use crate::sys_common::ignore_notfound;
 
@@ -2324,19 +2302,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<()> {
+    pub fn remove_dir_all(p: &CStr) -> io::Result<()> {
         remove_dir_all_modern(p)
     }
 }