From e39299d577ebb03f859283166b151eebe384275b Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Sun, 9 Jun 2024 14:29:28 +0000 Subject: [PATCH] Migrate more things to WinError --- std/src/sys/pal/windows/api.rs | 36 ++++++++++++++++++++++++++++++ std/src/sys/pal/windows/fs.rs | 30 ++++++++++++------------- std/src/sys/pal/windows/futex.rs | 4 ++-- std/src/sys/pal/windows/os.rs | 7 +++--- std/src/sys/pal/windows/pipe.rs | 10 ++++----- std/src/sys/pal/windows/process.rs | 8 ++++--- std/src/sys/pal/windows/stdio.rs | 4 ++-- 7 files changed, 69 insertions(+), 30 deletions(-) diff --git a/std/src/sys/pal/windows/api.rs b/std/src/sys/pal/windows/api.rs index 555ad581b8568..17a0e47ad5950 100644 --- a/std/src/sys/pal/windows/api.rs +++ b/std/src/sys/pal/windows/api.rs @@ -251,3 +251,39 @@ pub fn get_last_error() -> WinError { pub struct WinError { pub code: u32, } +impl WinError { + const fn new(code: u32) -> Self { + Self { code } + } +} + +// Error code constants. +// The constant names should be the same as the winapi constants except for the leading `ERROR_`. +// Due to the sheer number of codes, error codes should only be added here on an as-needed basis. +// However, they should never be removed as the assumption is they may be useful again in the future. +#[allow(unused)] +impl WinError { + /// Success is not an error. + /// Some Windows APIs do use this to distinguish between a zero return and an error return + /// but we should never return this to users as an error. + pub const SUCCESS: Self = Self::new(c::ERROR_SUCCESS); + // tidy-alphabetical-start + pub const ACCESS_DENIED: Self = Self::new(c::ERROR_ACCESS_DENIED); + pub const ALREADY_EXISTS: Self = Self::new(c::ERROR_ALREADY_EXISTS); + pub const CANT_ACCESS_FILE: Self = Self::new(c::ERROR_CANT_ACCESS_FILE); + pub const DELETE_PENDING: Self = Self::new(c::ERROR_DELETE_PENDING); + pub const DIRECTORY: Self = Self::new(c::ERROR_DIRECTORY); + pub const FILE_NOT_FOUND: Self = Self::new(c::ERROR_FILE_NOT_FOUND); + pub const INSUFFICIENT_BUFFER: Self = Self::new(c::ERROR_INSUFFICIENT_BUFFER); + pub const INVALID_FUNCTION: Self = Self::new(c::ERROR_INVALID_FUNCTION); + pub const INVALID_HANDLE: Self = Self::new(c::ERROR_INVALID_HANDLE); + pub const INVALID_PARAMETER: Self = Self::new(c::ERROR_INVALID_PARAMETER); + pub const NO_MORE_FILES: Self = Self::new(c::ERROR_NO_MORE_FILES); + pub const NOT_FOUND: Self = Self::new(c::ERROR_NOT_FOUND); + pub const NOT_SUPPORTED: Self = Self::new(c::ERROR_NOT_SUPPORTED); + pub const OPERATION_ABORTED: Self = Self::new(c::ERROR_OPERATION_ABORTED); + pub const PATH_NOT_FOUND: Self = Self::new(c::ERROR_PATH_NOT_FOUND); + pub const SHARING_VIOLATION: Self = Self::new(c::ERROR_SHARING_VIOLATION); + pub const TIMEOUT: Self = Self::new(c::ERROR_TIMEOUT); + // tidy-alphabetical-end +} diff --git a/std/src/sys/pal/windows/fs.rs b/std/src/sys/pal/windows/fs.rs index e92c5e80eac9c..629ff114b5a88 100644 --- a/std/src/sys/pal/windows/fs.rs +++ b/std/src/sys/pal/windows/fs.rs @@ -18,7 +18,8 @@ use crate::sys::{c, cvt, Align8}; use crate::sys_common::{AsInner, FromInner, IntoInner}; use crate::thread; -use super::{api, to_u16s, IoResult}; +use super::api::{self, WinError}; +use super::{to_u16s, IoResult}; use crate::sys::path::maybe_verbatim; pub struct File { @@ -130,10 +131,11 @@ impl Iterator for ReadDir { let mut wfd = mem::zeroed(); loop { if c::FindNextFileW(self.handle.0, &mut wfd) == 0 { - if api::get_last_error().code == c::ERROR_NO_MORE_FILES { - return None; - } else { - return Some(Err(Error::last_os_error())); + match api::get_last_error() { + WinError::NO_MORE_FILES => return None, + WinError { code } => { + return Some(Err(Error::from_raw_os_error(code as i32))); + } } } if let Some(e) = DirEntry::new(&self.root, &wfd) { @@ -244,8 +246,6 @@ impl OpenOptions { } fn get_access_mode(&self) -> io::Result { - const ERROR_INVALID_PARAMETER: i32 = 87; - match (self.read, self.write, self.append, self.access_mode) { (.., Some(mode)) => Ok(mode), (true, false, false, None) => Ok(c::GENERIC_READ), @@ -255,23 +255,23 @@ impl OpenOptions { (true, _, true, None) => { Ok(c::GENERIC_READ | (c::FILE_GENERIC_WRITE & !c::FILE_WRITE_DATA)) } - (false, false, false, None) => Err(Error::from_raw_os_error(ERROR_INVALID_PARAMETER)), + (false, false, false, None) => { + Err(Error::from_raw_os_error(c::ERROR_INVALID_PARAMETER as i32)) + } } } fn get_creation_mode(&self) -> io::Result { - const ERROR_INVALID_PARAMETER: i32 = 87; - match (self.write, self.append) { (true, false) => {} (false, false) => { if self.truncate || self.create || self.create_new { - return Err(Error::from_raw_os_error(ERROR_INVALID_PARAMETER)); + return Err(Error::from_raw_os_error(c::ERROR_INVALID_PARAMETER as i32)); } } (_, true) => { if self.truncate && !self.create_new { - return Err(Error::from_raw_os_error(ERROR_INVALID_PARAMETER)); + return Err(Error::from_raw_os_error(c::ERROR_INVALID_PARAMETER as i32)); } } } @@ -315,7 +315,7 @@ impl File { // Manual truncation. See #115745. if opts.truncate && creation == c::OPEN_ALWAYS - && unsafe { c::GetLastError() } == c::ERROR_ALREADY_EXISTS + && api::get_last_error() == WinError::ALREADY_EXISTS { unsafe { // This originally used `FileAllocationInfo` instead of @@ -845,7 +845,7 @@ fn open_link_no_reparse(parent: &File, name: &[u16], access: u32) -> io::Result< // We make a special exception for `STATUS_DELETE_PENDING` because // otherwise this will be mapped to `ERROR_ACCESS_DENIED` which is // very unhelpful. - Err(io::Error::from_raw_os_error(c::ERROR_DELETE_PENDING as _)) + Err(io::Error::from_raw_os_error(c::ERROR_DELETE_PENDING as i32)) } else if status == c::STATUS_INVALID_PARAMETER && ATTRIBUTES.load(Ordering::Relaxed) == c::OBJ_DONT_REPARSE { @@ -1097,7 +1097,7 @@ pub fn readdir(p: &Path) -> io::Result { // // See issue #120040: https://github.com/rust-lang/rust/issues/120040. let last_error = api::get_last_error(); - if last_error.code == c::ERROR_FILE_NOT_FOUND { + if last_error == WinError::FILE_NOT_FOUND { return Ok(ReadDir { handle: FindNextFileHandle(find_handle), root: Arc::new(root), diff --git a/std/src/sys/pal/windows/futex.rs b/std/src/sys/pal/windows/futex.rs index bc19c402d9c12..08b7fe300dc3c 100644 --- a/std/src/sys/pal/windows/futex.rs +++ b/std/src/sys/pal/windows/futex.rs @@ -1,4 +1,4 @@ -use super::api; +use super::api::{self, WinError}; use crate::sys::c; use crate::sys::dur2timeout; use core::ffi::c_void; @@ -72,7 +72,7 @@ pub fn wake_by_address_all(address: &T) { pub fn futex_wait(futex: &W::Atomic, expected: W, timeout: Option) -> bool { // return false only on timeout - wait_on_address(futex, expected, timeout) || api::get_last_error().code != c::ERROR_TIMEOUT + wait_on_address(futex, expected, timeout) || api::get_last_error() != WinError::TIMEOUT } pub fn futex_wake(futex: &T) -> bool { diff --git a/std/src/sys/pal/windows/os.rs b/std/src/sys/pal/windows/os.rs index 483b8b0072c8f..62199c16bfed3 100644 --- a/std/src/sys/pal/windows/os.rs +++ b/std/src/sys/pal/windows/os.rs @@ -17,7 +17,8 @@ use crate::ptr; use crate::slice; use crate::sys::{c, cvt}; -use super::{api, to_u16s}; +use super::api::{self, WinError}; +use super::to_u16s; pub fn errno() -> i32 { api::get_last_error().code as i32 @@ -333,7 +334,7 @@ fn home_dir_crt() -> Option { buf, &mut sz, ) { - 0 if api::get_last_error().code != c::ERROR_INSUFFICIENT_BUFFER => 0, + 0 if api::get_last_error() != WinError::INSUFFICIENT_BUFFER => 0, 0 => sz, _ => sz - 1, // sz includes the null terminator } @@ -358,7 +359,7 @@ fn home_dir_crt() -> Option { super::fill_utf16_buf( |buf, mut sz| { match c::GetUserProfileDirectoryW(token, buf, &mut sz) { - 0 if api::get_last_error().code != c::ERROR_INSUFFICIENT_BUFFER => 0, + 0 if api::get_last_error() != WinError::INSUFFICIENT_BUFFER => 0, 0 => sz, _ => sz - 1, // sz includes the null terminator } diff --git a/std/src/sys/pal/windows/pipe.rs b/std/src/sys/pal/windows/pipe.rs index dfa938d4d5769..67ef3ca82da02 100644 --- a/std/src/sys/pal/windows/pipe.rs +++ b/std/src/sys/pal/windows/pipe.rs @@ -12,6 +12,7 @@ use crate::sys::c; use crate::sys::fs::{File, OpenOptions}; use crate::sys::handle::Handle; use crate::sys::hashmap_random_keys; +use crate::sys::pal::windows::api::{self, WinError}; use crate::sys_common::{FromInner, IntoInner}; //////////////////////////////////////////////////////////////////////////////// @@ -124,20 +125,19 @@ pub fn anon_pipe(ours_readable: bool, their_handle_inheritable: bool) -> io::Res // testing strategy // For more info, see https://github.com/rust-lang/rust/pull/37677. if handle == c::INVALID_HANDLE_VALUE { - let err = io::Error::last_os_error(); - let raw_os_err = err.raw_os_error(); + let error = api::get_last_error(); if tries < 10 { - if raw_os_err == Some(c::ERROR_ACCESS_DENIED as i32) { + if error == WinError::ACCESS_DENIED { continue; } else if reject_remote_clients_flag != 0 - && raw_os_err == Some(c::ERROR_INVALID_PARAMETER as i32) + && error == WinError::INVALID_PARAMETER { reject_remote_clients_flag = 0; tries -= 1; continue; } } - return Err(err); + return Err(io::Error::from_raw_os_error(error.code as i32)); } ours = Handle::from_raw_handle(handle); break; diff --git a/std/src/sys/pal/windows/process.rs b/std/src/sys/pal/windows/process.rs index e4ab2ca7da1ce..2da986a1494ef 100644 --- a/std/src/sys/pal/windows/process.rs +++ b/std/src/sys/pal/windows/process.rs @@ -31,6 +31,8 @@ use crate::sys_common::IntoInner; use core::ffi::c_void; +use super::api::{self, WinError}; + //////////////////////////////////////////////////////////////////////////////// // Command //////////////////////////////////////////////////////////////////////////////// @@ -645,12 +647,12 @@ impl Process { pub fn kill(&mut self) -> io::Result<()> { let result = unsafe { c::TerminateProcess(self.handle.as_raw_handle(), 1) }; if result == c::FALSE { - let error = unsafe { c::GetLastError() }; + let error = api::get_last_error(); // TerminateProcess returns ERROR_ACCESS_DENIED if the process has already been // terminated (by us, or for any other reason). So check if the process was actually // terminated, and if so, do not return an error. - if error != c::ERROR_ACCESS_DENIED || self.try_wait().is_err() { - return Err(crate::io::Error::from_raw_os_error(error as i32)); + if error != WinError::ACCESS_DENIED || self.try_wait().is_err() { + return Err(crate::io::Error::from_raw_os_error(error.code as i32)); } } Ok(()) diff --git a/std/src/sys/pal/windows/stdio.rs b/std/src/sys/pal/windows/stdio.rs index 96c23f82aec2e..690b60d1decca 100644 --- a/std/src/sys/pal/windows/stdio.rs +++ b/std/src/sys/pal/windows/stdio.rs @@ -1,6 +1,6 @@ #![unstable(issue = "none", feature = "windows_stdio")] -use super::api; +use super::api::{self, WinError}; use crate::cmp; use crate::io; use crate::mem::MaybeUninit; @@ -370,7 +370,7 @@ fn read_u16s(handle: c::HANDLE, buf: &mut [MaybeUninit]) -> io::Result