Rollup merge of #139683 - ChrisDenton:windows-with-native, r=tgross35,joboet

Use `with_native_path` for Windows

Ideally, each platform should use their own native path type internally. This will, for example, allow passing a UTF-16 string directly to `std::fs::File::open` and therefore avoid the need for allocating a new null-terminated wide 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 just does some of the Windows parts.

As with the previous Unix PR (#138832) this is intended to be merely a refactoring so I've avoided anything that may require more substantial changes.
This commit is contained in:
Chris Denton 2025-04-13 11:48:18 +00:00 committed by GitHub
commit 8a6d6f5ae5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 86 additions and 47 deletions

View file

@ -20,6 +20,7 @@ cfg_if::cfg_if! {
mod windows;
use windows as imp;
pub use windows::{symlink_inner, junction_point};
use crate::sys::path::with_native_path;
} else if #[cfg(target_os = "hermit")] {
mod hermit;
use hermit as imp;
@ -39,7 +40,7 @@ cfg_if::cfg_if! {
}
// FIXME: Replace this with platform-specific path conversion functions.
#[cfg(not(target_family = "unix"))]
#[cfg(not(any(target_family = "unix", target_os = "windows")))]
#[inline]
pub fn with_native_path<T>(path: &Path, f: &dyn Fn(&Path) -> io::Result<T>) -> io::Result<T> {
f(path)
@ -51,7 +52,7 @@ pub use imp::{
};
pub fn read_dir(path: &Path) -> io::Result<ReadDir> {
// FIXME: use with_native_path
// FIXME: use with_native_path on all platforms
imp::readdir(path)
}
@ -68,8 +69,11 @@ pub fn remove_dir(path: &Path) -> io::Result<()> {
}
pub fn remove_dir_all(path: &Path) -> io::Result<()> {
// FIXME: use with_native_path
imp::remove_dir_all(path)
// FIXME: use with_native_path on all platforms
#[cfg(not(windows))]
return imp::remove_dir_all(path);
#[cfg(windows)]
with_native_path(path, &imp::remove_dir_all)
}
pub fn read_link(path: &Path) -> io::Result<PathBuf> {
@ -77,6 +81,10 @@ pub fn read_link(path: &Path) -> io::Result<PathBuf> {
}
pub fn symlink(original: &Path, link: &Path) -> io::Result<()> {
// FIXME: use with_native_path on all platforms
#[cfg(windows)]
return imp::symlink(original, link);
#[cfg(not(windows))]
with_native_path(original, &|original| {
with_native_path(link, &|link| imp::symlink(original, link))
})
@ -105,11 +113,17 @@ pub fn canonicalize(path: &Path) -> io::Result<PathBuf> {
}
pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
// FIXME: use with_native_path
imp::copy(from, to)
// FIXME: use with_native_path on all platforms
#[cfg(not(windows))]
return imp::copy(from, to);
#[cfg(windows)]
with_native_path(from, &|from| with_native_path(to, &|to| imp::copy(from, to)))
}
pub fn exists(path: &Path) -> io::Result<bool> {
// FIXME: use with_native_path
imp::exists(path)
// FIXME: use with_native_path on all platforms
#[cfg(not(windows))]
return imp::exists(path);
#[cfg(windows)]
with_native_path(path, &imp::exists)
}

View file

@ -12,7 +12,7 @@ use crate::sync::Arc;
use crate::sys::handle::Handle;
use crate::sys::pal::api::{self, WinError, set_file_information_by_handle};
use crate::sys::pal::{IoResult, fill_utf16_buf, to_u16s, truncate_utf16_at_nul};
use crate::sys::path::maybe_verbatim;
use crate::sys::path::{WCStr, maybe_verbatim};
use crate::sys::time::SystemTime;
use crate::sys::{Align8, c, cvt};
use crate::sys_common::{AsInner, FromInner, IntoInner};
@ -298,10 +298,12 @@ impl OpenOptions {
impl File {
pub fn open(path: &Path, opts: &OpenOptions) -> io::Result<File> {
let path = maybe_verbatim(path)?;
// SAFETY: maybe_verbatim returns null-terminated strings
let path = unsafe { WCStr::from_wchars_with_null_unchecked(&path) };
Self::open_native(&path, opts)
}
fn open_native(path: &[u16], opts: &OpenOptions) -> io::Result<File> {
fn open_native(path: &WCStr, opts: &OpenOptions) -> io::Result<File> {
let creation = opts.get_creation_mode()?;
let handle = unsafe {
c::CreateFileW(
@ -1212,9 +1214,8 @@ pub fn readdir(p: &Path) -> io::Result<ReadDir> {
}
}
pub fn unlink(p: &Path) -> io::Result<()> {
let p_u16s = maybe_verbatim(p)?;
if unsafe { c::DeleteFileW(p_u16s.as_ptr()) } == 0 {
pub fn unlink(path: &WCStr) -> io::Result<()> {
if unsafe { c::DeleteFileW(path.as_ptr()) } == 0 {
let err = api::get_last_error();
// if `DeleteFileW` fails with ERROR_ACCESS_DENIED then try to remove
// the file while ignoring the readonly attribute.
@ -1223,7 +1224,7 @@ pub fn unlink(p: &Path) -> io::Result<()> {
let mut opts = OpenOptions::new();
opts.access_mode(c::DELETE);
opts.custom_flags(c::FILE_FLAG_OPEN_REPARSE_POINT);
if let Ok(f) = File::open_native(&p_u16s, &opts) {
if let Ok(f) = File::open_native(&path, &opts) {
if f.posix_delete().is_ok() {
return Ok(());
}
@ -1236,10 +1237,7 @@ pub fn unlink(p: &Path) -> io::Result<()> {
}
}
pub fn rename(old: &Path, new: &Path) -> io::Result<()> {
let old = maybe_verbatim(old)?;
let new = maybe_verbatim(new)?;
pub fn rename(old: &WCStr, new: &WCStr) -> io::Result<()> {
if unsafe { c::MoveFileExW(old.as_ptr(), new.as_ptr(), c::MOVEFILE_REPLACE_EXISTING) } == 0 {
let err = api::get_last_error();
// if `MoveFileExW` fails with ERROR_ACCESS_DENIED then try to move
@ -1253,7 +1251,8 @@ pub fn rename(old: &Path, new: &Path) -> io::Result<()> {
// Calculate the layout of the `FILE_RENAME_INFO` we pass to `SetFileInformation`
// This is a dynamically sized struct so we need to get the position of the last field to calculate the actual size.
let Ok(new_len_without_nul_in_bytes): Result<u32, _> = ((new.len() - 1) * 2).try_into()
let Ok(new_len_without_nul_in_bytes): Result<u32, _> =
((new.count_bytes() - 1) * 2).try_into()
else {
return Err(err).io_result();
};
@ -1282,7 +1281,7 @@ pub fn rename(old: &Path, new: &Path) -> io::Result<()> {
new.as_ptr().copy_to_nonoverlapping(
(&raw mut (*file_rename_info).FileName).cast::<u16>(),
new.len(),
new.count_bytes(),
);
}
@ -1309,20 +1308,19 @@ pub fn rename(old: &Path, new: &Path) -> io::Result<()> {
Ok(())
}
pub fn rmdir(p: &Path) -> io::Result<()> {
let p = maybe_verbatim(p)?;
pub fn rmdir(p: &WCStr) -> io::Result<()> {
cvt(unsafe { c::RemoveDirectoryW(p.as_ptr()) })?;
Ok(())
}
pub fn remove_dir_all(path: &Path) -> io::Result<()> {
pub fn remove_dir_all(path: &WCStr) -> io::Result<()> {
// Open a file or directory without following symlinks.
let mut opts = OpenOptions::new();
opts.access_mode(c::FILE_LIST_DIRECTORY);
// `FILE_FLAG_BACKUP_SEMANTICS` allows opening directories.
// `FILE_FLAG_OPEN_REPARSE_POINT` opens a link instead of its target.
opts.custom_flags(c::FILE_FLAG_BACKUP_SEMANTICS | c::FILE_FLAG_OPEN_REPARSE_POINT);
let file = File::open(path, &opts)?;
let file = File::open_native(path, &opts)?;
// Test if the file is not a directory or a symlink to a directory.
if (file.basic_info()?.FileAttributes & c::FILE_ATTRIBUTE_DIRECTORY) == 0 {
@ -1333,14 +1331,14 @@ pub fn remove_dir_all(path: &Path) -> io::Result<()> {
remove_dir_all_iterative(file).io_result()
}
pub fn readlink(path: &Path) -> io::Result<PathBuf> {
pub fn readlink(path: &WCStr) -> io::Result<PathBuf> {
// Open the link with no access mode, instead of generic read.
// By default FILE_LIST_DIRECTORY is denied for the junction "C:\Documents and Settings", so
// this is needed for a common case.
let mut opts = OpenOptions::new();
opts.access_mode(0);
opts.custom_flags(c::FILE_FLAG_OPEN_REPARSE_POINT | c::FILE_FLAG_BACKUP_SEMANTICS);
let file = File::open(path, &opts)?;
let file = File::open_native(&path, &opts)?;
file.readlink()
}
@ -1378,19 +1376,17 @@ pub fn symlink_inner(original: &Path, link: &Path, dir: bool) -> io::Result<()>
}
#[cfg(not(target_vendor = "uwp"))]
pub fn link(original: &Path, link: &Path) -> io::Result<()> {
let original = maybe_verbatim(original)?;
let link = maybe_verbatim(link)?;
pub fn link(original: &WCStr, link: &WCStr) -> io::Result<()> {
cvt(unsafe { c::CreateHardLinkW(link.as_ptr(), original.as_ptr(), ptr::null_mut()) })?;
Ok(())
}
#[cfg(target_vendor = "uwp")]
pub fn link(_original: &Path, _link: &Path) -> io::Result<()> {
pub fn link(_original: &WCStr, _link: &WCStr) -> io::Result<()> {
return Err(io::const_error!(io::ErrorKind::Unsupported, "hard link are not supported on UWP"));
}
pub fn stat(path: &Path) -> io::Result<FileAttr> {
pub fn stat(path: &WCStr) -> io::Result<FileAttr> {
match metadata(path, ReparsePoint::Follow) {
Err(err) if err.raw_os_error() == Some(c::ERROR_CANT_ACCESS_FILE as i32) => {
if let Ok(attrs) = lstat(path) {
@ -1404,7 +1400,7 @@ pub fn stat(path: &Path) -> io::Result<FileAttr> {
}
}
pub fn lstat(path: &Path) -> io::Result<FileAttr> {
pub fn lstat(path: &WCStr) -> io::Result<FileAttr> {
metadata(path, ReparsePoint::Open)
}
@ -1420,7 +1416,7 @@ impl ReparsePoint {
}
}
fn metadata(path: &Path, reparse: ReparsePoint) -> io::Result<FileAttr> {
fn metadata(path: &WCStr, reparse: ReparsePoint) -> io::Result<FileAttr> {
let mut opts = OpenOptions::new();
// No read or write permissions are necessary
opts.access_mode(0);
@ -1429,7 +1425,7 @@ fn metadata(path: &Path, reparse: ReparsePoint) -> io::Result<FileAttr> {
// Attempt to open the file normally.
// If that fails with `ERROR_SHARING_VIOLATION` then retry using `FindFirstFileExW`.
// If the fallback fails for any reason we return the original error.
match File::open(path, &opts) {
match File::open_native(&path, &opts) {
Ok(file) => file.file_attr(),
Err(e)
if [Some(c::ERROR_SHARING_VIOLATION as _), Some(c::ERROR_ACCESS_DENIED as _)]
@ -1442,8 +1438,6 @@ fn metadata(path: &Path, reparse: ReparsePoint) -> io::Result<FileAttr> {
// However, there are special system files, such as
// `C:\hiberfil.sys`, that are locked in a way that denies even that.
unsafe {
let path = maybe_verbatim(path)?;
// `FindFirstFileExW` accepts wildcard file names.
// Fortunately wildcards are not valid file names and
// `ERROR_SHARING_VIOLATION` means the file exists (but is locked)
@ -1482,8 +1476,7 @@ fn metadata(path: &Path, reparse: ReparsePoint) -> io::Result<FileAttr> {
}
}
pub fn set_perm(p: &Path, perm: FilePermissions) -> io::Result<()> {
let p = maybe_verbatim(p)?;
pub fn set_perm(p: &WCStr, perm: FilePermissions) -> io::Result<()> {
unsafe {
cvt(c::SetFileAttributesW(p.as_ptr(), perm.attrs))?;
Ok(())
@ -1499,17 +1492,17 @@ fn get_path(f: &File) -> io::Result<PathBuf> {
)
}
pub fn canonicalize(p: &Path) -> io::Result<PathBuf> {
pub fn canonicalize(p: &WCStr) -> io::Result<PathBuf> {
let mut opts = OpenOptions::new();
// No read or write permissions are necessary
opts.access_mode(0);
// This flag is so we can open directories too
opts.custom_flags(c::FILE_FLAG_BACKUP_SEMANTICS);
let f = File::open(p, &opts)?;
let f = File::open_native(p, &opts)?;
get_path(&f)
}
pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
pub fn copy(from: &WCStr, to: &WCStr) -> io::Result<u64> {
unsafe extern "system" fn callback(
_TotalFileSize: i64,
_TotalBytesTransferred: i64,
@ -1528,13 +1521,11 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
c::PROGRESS_CONTINUE
}
}
let pfrom = maybe_verbatim(from)?;
let pto = maybe_verbatim(to)?;
let mut size = 0i64;
cvt(unsafe {
c::CopyFileExW(
pfrom.as_ptr(),
pto.as_ptr(),
from.as_ptr(),
to.as_ptr(),
Some(callback),
(&raw mut size) as *mut _,
ptr::null_mut(),
@ -1624,14 +1615,14 @@ pub fn junction_point(original: &Path, link: &Path) -> io::Result<()> {
}
// Try to see if a file exists but, unlike `exists`, report I/O errors.
pub fn exists(path: &Path) -> io::Result<bool> {
pub fn exists(path: &WCStr) -> io::Result<bool> {
// Open the file to ensure any symlinks are followed to their target.
let mut opts = OpenOptions::new();
// No read, write, etc access rights are needed.
opts.access_mode(0);
// Backup semantics enables opening directories as well as files.
opts.custom_flags(c::FILE_FLAG_BACKUP_SEMANTICS);
match File::open(path, &opts) {
match File::open_native(path, &opts) {
Err(e) => match e.kind() {
// The file definitely does not exist
io::ErrorKind::NotFound => Ok(false),

View file

@ -10,6 +10,40 @@ mod tests;
pub const MAIN_SEP_STR: &str = "\\";
pub const MAIN_SEP: char = '\\';
/// A null terminated wide string.
#[repr(transparent)]
pub struct WCStr([u16]);
impl WCStr {
/// Convert a slice to a WCStr without checks.
///
/// Though it is memory safe, the slice should also not contain interior nulls
/// as this may lead to unwanted truncation.
///
/// # Safety
///
/// The slice must end in a null.
pub unsafe fn from_wchars_with_null_unchecked(s: &[u16]) -> &Self {
unsafe { &*(s as *const [u16] as *const Self) }
}
pub fn as_ptr(&self) -> *const u16 {
self.0.as_ptr()
}
pub fn count_bytes(&self) -> usize {
self.0.len()
}
}
#[inline]
pub fn with_native_path<T>(path: &Path, f: &dyn Fn(&WCStr) -> io::Result<T>) -> io::Result<T> {
let path = maybe_verbatim(path)?;
// SAFETY: maybe_verbatim returns null-terminated strings
let path = unsafe { WCStr::from_wchars_with_null_unchecked(&path) };
f(path)
}
#[inline]
pub fn is_sep_byte(b: u8) -> bool {
b == b'/' || b == b'\\'