mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Rollup merge of #126194 - ChrisDenton:winerror, r=Mark-Simulacrum

Browse Source 
Migrate more things to `WinError`

In `library/std/src/sys/pal/windows`, we prefer to use the `WinError` type in place of using either `io::Result` or `unsafe { GetLastError }`. The latter is unsafe and weakly typed whereas the former is larger and requires unwrapping because it returns an `Option`.

I also fixed up a couple of places where we were unnecessarily defining error constants that are already defined.
This commit is contained in:
许杰友 Jieyou Xu (Joe) 2024-06-09 20:54:24 +01:00 committed by GitHub
commit 60657d36af
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 69 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
library/std/src/sys/pal/windows/api.rs
View File

@ -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
}

30
library/std/src/sys/pal/windows/fs.rs
View File

@ -18,7 +18,8 @@
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 @@ fn next(&mut self) -> Option<io::Result<DirEntry>> {
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 @@ pub fn security_attributes(&mut self, attrs: c::LPSECURITY_ATTRIBUTES) {
}
fn get_access_mode(&self) -> io::Result<c::DWORD> {
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 @@ fn get_access_mode(&self) -> io::Result<c::DWORD> {
(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<c::DWORD> {
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 @@ pub fn open(path: &Path, opts: &OpenOptions) -> io::Result<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<ReadDir> {
//
// 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),

4
library/std/src/sys/pal/windows/futex.rs
View File

@ -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<T>(address: &T) {
pub fn futex_wait<W: Waitable>(futex: &W::Atomic, expected: W, timeout: Option<Duration>) -> 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<T>(futex: &T) -> bool {

7
library/std/src/sys/pal/windows/os.rs
View File

@ -17,7 +17,8 @@
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<PathBuf> {
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<PathBuf> {
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
}

10
library/std/src/sys/pal/windows/pipe.rs
View File

@ -12,6 +12,7 @@
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;

8
library/std/src/sys/pal/windows/process.rs
View File

@ -31,6 +31,8 @@
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(())

4
library/std/src/sys/pal/windows/stdio.rs
View File

@ -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<u16>]) -> io::Result<usiz
// ReadConsoleW returns success with ERROR_OPERATION_ABORTED for Ctrl-C or Ctrl-Break.
// Explicitly check for that case here and try again.
if amount == 0 && api::get_last_error().code == c::ERROR_OPERATION_ABORTED {
if amount == 0 && api::get_last_error() == WinError::OPERATION_ABORTED {
continue;
}
break;