From a5ac299ac6345ff01036474c83bbcf72af427845 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sat, 16 May 2026 19:57:41 +0900 Subject: [PATCH 01/23] Convert host_env Windows path/argv params from raw *const u16 to &WideCStr --- crates/host_env/src/nt.rs | 68 ++++++++++++++++++++++++--------- crates/host_env/src/winapi.rs | 42 ++++++++++++-------- crates/vm/src/stdlib/_io.rs | 10 ++--- crates/vm/src/stdlib/_winapi.rs | 60 +++++++++++++++++++---------- crates/vm/src/stdlib/nt.rs | 65 ++++++++----------------------- 5 files changed, 136 insertions(+), 109 deletions(-) diff --git a/crates/host_env/src/nt.rs b/crates/host_env/src/nt.rs index 078d588aea5..8ca34880e9a 100644 --- a/crates/host_env/src/nt.rs +++ b/crates/host_env/src/nt.rs @@ -1709,13 +1709,13 @@ pub fn getppid() -> u32 { } } -pub fn path_skip_root(path: *const u16) -> Option { +pub fn path_skip_root(path: &widestring::WideCStr) -> Option { let mut end: *const u16 = core::ptr::null(); - let hr = unsafe { windows_sys::Win32::UI::Shell::PathCchSkipRoot(path, &mut end) }; + let hr = unsafe { windows_sys::Win32::UI::Shell::PathCchSkipRoot(path.as_ptr(), &mut end) }; if hr >= 0 { assert!(!end.is_null()); Some( - unsafe { end.offset_from(path) } + unsafe { end.offset_from(path.as_ptr()) } .try_into() .expect("len must be non-negative"), ) @@ -2150,7 +2150,7 @@ pub fn write_console_utf8(handle: HANDLE, data: &[u8], max_bytes: usize) -> io:: Ok(len) } -pub fn open_console_path_fd(path: *const u16, writable: bool) -> io::Result { +pub fn open_console_path_fd(path: &widestring::WideCStr, writable: bool) -> io::Result { use windows_sys::Win32::{ Foundation::{GENERIC_READ, GENERIC_WRITE}, Storage::FileSystem::{FILE_SHARE_READ, FILE_SHARE_WRITE}, @@ -2164,7 +2164,7 @@ pub fn open_console_path_fd(path: *const u16, writable: bool) -> io::Result let mut handle = unsafe { CreateFileW( - path, + path.as_ptr(), GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, core::ptr::null(), @@ -2176,7 +2176,7 @@ pub fn open_console_path_fd(path: *const u16, writable: bool) -> io::Result if handle == INVALID_HANDLE_VALUE { handle = unsafe { CreateFileW( - path, + path.as_ptr(), access, FILE_SHARE_READ | FILE_SHARE_WRITE, core::ptr::null(), @@ -2216,8 +2216,22 @@ pub fn cwait(pid: intptr_t, opt: i32) -> io::Result<(intptr_t, i32)> { } #[cfg(target_env = "msvc")] -pub fn spawnv(mode: i32, path: *const u16, argv: *const *const u16) -> io::Result { - let result = unsafe { crate::suppress_iph!(_wspawnv(mode, path, argv)) }; +fn null_terminated_ptrs(strings: &[&widestring::WideCStr]) -> Vec<*const u16> { + strings + .iter() + .map(|s| s.as_ptr()) + .chain(core::iter::once(core::ptr::null())) + .collect() +} + +#[cfg(target_env = "msvc")] +pub fn spawnv( + mode: i32, + path: &widestring::WideCStr, + argv: &[&widestring::WideCStr], +) -> io::Result { + let argv_ptrs = null_terminated_ptrs(argv); + let result = unsafe { crate::suppress_iph!(_wspawnv(mode, path.as_ptr(), argv_ptrs.as_ptr())) }; if result == -1 { Err(crate::os::errno_io_error()) } else { @@ -2228,11 +2242,20 @@ pub fn spawnv(mode: i32, path: *const u16, argv: *const *const u16) -> io::Resul #[cfg(target_env = "msvc")] pub fn spawnve( mode: i32, - path: *const u16, - argv: *const *const u16, - envp: *const *const u16, + path: &widestring::WideCStr, + argv: &[&widestring::WideCStr], + envp: &[&widestring::WideCStr], ) -> io::Result { - let result = unsafe { crate::suppress_iph!(_wspawnve(mode, path, argv, envp)) }; + let argv_ptrs = null_terminated_ptrs(argv); + let envp_ptrs = null_terminated_ptrs(envp); + let result = unsafe { + crate::suppress_iph!(_wspawnve( + mode, + path.as_ptr(), + argv_ptrs.as_ptr(), + envp_ptrs.as_ptr() + )) + }; if result == -1 { Err(crate::os::errno_io_error()) } else { @@ -2241,8 +2264,9 @@ pub fn spawnve( } #[cfg(target_env = "msvc")] -pub fn execv(path: *const u16, argv: *const *const u16) -> io::Result<()> { - let result = unsafe { crate::suppress_iph!(_wexecv(path, argv)) }; +pub fn execv(path: &widestring::WideCStr, argv: &[&widestring::WideCStr]) -> io::Result<()> { + let argv_ptrs = null_terminated_ptrs(argv); + let result = unsafe { crate::suppress_iph!(_wexecv(path.as_ptr(), argv_ptrs.as_ptr())) }; if result == -1 { Err(crate::os::errno_io_error()) } else { @@ -2252,11 +2276,19 @@ pub fn execv(path: *const u16, argv: *const *const u16) -> io::Result<()> { #[cfg(target_env = "msvc")] pub fn execve( - path: *const u16, - argv: *const *const u16, - envp: *const *const u16, + path: &widestring::WideCStr, + argv: &[&widestring::WideCStr], + envp: &[&widestring::WideCStr], ) -> io::Result<()> { - let result = unsafe { crate::suppress_iph!(_wexecve(path, argv, envp)) }; + let argv_ptrs = null_terminated_ptrs(argv); + let envp_ptrs = null_terminated_ptrs(envp); + let result = unsafe { + crate::suppress_iph!(_wexecve( + path.as_ptr(), + argv_ptrs.as_ptr(), + envp_ptrs.as_ptr() + )) + }; if result == -1 { Err(crate::os::errno_io_error()) } else { diff --git a/crates/host_env/src/winapi.rs b/crates/host_env/src/winapi.rs index b2a15bca7d4..5ffcd362c30 100644 --- a/crates/host_env/src/winapi.rs +++ b/crates/host_env/src/winapi.rs @@ -798,9 +798,10 @@ pub fn get_version() -> u32 { unsafe { windows_sys::Win32::System::SystemInformation::GetVersion() } } -pub fn create_job_object_w(name: *const u16) -> io::Result { +pub fn create_job_object_w(name: Option<&widestring::WideCStr>) -> io::Result { + let name_ptr = name.map_or(core::ptr::null(), |n| n.as_ptr()); let handle = unsafe { - windows_sys::Win32::System::JobObjects::CreateJobObjectW(core::ptr::null(), name) + windows_sys::Win32::System::JobObjects::CreateJobObjectW(core::ptr::null(), name_ptr) }; if handle.is_null() { Err(io::Error::last_os_error()) @@ -861,14 +862,14 @@ pub fn get_module_file_name(module: HMODULE, buffer: &mut [u16]) -> u32 { } } -pub fn get_short_path_name_w(path: *const u16) -> io::Result> { +pub fn get_short_path_name_w(path: &widestring::WideCStr) -> io::Result> { get_path_name_impl( path, windows_sys::Win32::Storage::FileSystem::GetShortPathNameW, ) } -pub fn get_long_path_name_w(path: *const u16) -> io::Result> { +pub fn get_long_path_name_w(path: &widestring::WideCStr) -> io::Result> { get_path_name_impl( path, windows_sys::Win32::Storage::FileSystem::GetLongPathNameW, @@ -876,16 +877,16 @@ pub fn get_long_path_name_w(path: *const u16) -> io::Result> { } fn get_path_name_impl( - path: *const u16, + path: &widestring::WideCStr, api_fn: unsafe extern "system" fn(*const u16, *mut u16, u32) -> u32, ) -> io::Result> { - let size = unsafe { api_fn(path, core::ptr::null_mut(), 0) }; + let size = unsafe { api_fn(path.as_ptr(), core::ptr::null_mut(), 0) }; if size == 0 { return Err(io::Error::last_os_error()); } let mut buffer = vec![0u16; size as usize]; - let result = unsafe { api_fn(path, buffer.as_mut_ptr(), buffer.len() as u32) }; + let result = unsafe { api_fn(path.as_ptr(), buffer.as_mut_ptr(), buffer.len() as u32) }; if result == 0 { return Err(io::Error::last_os_error()); } @@ -1040,13 +1041,19 @@ pub fn virtual_query_size(address: isize) -> io::Result { } } -pub fn copy_file2(src: *const u16, dst: *const u16, flags: u32) -> io::Result<()> { +pub fn copy_file2( + src: &widestring::WideCStr, + dst: &widestring::WideCStr, + flags: u32, +) -> io::Result<()> { let mut params: windows_sys::Win32::Storage::FileSystem::COPYFILE2_EXTENDED_PARAMETERS = unsafe { core::mem::zeroed() }; params.dwSize = core::mem::size_of_val(¶ms) as u32; params.dwCopyFlags = flags; - let hr = unsafe { windows_sys::Win32::Storage::FileSystem::CopyFile2(src, dst, ¶ms) }; + let hr = unsafe { + windows_sys::Win32::Storage::FileSystem::CopyFile2(src.as_ptr(), dst.as_ptr(), ¶ms) + }; if hr < 0 { let err = if (hr as u32 >> 16) == 0x8007 { (hr as u32) & 0xFFFF @@ -1217,8 +1224,8 @@ pub fn connect_named_pipe(handle: HANDLE) -> io::Result<()> { Ok(()) } -pub fn wait_named_pipe_w(name: *const u16, timeout: u32) -> io::Result<()> { - let ok = unsafe { windows_sys::Win32::System::Pipes::WaitNamedPipeW(name, timeout) }; +pub fn wait_named_pipe_w(name: &widestring::WideCStr, timeout: u32) -> io::Result<()> { + let ok = unsafe { windows_sys::Win32::System::Pipes::WaitNamedPipeW(name.as_ptr(), timeout) }; if ok == 0 { Err(io::Error::last_os_error()) } else { @@ -1359,12 +1366,16 @@ pub fn set_named_pipe_handle_state( } } -pub fn create_mutex_w(initial_owner: bool, name: *const u16) -> io::Result { +pub fn create_mutex_w( + initial_owner: bool, + name: Option<&widestring::WideCStr>, +) -> io::Result { + let name_ptr = name.map_or(core::ptr::null(), |n| n.as_ptr()); let handle = unsafe { windows_sys::Win32::System::Threading::CreateMutexW( core::ptr::null(), i32::from(initial_owner), - name, + name_ptr, ) }; if handle.is_null() { @@ -1393,8 +1404,9 @@ pub fn open_event_w( } } -pub fn need_current_directory_for_exe_path_w(exe_name: *const u16) -> bool { +pub fn need_current_directory_for_exe_path_w(exe_name: &widestring::WideCStr) -> bool { unsafe { - windows_sys::Win32::System::Environment::NeedCurrentDirectoryForExePathW(exe_name) != 0 + windows_sys::Win32::System::Environment::NeedCurrentDirectoryForExePathW(exe_name.as_ptr()) + != 0 } } diff --git a/crates/vm/src/stdlib/_io.rs b/crates/vm/src/stdlib/_io.rs index 65b7486dfd6..ca99cc07da2 100644 --- a/crates/vm/src/stdlib/_io.rs +++ b/crates/vm/src/stdlib/_io.rs @@ -6013,13 +6013,11 @@ mod winconsoleio { } let name_str = nameobj.str(vm)?; - let wide = name_str - .as_wtf8() - .encode_wide() - .chain(core::iter::once(0)) - .collect::>(); + let wide = widestring::WideCString::from_vec_truncate( + name_str.as_wtf8().encode_wide().collect::>(), + ); - fd = host_nt::open_console_path_fd(wide.as_ptr(), writable) + fd = host_nt::open_console_path_fd(&wide, writable) .map_err(|err| err.to_pyexception(vm))?; } else { // When opened by fd, never close the fd (user owns it) diff --git a/crates/vm/src/stdlib/_winapi.rs b/crates/vm/src/stdlib/_winapi.rs index 5d4865f1ffc..d31dc375e3c 100644 --- a/crates/vm/src/stdlib/_winapi.rs +++ b/crates/vm/src/stdlib/_winapi.rs @@ -301,8 +301,10 @@ mod _winapi { #[pyfunction] fn NeedCurrentDirectoryForExePath(exe_name: PyStrRef) -> bool { - let exe_name = exe_name.as_wtf8().to_wide_with_nul(); - host_winapi::need_current_directory_for_exe_path_w(exe_name.as_ptr()) + let exe_name = widestring::WideCString::from_vec_truncate( + exe_name.as_wtf8().encode_wide().collect::>(), + ); + host_winapi::need_current_directory_for_exe_path_w(&exe_name) } #[pyfunction] @@ -419,8 +421,12 @@ mod _winapi { name: OptionalArg>, vm: &VirtualMachine, ) -> PyResult { - let name = name.flatten().map(|name| name.as_wtf8().to_wide_with_nul()); - host_winapi::create_job_object_w(name.as_ref().map_or(null(), |name| name.as_ptr())) + let name = name.flatten().map(|name| { + widestring::WideCString::from_vec_truncate( + name.as_wtf8().encode_wide().collect::>(), + ) + }); + host_winapi::create_job_object_w(name.as_deref()) .map(WinHandle) .map_err(|e| e.to_pyexception(vm)) } @@ -672,27 +678,32 @@ mod _winapi { /// GetShortPathName - Return the short version of the provided path. #[pyfunction] fn GetShortPathName(path: PyStrRef, vm: &VirtualMachine) -> PyResult { - let path_wide = path.as_wtf8().to_wide_with_nul(); - let wide = host_winapi::get_short_path_name_w(path_wide.as_ptr()) - .map_err(|e| e.to_pyexception(vm))?; + let path_wide = widestring::WideCString::from_vec_truncate( + path.as_wtf8().encode_wide().collect::>(), + ); + let wide = + host_winapi::get_short_path_name_w(&path_wide).map_err(|e| e.to_pyexception(vm))?; Ok(path_name_result_to_pystr(wide, vm)) } /// GetLongPathName - Return the long version of the provided path. #[pyfunction] fn GetLongPathName(path: PyStrRef, vm: &VirtualMachine) -> PyResult { - let path_wide = path.as_wtf8().to_wide_with_nul(); - let wide = host_winapi::get_long_path_name_w(path_wide.as_ptr()) - .map_err(|e| e.to_pyexception(vm))?; + let path_wide = widestring::WideCString::from_vec_truncate( + path.as_wtf8().encode_wide().collect::>(), + ); + let wide = + host_winapi::get_long_path_name_w(&path_wide).map_err(|e| e.to_pyexception(vm))?; Ok(path_name_result_to_pystr(wide, vm)) } /// WaitNamedPipe - Wait for an instance of a named pipe to become available. #[pyfunction] fn WaitNamedPipe(name: PyStrRef, timeout: u32, vm: &VirtualMachine) -> PyResult<()> { - let name_wide = name.as_wtf8().to_wide_with_nul(); - host_winapi::wait_named_pipe_w(name_wide.as_ptr(), timeout) - .map_err(|e| e.to_pyexception(vm)) + let name_wide = widestring::WideCString::from_vec_truncate( + name.as_wtf8().encode_wide().collect::>(), + ); + host_winapi::wait_named_pipe_w(&name_wide, timeout).map_err(|e| e.to_pyexception(vm)) } /// PeekNamedPipe - Peek at data in a named pipe without removing it. @@ -872,9 +883,12 @@ mod _winapi { vm: &VirtualMachine, ) -> PyResult { let _ = security_attributes; - let name_wide = name.map(|n| n.as_wtf8().to_wide_with_nul()); - let name_ptr = name_wide.as_ref().map_or(null(), |n| n.as_ptr()); - host_winapi::create_mutex_w(initial_owner, name_ptr) + let name_wide = name.map(|n| { + widestring::WideCString::from_vec_truncate( + n.as_wtf8().encode_wide().collect::>(), + ) + }); + host_winapi::create_mutex_w(initial_owner, name_wide.as_deref()) .map(WinHandle) .map_err(|e| e.to_pyexception(vm)) } @@ -1073,10 +1087,16 @@ mod _winapi { _progress_routine: OptionalArg, vm: &VirtualMachine, ) -> PyResult<()> { - let src_wide = existing_file_name.as_wtf8().to_wide_with_nul(); - let dst_wide = new_file_name.as_wtf8().to_wide_with_nul(); - host_winapi::copy_file2(src_wide.as_ptr(), dst_wide.as_ptr(), flags) - .map_err(|e| e.to_pyexception(vm)) + let src_wide = widestring::WideCString::from_vec_truncate( + existing_file_name + .as_wtf8() + .encode_wide() + .collect::>(), + ); + let dst_wide = widestring::WideCString::from_vec_truncate( + new_file_name.as_wtf8().encode_wide().collect::>(), + ); + host_winapi::copy_file2(&src_wide, &dst_wide, flags).map_err(|e| e.to_pyexception(vm)) } /// _mimetypes_read_windows_registry - Read MIME type associations from registry. diff --git a/crates/vm/src/stdlib/nt.rs b/crates/vm/src/stdlib/nt.rs index ed6061f7fd2..4c2c560476b 100644 --- a/crates/vm/src/stdlib/nt.rs +++ b/crates/vm/src/stdlib/nt.rs @@ -412,7 +412,6 @@ pub(crate) mod module { vm: &VirtualMachine, ) -> PyResult { use crate::function::FsPath; - use core::iter::once; let path = path.to_wide_cstring(vm)?; @@ -429,14 +428,8 @@ pub(crate) mod module { return Err(vm.new_value_error("spawnv() arg 3 first element cannot be empty")); } - let argv_spawn: Vec<*const u16> = argv - .iter() - .map(|v| v.as_ptr()) - .chain(once(core::ptr::null())) - .collect(); - - host_nt::spawnv(mode, path.as_ptr(), argv_spawn.as_ptr()) - .map_err(|_| vm.new_last_errno_error()) + let argv_refs: Vec<&widestring::WideCStr> = argv.iter().map(|s| s.as_ref()).collect(); + host_nt::spawnv(mode, &path, &argv_refs).map_err(|_| vm.new_last_errno_error()) } #[cfg(target_env = "msvc")] @@ -449,7 +442,6 @@ pub(crate) mod module { vm: &VirtualMachine, ) -> PyResult { use crate::function::FsPath; - use core::iter::once; let path = path.to_wide_cstring(vm)?; @@ -466,12 +458,6 @@ pub(crate) mod module { return Err(vm.new_value_error("spawnve() arg 2 first element cannot be empty")); } - let argv_spawn: Vec<*const u16> = argv - .iter() - .map(|v| v.as_ptr()) - .chain(once(core::ptr::null())) - .collect(); - // Build environment strings as "KEY=VALUE\0" wide strings let mut env_strings: Vec = Vec::new(); for (key, value) in env { @@ -494,14 +480,10 @@ pub(crate) mod module { ); } - let envp: Vec<*const u16> = env_strings - .iter() - .map(|s| s.as_ptr()) - .chain(once(core::ptr::null())) - .collect(); - - host_nt::spawnve(mode, path.as_ptr(), argv_spawn.as_ptr(), envp.as_ptr()) - .map_err(|_| vm.new_last_errno_error()) + let argv_refs: Vec<&widestring::WideCStr> = argv.iter().map(|s| s.as_ref()).collect(); + let envp_refs: Vec<&widestring::WideCStr> = + env_strings.iter().map(|s| s.as_ref()).collect(); + host_nt::spawnve(mode, &path, &argv_refs, &envp_refs).map_err(|_| vm.new_last_errno_error()) } #[cfg(target_env = "msvc")] @@ -511,8 +493,6 @@ pub(crate) mod module { argv: Either, vm: &VirtualMachine, ) -> PyResult<()> { - use core::iter::once; - let make_widestring = |s: &str| widestring::WideCString::from_os_str(s).map_err(|err| err.to_pyexception(vm)); @@ -531,13 +511,8 @@ pub(crate) mod module { return Err(vm.new_value_error("execv() arg 2 first element cannot be empty")); } - let argv_execv: Vec<*const u16> = argv - .iter() - .map(|v| v.as_ptr()) - .chain(once(core::ptr::null())) - .collect(); - - host_nt::execv(path.as_ptr(), argv_execv.as_ptr()).map_err(|_| vm.new_last_errno_error()) + let argv_refs: Vec<&widestring::WideCStr> = argv.iter().map(|s| s.as_ref()).collect(); + host_nt::execv(&path, &argv_refs).map_err(|_| vm.new_last_errno_error()) } #[cfg(target_env = "msvc")] @@ -548,8 +523,6 @@ pub(crate) mod module { env: ArgMapping, vm: &VirtualMachine, ) -> PyResult<()> { - use core::iter::once; - let make_widestring = |s: &str| widestring::WideCString::from_os_str(s).map_err(|err| err.to_pyexception(vm)); @@ -568,12 +541,6 @@ pub(crate) mod module { return Err(vm.new_value_error("execve: argv first element cannot be empty")); } - let argv_execve: Vec<*const u16> = argv - .iter() - .map(|v| v.as_ptr()) - .chain(once(core::ptr::null())) - .collect(); - let env = crate::stdlib::os::envobj_to_dict(env, vm)?; // Build environment strings as "KEY=VALUE\0" wide strings let mut env_strings: Vec = Vec::new(); @@ -598,14 +565,10 @@ pub(crate) mod module { env_strings.push(make_widestring(&env_str)?); } - let envp: Vec<*const u16> = env_strings - .iter() - .map(|s| s.as_ptr()) - .chain(once(core::ptr::null())) - .collect(); - - host_nt::execve(path.as_ptr(), argv_execve.as_ptr(), envp.as_ptr()) - .map_err(|_| vm.new_last_errno_error()) + let argv_refs: Vec<&widestring::WideCStr> = argv.iter().map(|s| s.as_ref()).collect(); + let envp_refs: Vec<&widestring::WideCStr> = + env_strings.iter().map(|s| s.as_ref()).collect(); + host_nt::execve(&path, &argv_refs, &envp_refs).map_err(|_| vm.new_last_errno_error()) } #[pyfunction] @@ -792,7 +755,9 @@ pub(crate) mod module { .chain(core::iter::once(0)) // null-terminated .collect(); - if let Some(len) = host_nt::path_skip_root(backslashed.as_ptr()) { + let backslashed_wide = widestring::WideCStr::from_slice_truncate(&backslashed) + .expect("backslashed is null-terminated"); + if let Some(len) = host_nt::path_skip_root(backslashed_wide) { assert!( len < backslashed.len(), // backslashed is null-terminated "path: {:?} {} < {}", From cea48cc9e2c4286b98b421b4d4e06e5bd756925d Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sat, 16 May 2026 20:30:19 +0900 Subject: [PATCH 02/23] Migrate remaining winapi raw u16 pointer signatures to typed references --- crates/host_env/src/winapi.rs | 74 ++++++++++---------- crates/host_env/src/windows.rs | 3 + crates/stdlib/src/overlapped.rs | 16 ++--- crates/vm/src/stdlib/_io.rs | 5 +- crates/vm/src/stdlib/_winapi.rs | 119 +++++++++++--------------------- 5 files changed, 89 insertions(+), 128 deletions(-) diff --git a/crates/host_env/src/winapi.rs b/crates/host_env/src/winapi.rs index 5ffcd362c30..86542c0df28 100644 --- a/crates/host_env/src/winapi.rs +++ b/crates/host_env/src/winapi.rs @@ -163,7 +163,7 @@ impl Drop for AttrList { } pub fn create_file_w( - file_name: *const u16, + file_name: &widestring::WideCStr, desired_access: u32, share_mode: u32, creation_disposition: u32, @@ -171,7 +171,7 @@ pub fn create_file_w( ) -> io::Result { let handle = unsafe { windows_sys::Win32::Storage::FileSystem::CreateFileW( - file_name, + file_name.as_ptr(), desired_access, share_mode, core::ptr::null(), @@ -188,27 +188,27 @@ pub fn create_file_w( } /// # Safety -/// The pointer arguments must follow the Win32 `CreateProcessW` contract. -pub unsafe fn create_process_w( - app_name: *const u16, - command_line: *mut u16, +/// `startup_info` must point to a valid `STARTUPINFOW` (or extended). +unsafe fn create_process_w_raw( + app_name: Option<&widestring::WideCStr>, + command_line: Option<&mut [u16]>, inherit_handles: i32, creation_flags: u32, - env: *mut u16, - current_dir: *mut u16, + env: Option<&[u16]>, + current_dir: Option<&widestring::WideCStr>, startup_info: *mut windows_sys::Win32::System::Threading::STARTUPINFOW, ) -> io::Result { let mut procinfo = core::mem::MaybeUninit::::uninit(); let ok = unsafe { windows_sys::Win32::System::Threading::CreateProcessW( - app_name, - command_line, + app_name.map_or(core::ptr::null(), |s| s.as_ptr()), + command_line.map_or(core::ptr::null_mut(), |s| s.as_mut_ptr()), core::ptr::null(), core::ptr::null(), inherit_handles, creation_flags, - env.cast(), - current_dir, + env.map_or(core::ptr::null(), |s| s.as_ptr().cast()), + current_dir.map_or(core::ptr::null(), |s| s.as_ptr()), startup_info, procinfo.as_mut_ptr(), ) @@ -225,12 +225,12 @@ pub unsafe fn create_process_w( reason = "This is the semantic host wrapper for Win32 CreateProcess parameters." )] pub fn create_process( - app_name: *const u16, - command_line: *mut u16, + app_name: Option<&widestring::WideCStr>, + command_line: Option<&mut [u16]>, inherit_handles: i32, creation_flags: u32, - env: *mut u16, - current_dir: *mut u16, + env: Option<&[u16]>, + current_dir: Option<&widestring::WideCStr>, startup_info: StartupInfoData, handle_list: Option>, ) -> io::Result { @@ -249,7 +249,7 @@ pub fn create_process( .map_or_else(core::ptr::null_mut, |l| l.as_mut_ptr() as _); let procinfo = unsafe { - create_process_w( + create_process_w_raw( app_name, command_line, inherit_handles, @@ -416,14 +416,15 @@ pub fn create_pipe(size: u32) -> io::Result<(HANDLE, HANDLE)> { pub fn create_event_w( manual_reset: bool, initial_state: bool, - name: *const u16, + name: Option<&widestring::WideCStr>, ) -> io::Result { + let name_ptr = name.map_or(core::ptr::null(), |n| n.as_ptr()); let handle = unsafe { windows_sys::Win32::System::Threading::CreateEventW( core::ptr::null(), i32::from(manual_reset), i32::from(initial_state), - name, + name_ptr, ) }; if handle.is_null() { @@ -897,13 +898,13 @@ fn get_path_name_impl( pub fn open_mutex_w( desired_access: u32, inherit_handle: bool, - name: *const u16, + name: &widestring::WideCStr, ) -> io::Result { let handle = unsafe { windows_sys::Win32::System::Threading::OpenMutexW( desired_access, i32::from(inherit_handle), - name, + name.as_ptr(), ) }; if handle.is_null() { @@ -918,7 +919,7 @@ pub fn release_mutex(handle: HANDLE) -> i32 { } pub fn create_named_pipe_w( - name: *const u16, + name: &widestring::WideCStr, open_mode: u32, pipe_mode: u32, max_instances: u32, @@ -928,7 +929,7 @@ pub fn create_named_pipe_w( ) -> io::Result { let handle = unsafe { windows_sys::Win32::System::Pipes::CreateNamedPipeW( - name, + name.as_ptr(), open_mode, pipe_mode, max_instances, @@ -950,8 +951,9 @@ pub fn create_file_mapping_w( protect: u32, max_size_high: u32, max_size_low: u32, - name: *const u16, + name: Option<&widestring::WideCStr>, ) -> io::Result { + let name_ptr = name.map_or(core::ptr::null(), |n| n.as_ptr()); let handle = unsafe { windows_sys::Win32::System::Memory::CreateFileMappingW( file_handle, @@ -959,7 +961,7 @@ pub fn create_file_mapping_w( protect, max_size_high, max_size_low, - name, + name_ptr, ) }; if handle.is_null() { @@ -972,13 +974,13 @@ pub fn create_file_mapping_w( pub fn open_file_mapping_w( desired_access: u32, inherit_handle: bool, - name: *const u16, + name: &widestring::WideCStr, ) -> io::Result { let handle = unsafe { windows_sys::Win32::System::Memory::OpenFileMappingW( desired_access, i32::from(inherit_handle), - name, + name.as_ptr(), ) }; if handle.is_null() { @@ -1168,16 +1170,16 @@ where } pub fn lc_map_string_ex( - locale: *const u16, + locale: &widestring::WideCStr, flags: u32, - src: *const u16, - src_len: i32, + src: &[u16], ) -> io::Result> { + let src_len = src.len() as i32; let dest_size = unsafe { windows_sys::Win32::Globalization::LCMapStringEx( - locale, + locale.as_ptr(), flags, - src, + src.as_ptr(), src_len, core::ptr::null_mut(), 0, @@ -1193,9 +1195,9 @@ pub fn lc_map_string_ex( let mut dest = vec![0u16; dest_size as usize]; let nmapped = unsafe { windows_sys::Win32::Globalization::LCMapStringEx( - locale, + locale.as_ptr(), flags, - src, + src.as_ptr(), src_len, dest.as_mut_ptr(), dest_size, @@ -1388,13 +1390,13 @@ pub fn create_mutex_w( pub fn open_event_w( desired_access: u32, inherit_handle: bool, - name: *const u16, + name: &widestring::WideCStr, ) -> io::Result { let handle = unsafe { windows_sys::Win32::System::Threading::OpenEventW( desired_access, i32::from(inherit_handle), - name, + name.as_ptr(), ) }; if handle.is_null() { diff --git a/crates/host_env/src/windows.rs b/crates/host_env/src/windows.rs index e43cd1aa9f6..75b5ec8ab7c 100644 --- a/crates/host_env/src/windows.rs +++ b/crates/host_env/src/windows.rs @@ -313,6 +313,9 @@ pub fn multi_byte_to_wide( pub trait ToWideString { fn to_wide(&self) -> Vec; fn to_wide_with_nul(&self) -> Vec; + fn to_wide_cstring(&self) -> widestring::WideCString { + widestring::WideCString::from_vec_truncate(self.to_wide()) + } } impl ToWideString for T diff --git a/crates/stdlib/src/overlapped.rs b/crates/stdlib/src/overlapped.rs index 7ff51c4c26a..130d68e0d84 100644 --- a/crates/stdlib/src/overlapped.rs +++ b/crates/stdlib/src/overlapped.rs @@ -1077,7 +1077,7 @@ mod _overlapped { let mut event = event.unwrap_or(INVALID_HANDLE_VALUE); if event == INVALID_HANDLE_VALUE { - event = host_winapi::create_event_w(true, false, core::ptr::null()) + event = host_winapi::create_event_w(true, false, None) .map(|handle| handle as isize) .map_err(|err| { set_from_windows_err(err.raw_os_error().unwrap_or(0) as u32, vm) @@ -1251,15 +1251,11 @@ mod _overlapped { return Err(vm.new_value_error("EventAttributes must be None")); } - let name_wide: Option> = - name.map(|n| n.encode_utf16().chain(core::iter::once(0)).collect()); - host_winapi::create_event_w( - manual_reset, - initial_state, - name_wide.as_ref().map_or(core::ptr::null(), |n| n.as_ptr()), - ) - .map(|h| h as isize) - .map_err(|err| set_from_windows_err(err.raw_os_error().unwrap_or(0) as u32, vm)) + let name_wide: Option = + name.map(|n| widestring::WideCString::from_str_truncate(&n)); + host_winapi::create_event_w(manual_reset, initial_state, name_wide.as_deref()) + .map(|h| h as isize) + .map_err(|err| set_from_windows_err(err.raw_os_error().unwrap_or(0) as u32, vm)) } #[pyfunction] diff --git a/crates/vm/src/stdlib/_io.rs b/crates/vm/src/stdlib/_io.rs index ca99cc07da2..0c846fee2ac 100644 --- a/crates/vm/src/stdlib/_io.rs +++ b/crates/vm/src/stdlib/_io.rs @@ -5843,6 +5843,7 @@ mod winconsoleio { use crossbeam_utils::atomic::AtomicCell; use rustpython_host_env::io as host_io; use rustpython_host_env::nt as host_nt; + use rustpython_host_env::windows::ToWideString; type HANDLE = host_nt::Handle; const SMALLBUF: usize = 4; @@ -6013,9 +6014,7 @@ mod winconsoleio { } let name_str = nameobj.str(vm)?; - let wide = widestring::WideCString::from_vec_truncate( - name_str.as_wtf8().encode_wide().collect::>(), - ); + let wide = name_str.as_wtf8().to_wide_cstring(); fd = host_nt::open_console_path_fd(&wide, writable) .map_err(|err| err.to_pyexception(vm))?; diff --git a/crates/vm/src/stdlib/_winapi.rs b/crates/vm/src/stdlib/_winapi.rs index d31dc375e3c..979c763a1f5 100644 --- a/crates/vm/src/stdlib/_winapi.rs +++ b/crates/vm/src/stdlib/_winapi.rs @@ -14,7 +14,7 @@ mod _winapi { types::Constructor, windows::{WinHandle, WindowsSysResult}, }; - use core::ptr::{null, null_mut}; + use core::ptr::null_mut; use rustpython_common::wtf8::Wtf8Buf; use rustpython_host_env::overlapped as host_overlapped; use rustpython_host_env::winapi as host_winapi; @@ -92,9 +92,9 @@ mod _winapi { _template_file: PyObjectRef, // Always NULL (0) vm: &VirtualMachine, ) -> PyResult { - let file_name_wide = file_name.as_wtf8().to_wide_with_nul(); + let file_name_wide = file_name.as_wtf8().to_wide_cstring(); host_winapi::create_file_w( - file_name_wide.as_ptr(), + &file_name_wide, desired_access, share_mode, creation_disposition, @@ -223,18 +223,15 @@ mod _winapi { std_error: si_attr!(hStdError, isize), }; - let mut env = args + let env = args .env_mapping .map(|m| getenvironment(m, vm)) .transpose()?; - let env = env.as_mut().map_or_else(null_mut, |v| v.as_mut_ptr()); let handle_list = get_handle_list(args.startup_info.get_attr("lpAttributeList", vm)?, vm)?; - let wstr = |s: PyStrRef| { - let ws = widestring::WideCString::from_str(s.expect_str()) - .map_err(|err| err.to_pyexception(vm))?; - Ok(ws.into_vec_with_nul()) + let wcstring = |s: PyStrRef| { + widestring::WideCString::from_str(s.expect_str()).map_err(|err| err.to_pyexception(vm)) }; // Validate no embedded null bytes in command name and command line @@ -249,26 +246,20 @@ mod _winapi { return Err(crate::exceptions::cstring_error(vm)); } - let app_name = args.name.map(wstr).transpose()?; - let app_name = app_name.as_ref().map_or_else(null, |w| w.as_ptr()); - - let mut command_line = args.command_line.map(wstr).transpose()?; - let command_line = command_line - .as_mut() - .map_or_else(null_mut, |w| w.as_mut_ptr()); - - let mut current_dir = args.current_dir.map(wstr).transpose()?; - let current_dir = current_dir - .as_mut() - .map_or_else(null_mut, |w| w.as_mut_ptr()); + let app_name = args.name.map(wcstring).transpose()?; + let current_dir = args.current_dir.map(wcstring).transpose()?; + let mut command_line = args + .command_line + .map(|s| wcstring(s).map(|w| w.into_vec_with_nul())) + .transpose()?; let procinfo = host_winapi::create_process( - app_name, - command_line, + app_name.as_deref(), + command_line.as_deref_mut(), args.inherit_handles, args.creation_flags, - env, - current_dir, + env.as_deref(), + current_dir.as_deref(), startup_info, handle_list, ) @@ -301,9 +292,7 @@ mod _winapi { #[pyfunction] fn NeedCurrentDirectoryForExePath(exe_name: PyStrRef) -> bool { - let exe_name = widestring::WideCString::from_vec_truncate( - exe_name.as_wtf8().encode_wide().collect::>(), - ); + let exe_name = exe_name.as_wtf8().to_wide_cstring(); host_winapi::need_current_directory_for_exe_path_w(&exe_name) } @@ -421,11 +410,7 @@ mod _winapi { name: OptionalArg>, vm: &VirtualMachine, ) -> PyResult { - let name = name.flatten().map(|name| { - widestring::WideCString::from_vec_truncate( - name.as_wtf8().encode_wide().collect::>(), - ) - }); + let name = name.flatten().map(|name| name.as_wtf8().to_wide_cstring()); host_winapi::create_job_object_w(name.as_deref()) .map(WinHandle) .map_err(|e| e.to_pyexception(vm)) @@ -471,8 +456,8 @@ mod _winapi { name: PyStrRef, vm: &VirtualMachine, ) -> PyResult { - let name_wide = name.as_wtf8().to_wide_with_nul(); - host_winapi::open_mutex_w(desired_access, inherit_handle, name_wide.as_ptr()) + let name_wide = name.as_wtf8().to_wide_cstring(); + host_winapi::open_mutex_w(desired_access, inherit_handle, &name_wide) .map(WinHandle) .map_err(|e| e.to_pyexception(vm)) } @@ -515,20 +500,15 @@ mod _winapi { } // Use ToWideString which properly handles WTF-8 (including surrogates) - let locale_wide = locale.as_wtf8().to_wide_with_nul(); + let locale_wide = locale.as_wtf8().to_wide_cstring(); let src_wide = src.as_wtf8().to_wide(); if src_wide.len() > i32::MAX as usize { return Err(vm.new_overflow_error("input string is too long")); } - let dest = host_winapi::lc_map_string_ex( - locale_wide.as_ptr(), - flags, - src_wide.as_ptr(), - src_wide.len() as i32, - ) - .map_err(|e| e.to_pyexception(vm))?; + let dest = host_winapi::lc_map_string_ex(&locale_wide, flags, &src_wide) + .map_err(|e| e.to_pyexception(vm))?; // Convert UTF-16 back to WTF-8 (handles surrogates properly) let result = Wtf8Buf::from_wide(&dest); @@ -558,9 +538,9 @@ mod _winapi { /// CreateNamedPipe - Create a named pipe #[pyfunction] fn CreateNamedPipe(args: CreateNamedPipeArgs, vm: &VirtualMachine) -> PyResult { - let name_wide = args.name.as_wtf8().to_wide_with_nul(); + let name_wide = args.name.as_wtf8().to_wide_cstring(); host_winapi::create_named_pipe_w( - name_wide.as_ptr(), + &name_wide, args.open_mode, args.pipe_mode, args.max_instances, @@ -678,9 +658,7 @@ mod _winapi { /// GetShortPathName - Return the short version of the provided path. #[pyfunction] fn GetShortPathName(path: PyStrRef, vm: &VirtualMachine) -> PyResult { - let path_wide = widestring::WideCString::from_vec_truncate( - path.as_wtf8().encode_wide().collect::>(), - ); + let path_wide = path.as_wtf8().to_wide_cstring(); let wide = host_winapi::get_short_path_name_w(&path_wide).map_err(|e| e.to_pyexception(vm))?; Ok(path_name_result_to_pystr(wide, vm)) @@ -689,9 +667,7 @@ mod _winapi { /// GetLongPathName - Return the long version of the provided path. #[pyfunction] fn GetLongPathName(path: PyStrRef, vm: &VirtualMachine) -> PyResult { - let path_wide = widestring::WideCString::from_vec_truncate( - path.as_wtf8().encode_wide().collect::>(), - ); + let path_wide = path.as_wtf8().to_wide_cstring(); let wide = host_winapi::get_long_path_name_w(&path_wide).map_err(|e| e.to_pyexception(vm))?; Ok(path_name_result_to_pystr(wide, vm)) @@ -700,9 +676,7 @@ mod _winapi { /// WaitNamedPipe - Wait for an instance of a named pipe to become available. #[pyfunction] fn WaitNamedPipe(name: PyStrRef, timeout: u32, vm: &VirtualMachine) -> PyResult<()> { - let name_wide = widestring::WideCString::from_vec_truncate( - name.as_wtf8().encode_wide().collect::>(), - ); + let name_wide = name.as_wtf8().to_wide_cstring(); host_winapi::wait_named_pipe_w(&name_wide, timeout).map_err(|e| e.to_pyexception(vm)) } @@ -756,9 +730,8 @@ mod _winapi { ) -> PyResult { let _ = security_attributes; // Ignored, always NULL - let name_wide = name.map(|n| n.as_wtf8().to_wide_with_nul()); - let name_ptr = name_wide.as_ref().map_or(null(), |n| n.as_ptr()); - host_winapi::create_event_w(manual_reset, initial_state, name_ptr) + let name_wide = name.map(|n| n.as_wtf8().to_wide_cstring()); + host_winapi::create_event_w(manual_reset, initial_state, name_wide.as_deref()) .map(WinHandle) .map_err(|e| e.to_pyexception(vm)) } @@ -883,11 +856,7 @@ mod _winapi { vm: &VirtualMachine, ) -> PyResult { let _ = security_attributes; - let name_wide = name.map(|n| { - widestring::WideCString::from_vec_truncate( - n.as_wtf8().encode_wide().collect::>(), - ) - }); + let name_wide = name.map(|n| n.as_wtf8().to_wide_cstring()); host_winapi::create_mutex_w(initial_owner, name_wide.as_deref()) .map(WinHandle) .map_err(|e| e.to_pyexception(vm)) @@ -901,8 +870,8 @@ mod _winapi { name: PyStrRef, vm: &VirtualMachine, ) -> PyResult { - let name_wide = name.as_wtf8().to_wide_with_nul(); - host_winapi::open_event_w(desired_access, inherit_handle, name_wide.as_ptr()) + let name_wide = name.as_wtf8().to_wide_cstring(); + host_winapi::open_event_w(desired_access, inherit_handle, &name_wide) .map(WinHandle) .map_err(|e| e.to_pyexception(vm)) } @@ -949,7 +918,7 @@ mod _winapi { if is_main { let handle = crate::signal::get_sigint_event().map_or_else( || { - let handle = host_winapi::create_event_w(true, false, null()) + let handle = host_winapi::create_event_w(true, false, None) .unwrap_or(core::ptr::null_mut()); if !handle.is_null() { crate::signal::set_sigint_event(handle as isize); @@ -1014,14 +983,13 @@ mod _winapi { vm.new_value_error("CreateFileMapping: name must not contain null characters") ); } - let name_wide = name.as_ref().map(|n| n.as_wtf8().to_wide_with_nul()); - let name_ptr = name_wide.as_ref().map_or(null(), |n| n.as_ptr()); + let name_wide = name.as_ref().map(|n| n.as_wtf8().to_wide_cstring()); host_winapi::create_file_mapping_w( file_handle.0, protect, max_size_high, max_size_low, - name_ptr, + name_wide.as_deref(), ) .map(WinHandle) .map_err(|e| e.to_pyexception(vm)) @@ -1040,8 +1008,8 @@ mod _winapi { vm.new_value_error("OpenFileMapping: name must not contain null characters") ); } - let name_wide = name.as_wtf8().to_wide_with_nul(); - host_winapi::open_file_mapping_w(desired_access, inherit_handle, name_wide.as_ptr()) + let name_wide = name.as_wtf8().to_wide_cstring(); + host_winapi::open_file_mapping_w(desired_access, inherit_handle, &name_wide) .map(WinHandle) .map_err(|e| e.to_pyexception(vm)) } @@ -1087,15 +1055,8 @@ mod _winapi { _progress_routine: OptionalArg, vm: &VirtualMachine, ) -> PyResult<()> { - let src_wide = widestring::WideCString::from_vec_truncate( - existing_file_name - .as_wtf8() - .encode_wide() - .collect::>(), - ); - let dst_wide = widestring::WideCString::from_vec_truncate( - new_file_name.as_wtf8().encode_wide().collect::>(), - ); + let src_wide = existing_file_name.as_wtf8().to_wide_cstring(); + let dst_wide = new_file_name.as_wtf8().to_wide_cstring(); host_winapi::copy_file2(&src_wide, &dst_wide, flags).map_err(|e| e.to_pyexception(vm)) } From 25a9d4026aae9b526d0a5f8c788180090b29897b Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sat, 16 May 2026 20:35:07 +0900 Subject: [PATCH 03/23] Migrate winreg pub unsafe fn string parameters to typed references --- crates/host_env/src/winreg.rs | 70 ++++++++++++++------------ crates/vm/src/stdlib/winreg.rs | 92 ++++++++++++---------------------- 2 files changed, 72 insertions(+), 90 deletions(-) diff --git a/crates/host_env/src/winreg.rs b/crates/host_env/src/winreg.rs index fa95b066b1d..5324ac258e2 100644 --- a/crates/host_env/src/winreg.rs +++ b/crates/host_env/src/winreg.rs @@ -101,24 +101,25 @@ pub fn close_key(hkey: Registry::HKEY) -> u32 { } pub unsafe fn connect_registry( - computer_name: *const u16, + computer_name: Option<&widestring::WideCStr>, key: Registry::HKEY, out_key: *mut Registry::HKEY, ) -> u32 { - unsafe { Registry::RegConnectRegistryW(computer_name, key, out_key) } + let name_ptr = computer_name.map_or(core::ptr::null(), |n| n.as_ptr()); + unsafe { Registry::RegConnectRegistryW(name_ptr, key, out_key) } } pub unsafe fn create_key( key: Registry::HKEY, - sub_key: *const u16, + sub_key: &widestring::WideCStr, out_key: *mut Registry::HKEY, ) -> u32 { - unsafe { Registry::RegCreateKeyW(key, sub_key, out_key) } + unsafe { Registry::RegCreateKeyW(key, sub_key.as_ptr(), out_key) } } pub unsafe fn create_key_ex( key: Registry::HKEY, - sub_key: *const u16, + sub_key: &widestring::WideCStr, reserved: u32, class: *mut u16, options: u32, @@ -130,7 +131,7 @@ pub unsafe fn create_key_ex( unsafe { Registry::RegCreateKeyExW( key, - sub_key, + sub_key.as_ptr(), reserved, class, options, @@ -142,21 +143,22 @@ pub unsafe fn create_key_ex( } } -pub unsafe fn delete_key(key: Registry::HKEY, sub_key: *const u16) -> u32 { - unsafe { Registry::RegDeleteKeyW(key, sub_key) } +pub unsafe fn delete_key(key: Registry::HKEY, sub_key: &widestring::WideCStr) -> u32 { + unsafe { Registry::RegDeleteKeyW(key, sub_key.as_ptr()) } } pub unsafe fn delete_key_ex( key: Registry::HKEY, - sub_key: *const u16, + sub_key: &widestring::WideCStr, sam: u32, reserved: u32, ) -> u32 { - unsafe { Registry::RegDeleteKeyExW(key, sub_key, sam, reserved) } + unsafe { Registry::RegDeleteKeyExW(key, sub_key.as_ptr(), sam, reserved) } } -pub unsafe fn delete_value(key: Registry::HKEY, value_name: *const u16) -> u32 { - unsafe { Registry::RegDeleteValueW(key, value_name) } +pub unsafe fn delete_value(key: Registry::HKEY, value_name: Option<&widestring::WideCStr>) -> u32 { + let name_ptr = value_name.map_or(core::ptr::null(), |n| n.as_ptr()); + unsafe { Registry::RegDeleteValueW(key, name_ptr) } } pub unsafe fn enum_key_ex( @@ -268,31 +270,36 @@ pub fn flush_key(key: Registry::HKEY) -> u32 { unsafe { Registry::RegFlushKey(key) } } -pub unsafe fn load_key(key: Registry::HKEY, sub_key: *const u16, file_name: *const u16) -> u32 { - unsafe { Registry::RegLoadKeyW(key, sub_key, file_name) } +pub unsafe fn load_key( + key: Registry::HKEY, + sub_key: &widestring::WideCStr, + file_name: &widestring::WideCStr, +) -> u32 { + unsafe { Registry::RegLoadKeyW(key, sub_key.as_ptr(), file_name.as_ptr()) } } pub unsafe fn open_key_ex( key: Registry::HKEY, - sub_key: *const u16, + sub_key: &widestring::WideCStr, options: u32, sam: u32, out_key: *mut Registry::HKEY, ) -> u32 { - unsafe { Registry::RegOpenKeyExW(key, sub_key, options, sam, out_key) } + unsafe { Registry::RegOpenKeyExW(key, sub_key.as_ptr(), options, sam, out_key) } } pub unsafe fn query_value_ex( key: Registry::HKEY, - value_name: *const u16, + value_name: Option<&widestring::WideCStr>, value_type: *mut u32, data: *mut u8, data_len: *mut u32, ) -> u32 { + let name_ptr = value_name.map_or(core::ptr::null(), |n| n.as_ptr()); unsafe { Registry::RegQueryValueExW( key, - value_name, + name_ptr, core::ptr::null_mut(), value_type, data, @@ -301,18 +308,19 @@ pub unsafe fn query_value_ex( } } -pub unsafe fn save_key(key: Registry::HKEY, file_name: *const u16) -> u32 { - unsafe { Registry::RegSaveKeyW(key, file_name, core::ptr::null_mut()) } +pub unsafe fn save_key(key: Registry::HKEY, file_name: &widestring::WideCStr) -> u32 { + unsafe { Registry::RegSaveKeyW(key, file_name.as_ptr(), core::ptr::null_mut()) } } pub unsafe fn set_value_ex( key: Registry::HKEY, - value_name: *const u16, + value_name: Option<&widestring::WideCStr>, typ: u32, ptr: *const u8, len: u32, ) -> u32 { - unsafe { Registry::RegSetValueExW(key, value_name, 0, typ, ptr, len) } + let name_ptr = value_name.map_or(core::ptr::null(), |n| n.as_ptr()); + unsafe { Registry::RegSetValueExW(key, name_ptr, 0, typ, ptr, len) } } pub fn disable_reflection_key(key: Registry::HKEY) -> u32 { @@ -342,12 +350,12 @@ pub fn query_default_value( sub_key: Option<&OsStr>, ) -> Result { let child_key = if let Some(sub_key) = sub_key.filter(|s| !s.is_empty()) { - let wide_sub_key = sub_key.to_wide_with_nul(); + let wide_sub_key = sub_key.to_wide_cstring(); let mut out_key = core::ptr::null_mut(); let res = unsafe { open_key_ex( hkey, - wide_sub_key.as_ptr(), + &wide_sub_key, 0, Registry::KEY_QUERY_VALUE, &mut out_key, @@ -371,7 +379,7 @@ pub fn query_default_value( let res = unsafe { query_value_ex( target_key, - core::ptr::null(), + None, &mut reg_type, buffer.as_mut_ptr(), &mut size, @@ -408,12 +416,12 @@ pub fn query_default_value( } pub fn query_value_bytes(hkey: Registry::HKEY, value_name: &OsStr) -> Result<(Vec, u32), u32> { - let wide_name = value_name.to_wide_with_nul(); + let wide_name = value_name.to_wide_cstring(); let mut buf_size: u32 = 0; let res = unsafe { query_value_ex( hkey, - wide_name.as_ptr(), + Some(&wide_name), core::ptr::null_mut(), core::ptr::null_mut(), &mut buf_size, @@ -433,7 +441,7 @@ pub fn query_value_bytes(hkey: Registry::HKEY, value_name: &OsStr) -> Result<(Ve let res = unsafe { query_value_ex( hkey, - wide_name.as_ptr(), + Some(&wide_name), &mut typ, ret_buf.as_mut_ptr(), &mut ret_size, @@ -453,12 +461,12 @@ pub fn query_value_bytes(hkey: Registry::HKEY, value_name: &OsStr) -> Result<(Ve pub fn set_default_value(hkey: Registry::HKEY, sub_key: &OsStr, typ: u32, value: &OsStr) -> u32 { let child_key = if !sub_key.is_empty() { - let wide_sub_key = sub_key.to_wide_with_nul(); + let wide_sub_key = sub_key.to_wide_cstring(); let mut out_key = core::ptr::null_mut(); let res = unsafe { create_key_ex( hkey, - wide_sub_key.as_ptr(), + &wide_sub_key, 0, core::ptr::null_mut(), 0, @@ -481,7 +489,7 @@ pub fn set_default_value(hkey: Registry::HKEY, sub_key: &OsStr, typ: u32, value: let res = unsafe { set_value_ex( target_key, - core::ptr::null(), + None, typ, wide_value.as_ptr() as *const u8, (wide_value.len() * 2) as u32, diff --git a/crates/vm/src/stdlib/winreg.rs b/crates/vm/src/stdlib/winreg.rs index fcaaf927a72..f87512cf721 100644 --- a/crates/vm/src/stdlib/winreg.rs +++ b/crates/vm/src/stdlib/winreg.rs @@ -256,41 +256,27 @@ mod winreg { key: PyRef, vm: &VirtualMachine, ) -> PyResult { - if let Some(computer_name) = computer_name { - let mut ret_key = core::ptr::null_mut(); - let wide_computer_name = computer_name.to_wide_with_nul(); - let res = unsafe { - host_winreg::connect_registry( - wide_computer_name.as_ptr(), - key.hkey.load(), - &mut ret_key, - ) - }; - if res == 0 { - Ok(PyHkey::new(ret_key)) - } else { - Err(vm.new_os_error(format!("error code: {res}"))) - } + let wide_computer_name = computer_name.map(|n| n.to_wide_cstring()); + let mut ret_key = core::ptr::null_mut(); + let res = unsafe { + host_winreg::connect_registry( + wide_computer_name.as_deref(), + key.hkey.load(), + &mut ret_key, + ) + }; + if res == 0 { + Ok(PyHkey::new(ret_key)) } else { - let mut ret_key = core::ptr::null_mut(); - let res = unsafe { - host_winreg::connect_registry(core::ptr::null_mut(), key.hkey.load(), &mut ret_key) - }; - if res == 0 { - Ok(PyHkey::new(ret_key)) - } else { - Err(vm.new_os_error(format!("error code: {res}"))) - } + Err(vm.new_os_error(format!("error code: {res}"))) } } #[pyfunction] fn CreateKey(key: PyRef, sub_key: String, vm: &VirtualMachine) -> PyResult { - let wide_sub_key = sub_key.to_wide_with_nul(); + let wide_sub_key = sub_key.to_wide_cstring(); let mut out_key = core::ptr::null_mut(); - let res = unsafe { - host_winreg::create_key(key.hkey.load(), wide_sub_key.as_ptr(), &mut out_key) - }; + let res = unsafe { host_winreg::create_key(key.hkey.load(), &wide_sub_key, &mut out_key) }; if res == 0 { Ok(PyHkey::new(out_key)) } else { @@ -312,13 +298,13 @@ mod winreg { #[pyfunction] fn CreateKeyEx(args: CreateKeyExArgs, vm: &VirtualMachine) -> PyResult { - let wide_sub_key = args.sub_key.to_wide_with_nul(); + let wide_sub_key = args.sub_key.to_wide_cstring(); let mut res: host_winreg::HKEY = core::ptr::null_mut(); let err = unsafe { let key = args.key.hkey.load(); host_winreg::create_key_ex( key, - wide_sub_key.as_ptr(), + &wide_sub_key, args.reserved, core::ptr::null_mut(), host_winreg::REG_OPTION_NON_VOLATILE, @@ -345,8 +331,8 @@ mod winreg { #[pyfunction] fn DeleteKey(key: PyRef, sub_key: String, vm: &VirtualMachine) -> PyResult<()> { - let wide_sub_key = sub_key.to_wide_with_nul(); - let res = unsafe { host_winreg::delete_key(key.hkey.load(), wide_sub_key.as_ptr()) }; + let wide_sub_key = sub_key.to_wide_cstring(); + let res = unsafe { host_winreg::delete_key(key.hkey.load(), &wide_sub_key) }; if res == 0 { Ok(()) } else { @@ -356,11 +342,8 @@ mod winreg { #[pyfunction] fn DeleteValue(key: PyRef, value: Option, vm: &VirtualMachine) -> PyResult<()> { - let wide_value = value.map(|v| v.to_wide_with_nul()); - let value_ptr = wide_value - .as_ref() - .map_or(core::ptr::null(), |v| v.as_ptr()); - let res = unsafe { host_winreg::delete_value(key.hkey.load(), value_ptr) }; + let wide_value = value.map(|v| v.to_wide_cstring()); + let res = unsafe { host_winreg::delete_value(key.hkey.load(), wide_value.as_deref()) }; if res == 0 { Ok(()) } else { @@ -382,11 +365,11 @@ mod winreg { #[pyfunction] fn DeleteKeyEx(args: DeleteKeyExArgs, vm: &VirtualMachine) -> PyResult<()> { - let wide_sub_key = args.sub_key.to_wide_with_nul(); + let wide_sub_key = args.sub_key.to_wide_cstring(); let res = unsafe { host_winreg::delete_key_ex( args.key.hkey.load(), - wide_sub_key.as_ptr(), + &wide_sub_key, args.access, args.reserved, ) @@ -519,10 +502,9 @@ mod winreg { file_name: String, vm: &VirtualMachine, ) -> PyResult<()> { - let sub_key = sub_key.to_wide_with_nul(); - let file_name = file_name.to_wide_with_nul(); - let res = - unsafe { host_winreg::load_key(key.hkey.load(), sub_key.as_ptr(), file_name.as_ptr()) }; + let sub_key = sub_key.to_wide_cstring(); + let file_name = file_name.to_wide_cstring(); + let res = unsafe { host_winreg::load_key(key.hkey.load(), &sub_key, &file_name) }; if res == 0 { Ok(()) } else { @@ -545,17 +527,11 @@ mod winreg { #[pyfunction] #[pyfunction(name = "OpenKeyEx")] fn OpenKey(args: OpenKeyArgs, vm: &VirtualMachine) -> PyResult { - let wide_sub_key = args.sub_key.to_wide_with_nul(); + let wide_sub_key = args.sub_key.to_wide_cstring(); let mut res: host_winreg::HKEY = core::ptr::null_mut(); let err = unsafe { let key = args.key.hkey.load(); - host_winreg::open_key_ex( - key, - wide_sub_key.as_ptr(), - args.reserved, - args.access, - &mut res, - ) + host_winreg::open_key_ex(key, &wide_sub_key, args.reserved, args.access, &mut res) }; if err == 0 { Ok(PyHkey { @@ -614,8 +590,8 @@ mod winreg { #[pyfunction] fn SaveKey(key: PyRef, file_name: String, vm: &VirtualMachine) -> PyResult<()> { - let file_name = file_name.to_wide_with_nul(); - let res = unsafe { host_winreg::save_key(key.hkey.load(), file_name.as_ptr()) }; + let file_name = file_name.to_wide_cstring(); + let res = unsafe { host_winreg::save_key(key.hkey.load(), &file_name) }; if res == 0 { Ok(()) } else { @@ -825,17 +801,15 @@ mod winreg { value: PyObjectRef, vm: &VirtualMachine, ) -> PyResult<()> { - let wide_value_name = value_name.as_deref().map(|s| s.to_wide_with_nul()); - let value_name_ptr = wide_value_name - .as_deref() - .map_or(core::ptr::null(), |s| s.as_ptr()); + let wide_value_name = value_name.as_deref().map(|s| s.to_wide_cstring()); let reg_value = py2reg(value, typ, vm)?; let (ptr, len) = match ®_value { Some(v) => (v.as_ptr(), v.len() as u32), None => (core::ptr::null(), 0), }; - let res = - unsafe { host_winreg::set_value_ex(key.hkey.load(), value_name_ptr, typ, ptr, len) }; + let res = unsafe { + host_winreg::set_value_ex(key.hkey.load(), wide_value_name.as_deref(), typ, ptr, len) + }; if res != 0 { return Err(os_error_from_windows_code(vm, res as i32)); } From 0acdcadf4bba25ea8f758d0c147d5c0b483b7f6a Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sat, 16 May 2026 21:08:32 +0900 Subject: [PATCH 04/23] Add ToPyException impls for host_env error types (PyPy wrap_oserror analog) --- crates/host_env/src/wmi.rs | 2 +- crates/stdlib/src/fcntl.rs | 8 +- crates/stdlib/src/multiprocessing.rs | 10 +- crates/stdlib/src/socket.rs | 16 +-- crates/vm/src/exceptions.rs | 153 +++++++++++++++++++++++++++ crates/vm/src/stdlib/_io.rs | 29 +---- crates/vm/src/stdlib/_winapi.rs | 21 +--- crates/vm/src/stdlib/_wmi.rs | 12 +-- crates/vm/src/stdlib/nt.rs | 7 +- crates/vm/src/stdlib/posix.rs | 13 +-- crates/vm/src/stdlib/time.rs | 37 ++----- crates/vm/src/stdlib/winreg.rs | 22 +--- 12 files changed, 185 insertions(+), 145 deletions(-) diff --git a/crates/host_env/src/wmi.rs b/crates/host_env/src/wmi.rs index 1165a756306..a6d77f77e9f 100644 --- a/crates/host_env/src/wmi.rs +++ b/crates/host_env/src/wmi.rs @@ -17,7 +17,7 @@ use windows_sys::Win32::System::Threading::{ CreateEventW, CreateThread, GetExitCodeThread, SetEvent, WaitForSingleObject, }; -const BUFFER_SIZE: usize = 8192; +pub const BUFFER_SIZE: usize = 8192; pub enum ExecQueryError { MoreData, diff --git a/crates/stdlib/src/fcntl.rs b/crates/stdlib/src/fcntl.rs index c118ac99e51..5081c9e9c14 100644 --- a/crates/stdlib/src/fcntl.rs +++ b/crates/stdlib/src/fcntl.rs @@ -9,6 +9,7 @@ mod fcntl { use crate::vm::{ PyResult, VirtualMachine, builtins::PyIntRef, + convert::ToPyException, function::{ArgMemoryBuffer, ArgStrOrBytesLike, Either, OptionalArg}, stdlib::_io, }; @@ -167,11 +168,8 @@ mod fcntl { OptionalArg::Present(w) => w, OptionalArg::Missing => 0, }; - let ret = host_fcntl::lockf(fd, cmd, len, start, whence).map_err(|err| match err { - host_fcntl::LockfError::InvalidCmd => vm.new_value_error("unrecognized lockf argument"), - host_fcntl::LockfError::Overflow(e) => vm.new_overflow_error(e), - host_fcntl::LockfError::Io(_) => vm.new_last_errno_error(), - })?; + let ret = + host_fcntl::lockf(fd, cmd, len, start, whence).map_err(|err| err.to_pyexception(vm))?; Ok(vm.ctx.new_int(ret).into()) } } diff --git a/crates/stdlib/src/multiprocessing.rs b/crates/stdlib/src/multiprocessing.rs index 475ed7f6c16..53f6692577e 100644 --- a/crates/stdlib/src/multiprocessing.rs +++ b/crates/stdlib/src/multiprocessing.rs @@ -350,6 +350,7 @@ mod _multiprocessing { use crate::vm::{ Context, FromArgs, Py, PyPayload, PyRef, PyResult, VirtualMachine, builtins::{PyBaseExceptionRef, PyDict, PyType, PyTypeRef}, + convert::ToPyException, function::{FuncArgs, KwArgs}, types::Constructor, }; @@ -872,14 +873,7 @@ mod _multiprocessing { } fn os_error(vm: &VirtualMachine, err: SemError) -> PyBaseExceptionRef { - // _PyMp_SetError maps to PyErr_SetFromErrno - let exc_type = match err { - SemError::AlreadyExists => vm.ctx.exceptions.file_exists_error.to_owned(), - SemError::NotFound => vm.ctx.exceptions.file_not_found_error.to_owned(), - _ => vm.ctx.exceptions.os_error.to_owned(), - }; - vm.new_os_subtype_error(exc_type, Some(err.raw_os_error()), err.description()) - .upcast() + err.to_pyexception(vm) } } diff --git a/crates/stdlib/src/socket.rs b/crates/stdlib/src/socket.rs index 4193350776e..f1ba092ab08 100644 --- a/crates/stdlib/src/socket.rs +++ b/crates/stdlib/src/socket.rs @@ -14,7 +14,9 @@ mod _socket { PyBaseExceptionRef, PyListRef, PyModule, PyOSError, PyStrRef, PyTupleRef, PyTypeRef, PyUtf8StrRef, }, - convert::{IntoPyException, ToPyObject, TryFromBorrowedObject, TryFromObject}, + convert::{ + IntoPyException, ToPyException, ToPyObject, TryFromBorrowedObject, TryFromObject, + }, function::{ ArgBytesLike, ArgIntoFloat, ArgMemoryBuffer, ArgStrOrBytesLike, Either, FsPath, OptionalArg, OptionalOption, @@ -1947,17 +1949,7 @@ mod _socket { .map(|(lvl, typ, data)| (*lvl, *typ, data.as_slice())) .collect::>(); - host_socket::pack_ancillary_messages(&data_refs).map_err(|err| match err { - host_socket::AncillaryPackError::ItemTooLarge => { - vm.new_os_error("ancillary data item too large".to_owned()) - } - host_socket::AncillaryPackError::TooMuchData => { - vm.new_os_error("too much ancillary data".to_owned()) - } - host_socket::AncillaryPackError::UnexpectedNullHeader => { - vm.new_runtime_error("unexpected NULL result from CMSG_FIRSTHDR/CMSG_NXTHDR") - } - }) + host_socket::pack_ancillary_messages(&data_refs).map_err(|err| err.to_pyexception(vm)) } #[pymethod] diff --git a/crates/vm/src/exceptions.rs b/crates/vm/src/exceptions.rs index 1adc5f0a1b1..7f1b3cc7a14 100644 --- a/crates/vm/src/exceptions.rs +++ b/crates/vm/src/exceptions.rs @@ -1440,6 +1440,159 @@ impl IntoPyException for rustix::io::Errno { } } +#[cfg(not(any(target_os = "wasi", target_os = "redox")))] +impl ToPyException for rustpython_host_env::fcntl::LockfError { + fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { + match self { + Self::InvalidCmd => vm.new_value_error("unrecognized lockf argument"), + Self::Overflow(e) => vm.new_overflow_error(e.clone()), + Self::Io(err) => err.to_pyexception(vm), + } + } +} + +#[cfg(unix)] +impl ToPyException for rustpython_host_env::posix::AccessError { + fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { + match self { + Self::InvalidMode => vm.new_value_error( + "One of the flags is wrong, there are only 4 possibilities F_OK, R_OK, W_OK and X_OK", + ), + Self::Os(errno) => std::io::Error::from_raw_os_error(*errno).to_pyexception(vm), + } + } +} + +impl ToPyException for rustpython_host_env::socket::AncillaryPackError { + fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { + match self { + Self::ItemTooLarge => vm.new_os_error("ancillary data item too large"), + Self::TooMuchData => vm.new_os_error("too much ancillary data"), + Self::UnexpectedNullHeader => { + vm.new_runtime_error("unexpected NULL result from CMSG_FIRSTHDR/CMSG_NXTHDR") + } + } + } +} + +impl ToPyException for rustpython_host_env::time::CheckedTmError { + fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { + match self { + Self::YearOutOfRange => vm.new_overflow_error("year out of range"), + Self::MonthOutOfRange => vm.new_value_error("month out of range"), + Self::DayOfMonthOutOfRange => vm.new_value_error("day of month out of range"), + Self::HourOutOfRange => vm.new_value_error("hour out of range"), + Self::MinuteOutOfRange => vm.new_value_error("minute out of range"), + Self::SecondsOutOfRange => vm.new_value_error("seconds out of range"), + Self::DayOfWeekOutOfRange => vm.new_value_error("day of week out of range"), + Self::DayOfYearOutOfRange => vm.new_value_error("day of year out of range"), + Self::EmbeddedNul => vm.new_value_error("embedded null character"), + } + } +} + +#[cfg(windows)] +impl ToPyException for rustpython_host_env::winapi::BuildEnvironmentBlockError { + fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { + match self { + Self::ContainsNul => vm.new_value_error("embedded null character"), + Self::IllegalName => vm.new_value_error("illegal environment variable name"), + } + } +} + +#[cfg(windows)] +impl ToPyException for rustpython_host_env::winapi::BatchedWaitError { + fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { + match self { + Self::Timeout => vm + .new_os_subtype_error( + vm.ctx.exceptions.timeout_error.to_owned(), + None, + "timed out", + ) + .upcast(), + Self::Interrupted => vm + .new_errno_error(libc::EINTR, "Interrupted system call") + .upcast(), + Self::Os(err) => vm.new_os_error(*err as i32), + } + } +} + +#[cfg(windows)] +impl ToPyException for rustpython_host_env::nt::ReadlinkError { + fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { + match self { + Self::Io(err) => err.to_pyexception(vm), + Self::NotSymbolicLink => vm.new_value_error("not a symbolic link"), + Self::InvalidReparseData => vm.new_os_error("Invalid reparse data"), + } + } +} + +#[cfg(windows)] +impl ToPyException for rustpython_host_env::nt::ReadConsoleError { + fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { + match self { + Self::Io(err) => err.to_pyexception(vm), + Self::BufferTooSmall { + available, + required, + } => vm.new_system_error(format!( + "Buffer had room for {available} bytes but {required} bytes required", + )), + } + } +} + +#[cfg(windows)] +impl ToPyException for rustpython_host_env::winreg::ExpandEnvironmentStringsError { + fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { + match self { + Self::Os => vm.new_os_error("ExpandEnvironmentStringsW failed"), + Self::Utf16(e) => vm.new_value_error(format!("UTF16 error: {e}")), + } + } +} + +#[cfg(windows)] +impl ToPyException for rustpython_host_env::winreg::QueryStringError { + fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { + match self { + Self::Code(err) => std::io::Error::from_raw_os_error(*err as i32).to_pyexception(vm), + Self::Utf16(e) => vm.new_value_error(format!("UTF16 error: {e}")), + } + } +} + +#[cfg(windows)] +impl ToPyException for rustpython_host_env::wmi::ExecQueryError { + fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { + match self { + Self::MoreData => vm.new_os_error(format!( + "Query returns more than {} characters", + rustpython_host_env::wmi::BUFFER_SIZE + )), + Self::Code(err) => std::io::Error::from_raw_os_error(*err as i32).to_pyexception(vm), + } + } +} + +#[cfg(unix)] +impl ToPyException for rustpython_host_env::multiprocessing::SemError { + fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { + let excs = &vm.ctx.exceptions; + let exc_type = match self { + Self::AlreadyExists => excs.file_exists_error.to_owned(), + Self::NotFound => excs.file_not_found_error.to_owned(), + _ => excs.os_error.to_owned(), + }; + vm.new_os_subtype_error(exc_type, Some(self.raw_os_error()), self.description()) + .upcast() + } +} + pub(super) mod types { use crate::common::lock::PyRwLock; use crate::object::{MaybeTraverse, Traverse, TraverseFn}; diff --git a/crates/vm/src/stdlib/_io.rs b/crates/vm/src/stdlib/_io.rs index 0c846fee2ac..edebe5cec2f 100644 --- a/crates/vm/src/stdlib/_io.rs +++ b/crates/vm/src/stdlib/_io.rs @@ -6239,16 +6239,8 @@ mod winconsoleio { let dest = &mut *buf_ref; let mut smallbuf = self.buf.lock(); - match host_nt::read_console_into(handle, dest, &mut smallbuf) { - Ok(read_len) => Ok(read_len), - Err(host_nt::ReadConsoleError::BufferTooSmall { - available, - required, - }) => Err(vm.new_system_error(format!( - "Buffer had room for {available} bytes but {required} bytes required", - ))), - Err(host_nt::ReadConsoleError::Io(err)) => Err(err.into_pyexception(vm)), - } + host_nt::read_console_into(handle, dest, &mut smallbuf) + .map_err(|err| err.to_pyexception(vm)) } #[pymethod] @@ -6302,20 +6294,9 @@ mod winconsoleio { } { let mut ibuf = self.buf.lock(); - match host_nt::read_console_into(handle, &mut buf[read_len..], &mut ibuf) { - Ok(n) => read_len += n, - Err(host_nt::ReadConsoleError::BufferTooSmall { - available, - required, - }) => { - return Err(vm.new_system_error(format!( - "Buffer had room for {available} bytes but {required} bytes required", - ))); - } - Err(host_nt::ReadConsoleError::Io(err)) => { - return Err(err.into_pyexception(vm)); - } - } + let n = host_nt::read_console_into(handle, &mut buf[read_len..], &mut ibuf) + .map_err(|err| err.to_pyexception(vm))?; + read_len += n; } buf.truncate(read_len); diff --git a/crates/vm/src/stdlib/_winapi.rs b/crates/vm/src/stdlib/_winapi.rs index 979c763a1f5..6f7a338f26e 100644 --- a/crates/vm/src/stdlib/_winapi.rs +++ b/crates/vm/src/stdlib/_winapi.rs @@ -327,14 +327,7 @@ mod _winapi { entries.push((k, v)); } - host_winapi::build_environment_block(entries).map_err(|err| match err { - host_winapi::BuildEnvironmentBlockError::ContainsNul => { - crate::exceptions::cstring_error(vm) - } - host_winapi::BuildEnvironmentBlockError::IllegalName => { - vm.new_value_error("illegal environment variable name") - } - }) + host_winapi::build_environment_block(entries).map_err(|err| err.to_pyexception(vm)) } fn get_handle_list(obj: PyObjectRef, vm: &VirtualMachine) -> PyResult>> { @@ -951,17 +944,7 @@ mod _winapi { .collect(), ) .into()), - Err(host_winapi::BatchedWaitError::Timeout) => Err(vm - .new_os_subtype_error( - vm.ctx.exceptions.timeout_error.to_owned(), - None, - "timed out", - ) - .upcast()), - Err(host_winapi::BatchedWaitError::Interrupted) => Err(vm - .new_errno_error(libc::EINTR, "Interrupted system call") - .upcast()), - Err(host_winapi::BatchedWaitError::Os(err)) => Err(vm.new_os_error(err as i32)), + Err(err) => Err(err.to_pyexception(vm)), } } diff --git a/crates/vm/src/stdlib/_wmi.rs b/crates/vm/src/stdlib/_wmi.rs index d0f23ac6914..e1549ef12d0 100644 --- a/crates/vm/src/stdlib/_wmi.rs +++ b/crates/vm/src/stdlib/_wmi.rs @@ -10,8 +10,6 @@ mod _wmi { use crate::{PyResult, VirtualMachine}; use rustpython_host_env::wmi as host_wmi; - const BUFFER_SIZE: usize = 8192; - #[pyfunction] fn exec_query(query: PyStrRef, vm: &VirtualMachine) -> PyResult { let query_str = query.expect_str(); @@ -23,14 +21,6 @@ mod _wmi { return Err(vm.new_value_error("only SELECT queries are supported")); } - match host_wmi::exec_query(query_str) { - Ok(result) => Ok(result), - Err(host_wmi::ExecQueryError::MoreData) => { - Err(vm.new_os_error(format!("Query returns more than {BUFFER_SIZE} characters"))) - } - Err(host_wmi::ExecQueryError::Code(err)) => { - Err(std::io::Error::from_raw_os_error(err as i32).to_pyexception(vm)) - } - } + host_wmi::exec_query(query_str).map_err(|err| err.to_pyexception(vm)) } } diff --git a/crates/vm/src/stdlib/nt.rs b/crates/vm/src/stdlib/nt.rs index 4c2c560476b..3303b1c67e2 100644 --- a/crates/vm/src/stdlib/nt.rs +++ b/crates/vm/src/stdlib/nt.rs @@ -1072,12 +1072,7 @@ pub(crate) mod module { Err(host_nt::ReadlinkError::Io(err)) => { Err(OSErrorBuilder::with_filename(&err, path.clone(), vm)) } - Err(host_nt::ReadlinkError::NotSymbolicLink) => { - Err(vm.new_value_error("not a symbolic link")) - } - Err(host_nt::ReadlinkError::InvalidReparseData) => { - Err(vm.new_os_error("Invalid reparse data".to_owned())) - } + Err(err) => Err(err.to_pyexception(vm)), } } diff --git a/crates/vm/src/stdlib/posix.rs b/crates/vm/src/stdlib/posix.rs index 5532c06c67f..51723e59d6c 100644 --- a/crates/vm/src/stdlib/posix.rs +++ b/crates/vm/src/stdlib/posix.rs @@ -18,7 +18,7 @@ pub mod module { use crate::{ AsObject, Py, PyObjectRef, PyResult, VirtualMachine, builtins::{PyDictRef, PyInt, PyListRef, PyTupleRef, PyUtf8Str}, - convert::{IntoPyException, ToPyObject, TryFromObject}, + convert::{IntoPyException, ToPyException, ToPyObject, TryFromObject}, exceptions::OSErrorBuilder, function::{ArgMapping, Either, KwArgs, OptionalArg}, ospath::{OsPath, OsPathOrFd}, @@ -380,15 +380,8 @@ pub mod module { #[pyfunction] pub(super) fn access(path: OsPath, mode: u8, vm: &VirtualMachine) -> PyResult { - match rustpython_host_env::posix::check_access(path.as_ref(), mode) { - Ok(ok) => Ok(ok), - Err(rustpython_host_env::posix::AccessError::InvalidMode) => Err(vm.new_value_error( - "One of the flags is wrong, there are only 4 possibilities F_OK, R_OK, W_OK and X_OK", - )), - Err(rustpython_host_env::posix::AccessError::Os(err)) => { - Err(io::Error::from_raw_os_error(err).into_pyexception(vm)) - } - } + rustpython_host_env::posix::check_access(path.as_ref(), mode) + .map_err(|err| err.to_pyexception(vm)) } #[pyattr] diff --git a/crates/vm/src/stdlib/time.rs b/crates/vm/src/stdlib/time.rs index 80a832d37f1..e2cc8536d88 100644 --- a/crates/vm/src/stdlib/time.rs +++ b/crates/vm/src/stdlib/time.rs @@ -36,7 +36,10 @@ mod decl { types::{PyStructSequence, struct_sequence_new}, }; #[cfg(any(unix, windows))] - use crate::{common::wtf8::Wtf8Buf, convert::ToPyObject}; + use crate::{ + common::wtf8::Wtf8Buf, + convert::{ToPyException, ToPyObject}, + }; #[cfg(not(any(unix, windows)))] use chrono::{ DateTime, Datelike, TimeZone, Timelike, @@ -411,7 +414,7 @@ mod decl { zone, gmtoff, }) - .map_err(|err| map_checked_tm_error(vm, err)) + .map_err(|err| err.to_pyexception(vm)) } #[cfg(windows)] { @@ -426,35 +429,7 @@ mod decl { tm_yday, tm_isdst, }) - .map_err(|err| map_checked_tm_error(vm, err)) - } - } - - #[cfg(any(unix, windows))] - fn map_checked_tm_error( - vm: &VirtualMachine, - err: host_time::CheckedTmError, - ) -> PyBaseExceptionRef { - match err { - host_time::CheckedTmError::YearOutOfRange => vm.new_overflow_error("year out of range"), - host_time::CheckedTmError::MonthOutOfRange => vm.new_value_error("month out of range"), - host_time::CheckedTmError::DayOfMonthOutOfRange => { - vm.new_value_error("day of month out of range") - } - host_time::CheckedTmError::HourOutOfRange => vm.new_value_error("hour out of range"), - host_time::CheckedTmError::MinuteOutOfRange => { - vm.new_value_error("minute out of range") - } - host_time::CheckedTmError::SecondsOutOfRange => { - vm.new_value_error("seconds out of range") - } - host_time::CheckedTmError::DayOfWeekOutOfRange => { - vm.new_value_error("day of week out of range") - } - host_time::CheckedTmError::DayOfYearOutOfRange => { - vm.new_value_error("day of year out of range") - } - host_time::CheckedTmError::EmbeddedNul => vm.new_value_error("embedded null character"), + .map_err(|err| err.to_pyexception(vm)) } } diff --git a/crates/vm/src/stdlib/winreg.rs b/crates/vm/src/stdlib/winreg.rs index f87512cf721..7aed53ab882 100644 --- a/crates/vm/src/stdlib/winreg.rs +++ b/crates/vm/src/stdlib/winreg.rs @@ -7,7 +7,7 @@ pub(crate) use winreg::module_def; mod winreg { use crate::builtins::{PyInt, PyStr, PyTuple, PyTypeRef}; use crate::common::hash::PyHash; - use crate::convert::TryFromObject; + use crate::convert::{ToPyException, TryFromObject}; use crate::function::FuncArgs; use crate::host_env::windows::ToWideString; use crate::object::AsObject; @@ -27,7 +27,6 @@ mod winreg { vm: &VirtualMachine, code: i32, ) -> crate::PyRef { - use crate::convert::ToPyException; std::io::Error::from_raw_os_error(code).to_pyexception(vm) } @@ -568,14 +567,7 @@ mod winreg { } host_winreg::query_default_value(hkey, sub_key.as_deref().map(std::ffi::OsStr::new)) - .map_err(|err| match err { - host_winreg::QueryStringError::Code(err) => { - os_error_from_windows_code(vm, err as i32) - } - host_winreg::QueryStringError::Utf16(e) => { - vm.new_value_error(format!("UTF16 error: {e}")) - } - }) + .map_err(|err| err.to_pyexception(vm)) } #[pyfunction] @@ -849,13 +841,7 @@ mod winreg { #[pyfunction] fn ExpandEnvironmentStrings(i: String, vm: &VirtualMachine) -> PyResult { - host_winreg::expand_environment_strings(std::ffi::OsStr::new(&i)).map_err(|err| match err { - host_winreg::ExpandEnvironmentStringsError::Os => { - vm.new_os_error("ExpandEnvironmentStringsW failed".to_string()) - } - host_winreg::ExpandEnvironmentStringsError::Utf16(e) => { - vm.new_value_error(format!("UTF16 error: {e}")) - } - }) + host_winreg::expand_environment_strings(std::ffi::OsStr::new(&i)) + .map_err(|err| err.to_pyexception(vm)) } } From dc57418c921d0f60a879ec6ef91f3a9356509ae5 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sat, 16 May 2026 21:15:50 +0900 Subject: [PATCH 05/23] Add CheckLibcResult helper and apply to socket/fcntl/shm/posix_wasi --- crates/host_env/src/fcntl.rs | 46 +++++------------------ crates/host_env/src/os.rs | 24 ++++++++++++ crates/host_env/src/posix_wasi.rs | 48 ++++++++---------------- crates/host_env/src/shm.rs | 17 +++------ crates/host_env/src/socket.rs | 62 +++++++++++-------------------- 5 files changed, 75 insertions(+), 122 deletions(-) diff --git a/crates/host_env/src/fcntl.rs b/crates/host_env/src/fcntl.rs index f8881a46456..2467a8727bc 100644 --- a/crates/host_env/src/fcntl.rs +++ b/crates/host_env/src/fcntl.rs @@ -3,17 +3,14 @@ use std::io; #[cfg(unix)] use std::os::fd::BorrowedFd; +use crate::os::CheckLibcResult; + pub fn normalize_ioctl_request(request: i64) -> libc::c_ulong { (request as u32) as libc::c_ulong } pub fn fcntl_int(fd: i32, cmd: i32, arg: i32) -> io::Result { - let ret = unsafe { libc::fcntl(fd, cmd, arg) }; - if ret < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(ret) - } + unsafe { libc::fcntl(fd, cmd, arg) }.check_libc_neg() } pub fn validate_fd(fd: i32) -> io::Result<()> { @@ -56,12 +53,7 @@ pub fn set_blocking(fd: BorrowedFd<'_>, blocking: bool) -> io::Result<()> { } pub fn fcntl_with_bytes(fd: i32, cmd: i32, arg: &mut [u8]) -> io::Result { - let ret = unsafe { libc::fcntl(fd, cmd, arg.as_mut_ptr()) }; - if ret < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(ret) - } + unsafe { libc::fcntl(fd, cmd, arg.as_mut_ptr()) }.check_libc_neg() } /// # Safety @@ -73,31 +65,16 @@ pub unsafe fn ioctl_ptr( request: libc::c_ulong, arg: *mut libc::c_void, ) -> io::Result { - let ret = unsafe { libc::ioctl(fd, request as _, arg) }; - if ret < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(ret) - } + unsafe { libc::ioctl(fd, request as _, arg) }.check_libc_neg() } pub fn ioctl_int(fd: i32, request: libc::c_ulong, arg: i32) -> io::Result { - let ret = unsafe { libc::ioctl(fd, request as _, arg) }; - if ret < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(ret) - } + unsafe { libc::ioctl(fd, request as _, arg) }.check_libc_neg() } #[cfg(not(any(target_os = "wasi", target_os = "redox")))] pub fn flock(fd: i32, operation: i32) -> io::Result { - let ret = unsafe { libc::flock(fd, operation) }; - if ret < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(ret) - } + unsafe { libc::flock(fd, operation) }.check_libc_neg() } #[cfg(not(any(target_os = "wasi", target_os = "redox")))] @@ -137,7 +114,7 @@ pub fn lockf(fd: i32, cmd: i32, len: i64, start: i64, whence: i32) -> Result Result io::Error { std::io::Error::last_os_error() } +/// Convert a libc-style return value into an `io::Result`. +/// +/// Negative values are treated as errors; errno is read via [`errno_io_error`]. +/// Modeled after PyPy's `rposix.handle_posix_error`. +pub trait CheckLibcResult: Sized { + /// Returns `Ok(self)` if non-negative, otherwise `Err` with the current errno. + fn check_libc_neg(self) -> io::Result; +} + +macro_rules! impl_check_libc_result { + ($($ty:ty),* $(,)?) => { + $( + impl CheckLibcResult for $ty { + #[inline] + fn check_libc_neg(self) -> io::Result { + if self < 0 { Err(errno_io_error()) } else { Ok(self) } + } + } + )* + }; +} + +impl_check_libc_result!(i16, i32, i64, isize); + #[cfg(windows)] pub fn get_errno() -> i32 { unsafe extern "C" { diff --git a/crates/host_env/src/posix_wasi.rs b/crates/host_env/src/posix_wasi.rs index 8650eaa763e..d4eff8c6866 100644 --- a/crates/host_env/src/posix_wasi.rs +++ b/crates/host_env/src/posix_wasi.rs @@ -2,31 +2,21 @@ use alloc::ffi::CString; use core::{ffi::CStr, time::Duration}; use std::{ffi::OsStr, io}; +use crate::os::CheckLibcResult; + pub fn make_dir(path: &CStr, mode: u32) -> io::Result<()> { - let ret = unsafe { libc::mkdir(path.as_ptr(), mode as _) }; - if ret < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + unsafe { libc::mkdir(path.as_ptr(), mode as _) }.check_libc_neg()?; + Ok(()) } pub fn make_dir_at(dir_fd: i32, path: &CStr, mode: u32) -> io::Result<()> { - let ret = unsafe { libc::mkdirat(dir_fd, path.as_ptr(), mode as _) }; - if ret < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + unsafe { libc::mkdirat(dir_fd, path.as_ptr(), mode as _) }.check_libc_neg()?; + Ok(()) } pub fn remove_dir_at(dir_fd: i32, path: &CStr) -> io::Result<()> { - let ret = unsafe { libc::unlinkat(dir_fd, path.as_ptr(), libc::AT_REMOVEDIR) }; - if ret < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + unsafe { libc::unlinkat(dir_fd, path.as_ptr(), libc::AT_REMOVEDIR) }.check_libc_neg()?; + Ok(()) } pub fn stat_path( @@ -48,10 +38,8 @@ pub fn stat_path( } else { libc::AT_SYMLINK_NOFOLLOW }; - let ret = unsafe { libc::fstatat(dir_fd, path.as_ptr(), stat.as_mut_ptr(), flags) }; - if ret < 0 { - return Err(io::Error::last_os_error()); - } + unsafe { libc::fstatat(dir_fd, path.as_ptr(), stat.as_mut_ptr(), flags) } + .check_libc_neg()?; return Ok(Some(unsafe { stat.assume_init() })); } @@ -60,11 +48,8 @@ pub fn stat_path( } else { unsafe { libc::lstat(path.as_ptr(), stat.as_mut_ptr()) } }; - if ret < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(Some(unsafe { stat.assume_init() })) - } + ret.check_libc_neg()?; + Ok(Some(unsafe { stat.assume_init() })) } pub fn stat_fd(fd: crate::crt_fd::Borrowed<'_>) -> io::Result { @@ -83,7 +68,7 @@ pub fn set_file_times_at( tv_nsec: d.subsec_nanos() as _, }; let times = [ts(access), ts(modified)]; - let ret = unsafe { + unsafe { libc::utimensat( dir_fd, path.as_ptr(), @@ -94,10 +79,7 @@ pub fn set_file_times_at( libc::AT_SYMLINK_NOFOLLOW }, ) - }; - if ret < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) } + .check_libc_neg()?; + Ok(()) } diff --git a/crates/host_env/src/shm.rs b/crates/host_env/src/shm.rs index 08a2ae9787c..78e7d3921bc 100644 --- a/crates/host_env/src/shm.rs +++ b/crates/host_env/src/shm.rs @@ -1,23 +1,16 @@ use core::ffi::CStr; use std::io; +use crate::os::CheckLibcResult; + pub fn shm_open(name: &CStr, flags: libc::c_int, mode: libc::c_uint) -> io::Result { #[cfg(target_os = "freebsd")] let mode = mode.try_into().unwrap(); - let fd = unsafe { libc::shm_open(name.as_ptr(), flags, mode) }; - if fd == -1 { - Err(io::Error::last_os_error()) - } else { - Ok(fd) - } + unsafe { libc::shm_open(name.as_ptr(), flags, mode) }.check_libc_neg() } pub fn shm_unlink(name: &CStr) -> io::Result<()> { - let ret = unsafe { libc::shm_unlink(name.as_ptr()) }; - if ret == -1 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + unsafe { libc::shm_unlink(name.as_ptr()) }.check_libc_neg()?; + Ok(()) } diff --git a/crates/host_env/src/socket.rs b/crates/host_env/src/socket.rs index ee8edab2d47..d6f1e078c72 100644 --- a/crates/host_env/src/socket.rs +++ b/crates/host_env/src/socket.rs @@ -1,4 +1,6 @@ #[cfg(unix)] +use crate::os::CheckLibcResult; +#[cfg(unix)] use core::ffi::CStr; #[cfg(unix)] use core::time::Duration; @@ -36,7 +38,7 @@ pub fn close_socket_ignore_connreset(socket: libc::c_int) -> io::Result<()> { pub fn getsockopt_int(fd: libc::c_int, level: i32, name: i32) -> io::Result { let mut flag: libc::c_int = 0; let mut flagsize = core::mem::size_of::() as libc::socklen_t; - let ret = unsafe { + unsafe { libc::getsockopt( fd, level, @@ -44,12 +46,9 @@ pub fn getsockopt_int(fd: libc::c_int, level: i32, name: i32) -> io::Result &mut flag as *mut libc::c_int as *mut _, &mut flagsize, ) - }; - if ret < 0 { - Err(crate::os::errno_io_error()) - } else { - Ok(flag) } + .check_libc_neg()?; + Ok(flag) } #[cfg(unix)] @@ -61,18 +60,15 @@ pub fn getsockopt_bytes( ) -> io::Result> { let mut buf = vec![0u8; buflen]; let mut optlen = buflen as libc::socklen_t; - let ret = unsafe { libc::getsockopt(fd, level, name, buf.as_mut_ptr() as *mut _, &mut optlen) }; - if ret < 0 { - Err(crate::os::errno_io_error()) - } else { - buf.truncate(optlen as usize); - Ok(buf) - } + unsafe { libc::getsockopt(fd, level, name, buf.as_mut_ptr() as *mut _, &mut optlen) } + .check_libc_neg()?; + buf.truncate(optlen as usize); + Ok(buf) } #[cfg(unix)] pub fn setsockopt_bytes(fd: libc::c_int, level: i32, name: i32, value: &[u8]) -> io::Result<()> { - let ret = unsafe { + unsafe { libc::setsockopt( fd, level, @@ -80,17 +76,14 @@ pub fn setsockopt_bytes(fd: libc::c_int, level: i32, name: i32, value: &[u8]) -> value.as_ptr() as *const _, value.len() as libc::socklen_t, ) - }; - if ret < 0 { - Err(crate::os::errno_io_error()) - } else { - Ok(()) } + .check_libc_neg()?; + Ok(()) } #[cfg(unix)] pub fn setsockopt_int(fd: libc::c_int, level: i32, name: i32, value: i32) -> io::Result<()> { - let ret = unsafe { + unsafe { libc::setsockopt( fd, level, @@ -98,17 +91,14 @@ pub fn setsockopt_int(fd: libc::c_int, level: i32, name: i32, value: i32) -> io: &value as *const i32 as *const _, core::mem::size_of::() as libc::socklen_t, ) - }; - if ret < 0 { - Err(crate::os::errno_io_error()) - } else { - Ok(()) } + .check_libc_neg()?; + Ok(()) } #[cfg(unix)] pub fn setsockopt_none(fd: libc::c_int, level: i32, name: i32, optlen: u32) -> io::Result<()> { - let ret = unsafe { + unsafe { libc::setsockopt( fd, level, @@ -116,12 +106,9 @@ pub fn setsockopt_none(fd: libc::c_int, level: i32, name: i32, optlen: u32) -> i core::ptr::null(), optlen as libc::socklen_t, ) - }; - if ret < 0 { - Err(crate::os::errno_io_error()) - } else { - Ok(()) } + .check_libc_neg()?; + Ok(()) } #[cfg(unix)] @@ -367,10 +354,7 @@ pub fn recvmsg( msg.msg_controllen = ancbufsize as _; } - let ret = unsafe { libc::recvmsg(fd.as_raw_fd(), &mut msg, flags) }; - if ret < 0 { - return Err(io::Error::last_os_error()); - } + let ret = unsafe { libc::recvmsg(fd.as_raw_fd(), &mut msg, flags) }.check_libc_neg()?; let data = unsafe { data_buf.set_len(ret as usize); @@ -477,12 +461,8 @@ pub fn sendmsg_afalg( msghdr.msg_controllen = control_buf.len() as _; } - let ret = unsafe { libc::sendmsg(fd.as_raw_fd(), &msghdr, flags) }; - if ret < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(ret as usize) - } + let ret = unsafe { libc::sendmsg(fd.as_raw_fd(), &msghdr, flags) }.check_libc_neg()?; + Ok(ret as usize) } #[cfg(windows)] From 2d4be2ce423c31a04343724ded1167e3b46026c5 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sat, 16 May 2026 22:24:50 +0900 Subject: [PATCH 06/23] Add Win32 BOOL/HANDLE check helpers; apply check helpers across host_env --- crates/host_env/src/mmap.rs | 31 ++++------ crates/host_env/src/msvcrt.rs | 31 +++------- crates/host_env/src/multiprocessing.rs | 10 ++-- crates/host_env/src/os.rs | 29 ++++++++-- crates/host_env/src/resource.rs | 33 ++++------- crates/host_env/src/signal.rs | 70 +++++++---------------- crates/host_env/src/windows.rs | 78 ++++++++++++++++++++------ 7 files changed, 133 insertions(+), 149 deletions(-) diff --git a/crates/host_env/src/mmap.rs b/crates/host_env/src/mmap.rs index ead802949db..44e3fbecf1b 100644 --- a/crates/host_env/src/mmap.rs +++ b/crates/host_env/src/mmap.rs @@ -5,6 +5,8 @@ use std::io; +#[cfg(windows)] +use crate::windows::{CheckWin32Bool, CheckWin32Handle}; #[cfg(unix)] use crate::{crt_fd, fileutils, posix}; use memmap2::{Mmap, MmapMut, MmapOptions}; @@ -131,7 +133,7 @@ impl Drop for NamedMmap { #[cfg(windows)] pub fn duplicate_handle(handle: Handle) -> io::Result { let mut new_handle: Handle = INVALID_HANDLE; - let result = unsafe { + unsafe { DuplicateHandle( GetCurrentProcess(), handle, @@ -141,12 +143,9 @@ pub fn duplicate_handle(handle: Handle) -> io::Result { 0, DUPLICATE_SAME_ACCESS, ) - }; - if result == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(new_handle) } + .check_win32_bool()?; + Ok(new_handle) } #[cfg(windows)] @@ -187,13 +186,9 @@ pub fn is_invalid_handle_value(handle: isize) -> bool { #[cfg(windows)] pub fn extend_file(handle: Handle, size: i64) -> io::Result<()> { - if unsafe { SetFilePointerEx(handle, size, core::ptr::null_mut(), FILE_BEGIN) } == 0 { - return Err(io::Error::last_os_error()); - } - if unsafe { SetEndOfFile(handle) } == 0 { - return Err(io::Error::last_os_error()); - } - Ok(()) + unsafe { SetFilePointerEx(handle, size, core::ptr::null_mut(), FILE_BEGIN) } + .check_win32_bool()?; + unsafe { SetEndOfFile(handle) }.check_win32_bool() } #[cfg(unix)] @@ -210,11 +205,7 @@ pub fn close_handle(handle: Handle) { #[cfg(windows)] pub fn flush_view(ptr: *const core::ffi::c_void, size: usize) -> io::Result<()> { - if unsafe { FlushViewOfFile(ptr, size) } == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + unsafe { FlushViewOfFile(ptr, size) }.check_win32_bool() } #[cfg(windows)] @@ -252,10 +243,8 @@ pub fn create_named_mapping( size_lo, tag_wide.as_ptr(), ) - }; - if map_handle.is_null() { - return Err(io::Error::last_os_error()); } + .check_nonnull()?; let off_hi = (offset as u64 >> 32) as u32; let off_lo = offset as u32; diff --git a/crates/host_env/src/msvcrt.rs b/crates/host_env/src/msvcrt.rs index 47ada95c96f..b94a6d060c7 100644 --- a/crates/host_env/src/msvcrt.rs +++ b/crates/host_env/src/msvcrt.rs @@ -2,6 +2,7 @@ use alloc::{string::String, vec::Vec}; use std::io; use crate::crt_fd; +use crate::os::CheckLibcResult; use windows_sys::Win32::System::Diagnostics::Debug; pub type ErrorMode = u32; @@ -89,39 +90,21 @@ pub fn kbhit() -> i32 { } pub fn locking(fd: i32, mode: i32, nbytes: i64) -> io::Result<()> { - let ret = unsafe { suppress_iph!(_locking(fd, mode, nbytes)) }; - if ret == -1 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + unsafe { suppress_iph!(_locking(fd, mode, nbytes)) }.check_libc_neg()?; + Ok(()) } pub fn heapmin() -> io::Result<()> { - let ret = unsafe { suppress_iph!(_heapmin()) }; - if ret == -1 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + unsafe { suppress_iph!(_heapmin()) }.check_libc_neg()?; + Ok(()) } pub fn setmode(fd: crt_fd::Borrowed<'_>, flags: i32) -> io::Result { - let ret = unsafe { suppress_iph!(_setmode(fd, flags)) }; - if ret == -1 { - Err(io::Error::last_os_error()) - } else { - Ok(ret) - } + unsafe { suppress_iph!(_setmode(fd, flags)) }.check_libc_neg() } pub fn open_osfhandle(handle: isize, flags: i32) -> io::Result { - let ret = unsafe { suppress_iph!(libc::open_osfhandle(handle, flags)) }; - if ret == -1 { - Err(io::Error::last_os_error()) - } else { - Ok(ret) - } + unsafe { suppress_iph!(libc::open_osfhandle(handle, flags)) }.check_libc_neg() } pub fn get_error_mode() -> u32 { diff --git a/crates/host_env/src/multiprocessing.rs b/crates/host_env/src/multiprocessing.rs index 32289c99bfd..4e79b2573cb 100644 --- a/crates/host_env/src/multiprocessing.rs +++ b/crates/host_env/src/multiprocessing.rs @@ -159,13 +159,11 @@ impl SemHandle { #[cfg(windows)] impl SemHandle { pub fn create(value: i32, maxvalue: i32) -> io::Result { + use crate::windows::CheckWin32Handle; let handle = - unsafe { CreateSemaphoreW(core::ptr::null(), value, maxvalue, core::ptr::null()) }; - if handle == 0 as HANDLE { - Err(io::Error::last_os_error()) - } else { - Ok(Self { raw: handle }) - } + unsafe { CreateSemaphoreW(core::ptr::null(), value, maxvalue, core::ptr::null()) } + .check_nonnull()?; + Ok(Self { raw: handle }) } #[inline] diff --git a/crates/host_env/src/os.rs b/crates/host_env/src/os.rs index da17fc92fd2..e2bba1d6895 100644 --- a/crates/host_env/src/os.rs +++ b/crates/host_env/src/os.rs @@ -304,12 +304,7 @@ pub fn seek_fd( position: crt_fd::Offset, how: i32, ) -> io::Result { - let ret = unsafe { suppress_iph!(libc::lseek(fd.as_raw(), position, how)) }; - if ret < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(ret) - } + unsafe { suppress_iph!(libc::lseek(fd.as_raw(), position, how)) }.check_libc_neg() } #[cfg(windows)] @@ -402,6 +397,28 @@ macro_rules! impl_check_libc_result { impl_check_libc_result!(i16, i32, i64, isize); +/// libc convention where `0` means success and any non-zero value indicates failure +/// (with errno set). Used by APIs like `sigemptyset`, `sigaction`, `pthread_*`, etc. +pub trait CheckLibcZero { + /// Returns `Ok(())` if `self == 0`, otherwise the current errno as an `io::Error`. + fn check_libc_zero(self) -> io::Result<()>; +} + +macro_rules! impl_check_libc_zero { + ($($ty:ty),* $(,)?) => { + $( + impl CheckLibcZero for $ty { + #[inline] + fn check_libc_zero(self) -> io::Result<()> { + if self == 0 { Ok(()) } else { Err(errno_io_error()) } + } + } + )* + }; +} + +impl_check_libc_zero!(i32, i64, isize); + #[cfg(windows)] pub fn get_errno() -> i32 { unsafe extern "C" { diff --git a/crates/host_env/src/resource.rs b/crates/host_env/src/resource.rs index c18e41da27e..587428fe9b0 100644 --- a/crates/host_env/src/resource.rs +++ b/crates/host_env/src/resource.rs @@ -1,5 +1,7 @@ use std::io; +use crate::os::CheckLibcResult; + #[derive(Debug, Clone, Copy)] pub struct RUsage { pub ru_utime: libc::timeval, @@ -44,35 +46,20 @@ impl From for RUsage { } pub fn getrusage(who: i32) -> io::Result { - unsafe { - let mut rusage = core::mem::MaybeUninit::::uninit(); - if libc::getrusage(who, rusage.as_mut_ptr()) == -1 { - Err(io::Error::last_os_error()) - } else { - Ok(rusage.assume_init().into()) - } - } + let mut rusage = core::mem::MaybeUninit::::uninit(); + unsafe { libc::getrusage(who, rusage.as_mut_ptr()) }.check_libc_neg()?; + Ok(unsafe { rusage.assume_init() }.into()) } pub fn getrlimit(resource: libc::rlim_t) -> io::Result { - unsafe { - let mut rlimit = core::mem::MaybeUninit::::uninit(); - if libc::getrlimit(resource as _, rlimit.as_mut_ptr()) == -1 { - Err(io::Error::last_os_error()) - } else { - Ok(rlimit.assume_init()) - } - } + let mut rlimit = core::mem::MaybeUninit::::uninit(); + unsafe { libc::getrlimit(resource as _, rlimit.as_mut_ptr()) }.check_libc_neg()?; + Ok(unsafe { rlimit.assume_init() }) } pub fn setrlimit(resource: libc::rlim_t, limits: libc::rlimit) -> io::Result<()> { - unsafe { - if libc::setrlimit(resource as _, &limits) == -1 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } - } + unsafe { libc::setrlimit(resource as _, &limits) }.check_libc_neg()?; + Ok(()) } #[cfg(not(any(target_os = "redox", target_os = "wasi")))] diff --git a/crates/host_env/src/signal.rs b/crates/host_env/src/signal.rs index c246ecc122f..bf1f2e975b1 100644 --- a/crates/host_env/src/signal.rs +++ b/crates/host_env/src/signal.rs @@ -2,6 +2,9 @@ use std::io; #[cfg(windows)] use std::sync::Once; +#[cfg(any(unix, windows))] +use crate::os::{CheckLibcResult, CheckLibcZero}; + #[cfg(any(unix, windows))] pub use libc::sighandler_t; @@ -81,12 +84,7 @@ pub unsafe fn install_handler(signalnum: i32, handler: sighandler_t) -> io::Resu #[cfg(any(unix, windows))] pub fn raise_signal(signalnum: i32) -> io::Result<()> { - let res = unsafe { libc::raise(signalnum) }; - if res != 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + unsafe { libc::raise(signalnum) }.check_libc_zero() } #[cfg(unix)] @@ -104,21 +102,13 @@ pub fn set_sigint_default_onstack() -> io::Result<()> { let mut action: libc::sigaction = unsafe { core::mem::zeroed() }; action.sa_sigaction = libc::SIG_DFL; action.sa_flags = libc::SA_ONSTACK; - if unsafe { libc::sigemptyset(&mut action.sa_mask) } != 0 { - return Err(io::Error::last_os_error()); - } - if unsafe { libc::sigaction(libc::SIGINT, &action, core::ptr::null_mut()) } != 0 { - return Err(io::Error::last_os_error()); - } - Ok(()) + unsafe { libc::sigemptyset(&mut action.sa_mask) }.check_libc_zero()?; + unsafe { libc::sigaction(libc::SIGINT, &action, core::ptr::null_mut()) }.check_libc_zero() } #[cfg(unix)] pub fn send_sigint_to_self() -> io::Result<()> { - if unsafe { libc::kill(libc::getpid(), libc::SIGINT) } != 0 { - return Err(io::Error::last_os_error()); - } - Ok(()) + unsafe { libc::kill(libc::getpid(), libc::SIGINT) }.check_libc_zero() } #[cfg(unix)] @@ -128,11 +118,8 @@ pub fn setitimer(which: i32, new: &libc::itimerval) -> io::Result io::Result { let ret = unsafe { ffi::getitimer(which, old.as_mut_ptr()) }; #[cfg(not(any(target_os = "linux", target_os = "android")))] let ret = unsafe { libc::getitimer(which, old.as_mut_ptr()) }; - if ret != 0 { - Err(io::Error::last_os_error()) - } else { - Ok(unsafe { old.assume_init() }) - } + ret.check_libc_zero()?; + Ok(unsafe { old.assume_init() }) } #[cfg(unix)] pub fn sigemptyset() -> io::Result { let mut set: libc::sigset_t = unsafe { core::mem::zeroed() }; - if unsafe { libc::sigemptyset(&mut set) } != 0 { - Err(io::Error::last_os_error()) - } else { - Ok(set) - } + unsafe { libc::sigemptyset(&mut set) }.check_libc_zero()?; + Ok(set) } #[cfg(unix)] pub fn sigaddset(set: &mut libc::sigset_t, signum: i32) -> io::Result<()> { - if unsafe { libc::sigaddset(set, signum) } != 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + unsafe { libc::sigaddset(set, signum) }.check_libc_zero() } #[cfg(unix)] @@ -190,21 +167,14 @@ pub fn pidfd_send_signal(pidfd: i32, sig: i32, flags: u32) -> io::Result<()> { flags, ) as libc::c_long }; - if ret == -1 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + ret.check_libc_neg()?; + Ok(()) } #[cfg(all(unix, not(target_os = "redox")))] pub fn siginterrupt(signalnum: i32, flag: i32) -> io::Result<()> { - let res = unsafe { c_siginterrupt(signalnum, flag) }; - if res < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + unsafe { c_siginterrupt(signalnum, flag) }.check_libc_neg()?; + Ok(()) } #[cfg(windows)] @@ -349,9 +319,7 @@ pub fn strsignal(signalnum: i32) -> Option { #[cfg(unix)] pub fn valid_signals(max_signum: usize) -> io::Result> { let mut mask: libc::sigset_t = unsafe { core::mem::zeroed() }; - if unsafe { libc::sigfillset(&mut mask) } != 0 { - return Err(io::Error::last_os_error()); - } + unsafe { libc::sigfillset(&mut mask) }.check_libc_zero()?; let mut signals = Vec::new(); for signum in 1..max_signum { if unsafe { libc::sigismember(&mask, signum as i32) } == 1 { diff --git a/crates/host_env/src/windows.rs b/crates/host_env/src/windows.rs index 75b5ec8ab7c..949e746c9ee 100644 --- a/crates/host_env/src/windows.rs +++ b/crates/host_env/src/windows.rs @@ -47,6 +47,56 @@ pub fn init_winsock() { }) } +/// Win32 BOOL convention: 0 = failure, nonzero = success. +/// Reads error via `GetLastError()` through [`io::Error::last_os_error`]. +pub trait CheckWin32Bool { + fn check_win32_bool(self) -> io::Result<()>; +} + +impl CheckWin32Bool for i32 { + #[inline] + fn check_win32_bool(self) -> io::Result<()> { + if self == 0 { + Err(io::Error::last_os_error()) + } else { + Ok(()) + } + } +} + +/// Convenience checks for Win32 `HANDLE` return values. +pub trait CheckWin32Handle: Sized { + /// Returns `Ok(self)` if the handle is non-NULL, otherwise the last OS error. + /// Use for APIs whose failure sentinel is NULL (Create*Event, Create*Mutex, + /// CreateFileMapping, GetModuleHandle, etc.). + fn check_nonnull(self) -> io::Result; + + /// Returns `Ok(self)` unless the handle equals `INVALID_HANDLE_VALUE`. + /// Use for APIs whose failure sentinel is `INVALID_HANDLE_VALUE` + /// (CreateFileW, CreateNamedPipeW, etc.). + fn check_valid(self) -> io::Result; +} + +impl CheckWin32Handle for windows_sys::Win32::Foundation::HANDLE { + #[inline] + fn check_nonnull(self) -> io::Result { + if self.is_null() { + Err(io::Error::last_os_error()) + } else { + Ok(self) + } + } + + #[inline] + fn check_valid(self) -> io::Result { + if self == windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE { + Err(io::Error::last_os_error()) + } else { + Ok(self) + } + } +} + #[derive(Clone, Debug)] pub struct WindowsVersionInfo { pub major: u32, @@ -63,10 +113,7 @@ pub struct WindowsVersionInfo { fn get_kernel32_version() -> io::Result<(u32, u32, u32)> { unsafe { let module_name: Vec = OsStr::new("kernel32.dll").to_wide_with_nul(); - let h_kernel32 = GetModuleHandleW(module_name.as_ptr()); - if h_kernel32.is_null() { - return Err(io::Error::last_os_error()); - } + let h_kernel32 = GetModuleHandleW(module_name.as_ptr()).check_nonnull()?; let mut kernel32_path = [0u16; MAX_PATH as usize]; let len = GetModuleFileNameW( @@ -84,28 +131,26 @@ fn get_kernel32_version() -> io::Result<(u32, u32, u32)> { } let mut ver_block = vec![0u8; ver_block_size as usize]; - if GetFileVersionInfoW( + GetFileVersionInfoW( kernel32_path.as_ptr(), 0, ver_block_size, ver_block.as_mut_ptr() as *mut _, - ) == 0 - { - return Err(io::Error::last_os_error()); - } + ) + .check_win32_bool()?; let sub_block: Vec = OsStr::new("").to_wide_with_nul(); let mut ffi_ptr: *mut VS_FIXEDFILEINFO = core::ptr::null_mut(); let mut ffi_len: u32 = 0; - if VerQueryValueW( + VerQueryValueW( ver_block.as_ptr() as *const _, sub_block.as_ptr(), &mut ffi_ptr as *mut *mut VS_FIXEDFILEINFO as *mut *mut _, &mut ffi_len as *mut u32, - ) == 0 - || ffi_ptr.is_null() - { + ) + .check_win32_bool()?; + if ffi_ptr.is_null() { return Err(io::Error::last_os_error()); } @@ -121,14 +166,11 @@ fn get_kernel32_version() -> io::Result<(u32, u32, u32)> { pub fn get_windows_version() -> io::Result { let mut version: OSVERSIONINFOEXW = unsafe { core::mem::zeroed() }; version.dwOSVersionInfoSize = core::mem::size_of::() as u32; - let result = unsafe { + unsafe { let os_vi = &mut version as *mut OSVERSIONINFOEXW as *mut OSVERSIONINFOW; GetVersionExW(os_vi) - }; - - if result == 0 { - return Err(io::Error::last_os_error()); } + .check_win32_bool()?; let service_pack = { let (last, _) = version From 592e8b482ad275216092e3249e6e7d003dbc3e28 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sat, 16 May 2026 22:28:01 +0900 Subject: [PATCH 07/23] Apply Win32/libc check helpers to overlapped/testconsole/os.rs --- crates/host_env/src/os.rs | 9 ++---- crates/host_env/src/overlapped.rs | 48 +++++++++--------------------- crates/host_env/src/testconsole.rs | 15 ++++------ 3 files changed, 23 insertions(+), 49 deletions(-) diff --git a/crates/host_env/src/os.rs b/crates/host_env/src/os.rs index e2bba1d6895..ab94b6b9ad7 100644 --- a/crates/host_env/src/os.rs +++ b/crates/host_env/src/os.rs @@ -323,22 +323,19 @@ pub fn set_file_times( access: Duration, modified: Duration, ) -> io::Result<()> { + use crate::windows::CheckWin32Bool; let access = filetime_from_duration(access); let modified = filetime_from_duration(modified); let file = fs::open_write_with_custom_flags(path, FILE_FLAG_BACKUP_SEMANTICS)?; - let ret = unsafe { + unsafe { SetFileTime( file.as_raw_handle() as _, core::ptr::null(), &access, &modified, ) - }; - if ret == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) } + .check_win32_bool() } pub trait ErrorExt { diff --git a/crates/host_env/src/overlapped.rs b/crates/host_env/src/overlapped.rs index 3ee3da80170..360bf49302d 100644 --- a/crates/host_env/src/overlapped.rs +++ b/crates/host_env/src/overlapped.rs @@ -14,6 +14,8 @@ use std::{ io, sync::{Mutex, OnceLock}, }; + +use crate::windows::{CheckWin32Bool, CheckWin32Handle}; use windows_sys::Win32::{ Foundation::{CloseHandle, ERROR_IO_PENDING, ERROR_MORE_DATA, ERROR_SUCCESS, HANDLE}, Networking::WinSock::{AF_INET, AF_INET6, SOCKADDR, SOCKADDR_IN, SOCKADDR_IN6}, @@ -76,10 +78,8 @@ unsafe impl Send for Operation {} impl Operation { pub fn new(handle: HANDLE) -> io::Result { - let event = unsafe { CreateEventW(core::ptr::null(), 1, 0, core::ptr::null()) }; - if event.is_null() { - return Err(io::Error::last_os_error()); - } + let event = + unsafe { CreateEventW(core::ptr::null(), 1, 0, core::ptr::null()) }.check_nonnull()?; let mut overlapped: OVERLAPPED = unsafe { core::mem::zeroed() }; overlapped.hEvent = event; @@ -177,9 +177,7 @@ impl Operation { self.pending = true; } ERROR_PIPE_CONNECTED => { - if unsafe { SetEvent(self.overlapped.hEvent) } == 0 { - return Err(io::Error::last_os_error()); - } + unsafe { SetEvent(self.overlapped.hEvent) }.check_win32_bool()?; } _ => return Err(io::Error::from_raw_os_error(err as i32)), } @@ -808,11 +806,8 @@ pub fn connect_pipe(address: &str) -> io::Result { core::ptr::null_mut(), ) }; - if handle == INVALID_HANDLE_VALUE { - Err(io::Error::last_os_error()) - } else { - Ok(handle as isize) - } + let handle = handle.check_valid()?; + Ok(handle as isize) } pub fn create_io_completion_port( @@ -827,13 +822,10 @@ pub fn create_io_completion_port( port as HANDLE, key, concurrency, - ) as isize - }; - if r == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(r) + ) } + .check_nonnull()?; + Ok(r as isize) } pub fn get_queued_completion_status(port: isize, msecs: u32) -> io::Result { @@ -876,19 +868,15 @@ pub fn post_queued_completion_status( key: usize, address: usize, ) -> io::Result<()> { - let ret = unsafe { + unsafe { windows_sys::Win32::System::IO::PostQueuedCompletionStatus( port as HANDLE, bytes, key, address as *mut OVERLAPPED, ) - }; - if ret == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) } + .check_win32_bool() } unsafe impl Send for WaitCallbackData {} @@ -974,11 +962,7 @@ pub fn unregister_wait(wait_handle: isize) -> io::Result<()> { let ret = unsafe { windows_sys::Win32::System::Threading::UnregisterWait(wait_handle as HANDLE) }; cleanup_wait_callback_data(wait_handle); - if ret == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + ret.check_win32_bool() } pub fn unregister_wait_ex(wait_handle: isize, event: isize) -> io::Result<()> { @@ -989,11 +973,7 @@ pub fn unregister_wait_ex(wait_handle: isize, event: isize) -> io::Result<()> { ) }; cleanup_wait_callback_data(wait_handle); - if ret == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + ret.check_win32_bool() } pub fn bind_local(socket: isize, family: i32) -> io::Result<()> { diff --git a/crates/host_env/src/testconsole.rs b/crates/host_env/src/testconsole.rs index 001e72e6ed5..02b82f4bf4d 100644 --- a/crates/host_env/src/testconsole.rs +++ b/crates/host_env/src/testconsole.rs @@ -1,14 +1,13 @@ use std::io; use windows_sys::Win32::{ - Foundation::{HANDLE, INVALID_HANDLE_VALUE}, + Foundation::HANDLE, System::Console::{INPUT_RECORD, KEY_EVENT, WriteConsoleInputW}, }; +use crate::windows::{CheckWin32Bool, CheckWin32Handle}; + pub fn write_console_input(fd: i32, data: &[u16]) -> io::Result<()> { - let handle = unsafe { libc::get_osfhandle(fd) } as HANDLE; - if handle == INVALID_HANDLE_VALUE { - return Err(io::Error::last_os_error()); - } + let handle = (unsafe { libc::get_osfhandle(fd) } as HANDLE).check_valid()?; let size = data.len() as u32; let mut records: Vec = Vec::with_capacity(data.len()); @@ -24,17 +23,15 @@ pub fn write_console_input(fd: i32, data: &[u16]) -> io::Result<()> { let mut total: u32 = 0; while total < size { let mut wrote: u32 = 0; - let res = unsafe { + unsafe { WriteConsoleInputW( handle, records[total as usize..].as_ptr(), size - total, &mut wrote, ) - }; - if res == 0 { - return Err(io::Error::last_os_error()); } + .check_win32_bool()?; if wrote == 0 { return Err(io::Error::new( io::ErrorKind::WriteZero, From 86055d28b4059507e56ede7da7994d7dd6751de9 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sat, 16 May 2026 22:30:16 +0900 Subject: [PATCH 08/23] Apply Win32 check helpers to winapi.rs (partial) --- crates/host_env/src/winapi.rs | 74 +++++++++++------------------------ 1 file changed, 22 insertions(+), 52 deletions(-) diff --git a/crates/host_env/src/winapi.rs b/crates/host_env/src/winapi.rs index 86542c0df28..62487075c32 100644 --- a/crates/host_env/src/winapi.rs +++ b/crates/host_env/src/winapi.rs @@ -9,6 +9,8 @@ use windows_sys::Win32::{ System::Threading::PROCESS_INFORMATION, }; +use crate::windows::{CheckWin32Bool, CheckWin32Handle}; + pub use windows_sys::Win32::{ Foundation::{ DUPLICATE_CLOSE_SOURCE, DUPLICATE_SAME_ACCESS, ERROR_ACCESS_DENIED, ERROR_ALREADY_EXISTS, @@ -180,11 +182,7 @@ pub fn create_file_w( core::ptr::null_mut(), ) }; - if handle == windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE { - Err(io::Error::last_os_error()) - } else { - Ok(handle) - } + handle.check_valid() } /// # Safety @@ -199,7 +197,7 @@ unsafe fn create_process_w_raw( startup_info: *mut windows_sys::Win32::System::Threading::STARTUPINFOW, ) -> io::Result { let mut procinfo = core::mem::MaybeUninit::::uninit(); - let ok = unsafe { + unsafe { windows_sys::Win32::System::Threading::CreateProcessW( app_name.map_or(core::ptr::null(), |s| s.as_ptr()), command_line.map_or(core::ptr::null_mut(), |s| s.as_mut_ptr()), @@ -212,12 +210,9 @@ unsafe fn create_process_w_raw( startup_info, procinfo.as_mut_ptr(), ) - }; - if ok == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(unsafe { procinfo.assume_init() }) } + .check_win32_bool()?; + Ok(unsafe { procinfo.assume_init() }) } #[allow( @@ -335,19 +330,17 @@ pub fn create_handle_list_attribute_list( handlelist, attrlist: vec![0u8; size], }; - let ok = unsafe { + unsafe { windows_sys::Win32::System::Threading::InitializeProcThreadAttributeList( attrs.attrlist.as_mut_ptr().cast(), 1, 0, &mut size, ) - }; - if ok == 0 { - return Err(io::Error::last_os_error()); } + .check_win32_bool()?; - let ok = unsafe { + unsafe { windows_sys::Win32::System::Threading::UpdateProcThreadAttribute( attrs.attrlist.as_mut_ptr().cast(), 0, @@ -357,10 +350,8 @@ pub fn create_handle_list_attribute_list( core::ptr::null_mut(), core::ptr::null(), ) - }; - if ok == 0 { - return Err(io::Error::last_os_error()); } + .check_win32_bool()?; Ok(Some(attrs)) } @@ -381,36 +372,29 @@ pub fn open_process( inherit_handle: bool, process_id: u32, ) -> io::Result { - let handle = unsafe { + unsafe { windows_sys::Win32::System::Threading::OpenProcess( desired_access, i32::from(inherit_handle), process_id, ) - }; - if handle.is_null() { - Err(io::Error::last_os_error()) - } else { - Ok(handle) } + .check_nonnull() } pub fn create_pipe(size: u32) -> io::Result<(HANDLE, HANDLE)> { - let (read, write) = unsafe { + unsafe { let mut read = core::mem::MaybeUninit::::uninit(); let mut write = core::mem::MaybeUninit::::uninit(); - let ok = windows_sys::Win32::System::Pipes::CreatePipe( + windows_sys::Win32::System::Pipes::CreatePipe( read.as_mut_ptr(), write.as_mut_ptr(), core::ptr::null(), size, - ); - if ok == 0 { - return Err(io::Error::last_os_error()); - } - (read.assume_init(), write.assume_init()) - }; - Ok((read, write)) + ) + .check_win32_bool()?; + Ok((read.assume_init(), write.assume_init())) + } } pub fn create_event_w( @@ -419,37 +403,23 @@ pub fn create_event_w( name: Option<&widestring::WideCStr>, ) -> io::Result { let name_ptr = name.map_or(core::ptr::null(), |n| n.as_ptr()); - let handle = unsafe { + unsafe { windows_sys::Win32::System::Threading::CreateEventW( core::ptr::null(), i32::from(manual_reset), i32::from(initial_state), name_ptr, ) - }; - if handle.is_null() { - Err(io::Error::last_os_error()) - } else { - Ok(handle) } + .check_nonnull() } pub fn set_event(handle: HANDLE) -> io::Result<()> { - let ok = unsafe { windows_sys::Win32::System::Threading::SetEvent(handle) }; - if ok == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + unsafe { windows_sys::Win32::System::Threading::SetEvent(handle) }.check_win32_bool() } pub fn reset_event(handle: HANDLE) -> io::Result<()> { - let ok = unsafe { windows_sys::Win32::System::Threading::ResetEvent(handle) }; - if ok == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + unsafe { windows_sys::Win32::System::Threading::ResetEvent(handle) }.check_win32_bool() } pub fn wait_for_single_object(handle: HANDLE, milliseconds: u32) -> io::Result { From fe71c58a015f0819e4f7515b26c1afb1bfe7f58d Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sat, 16 May 2026 22:33:45 +0900 Subject: [PATCH 09/23] Apply Win32 check helpers across more winapi.rs functions --- crates/host_env/src/winapi.rs | 127 +++++++++------------------------- 1 file changed, 32 insertions(+), 95 deletions(-) diff --git a/crates/host_env/src/winapi.rs b/crates/host_env/src/winapi.rs index 62487075c32..b1f08932ae0 100644 --- a/crates/host_env/src/winapi.rs +++ b/crates/host_env/src/winapi.rs @@ -732,14 +732,11 @@ pub fn get_current_process() -> HANDLE { pub fn get_exit_code_process(handle: HANDLE) -> io::Result { let mut exit_code = core::mem::MaybeUninit::::uninit(); - let ok = unsafe { + unsafe { windows_sys::Win32::System::Threading::GetExitCodeProcess(handle, exit_code.as_mut_ptr()) - }; - if ok == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(unsafe { exit_code.assume_init() }) } + .check_win32_bool()?; + Ok(unsafe { exit_code.assume_init() }) } pub fn get_file_type(handle: HANDLE) -> io::Result { @@ -771,33 +768,18 @@ pub fn get_version() -> u32 { pub fn create_job_object_w(name: Option<&widestring::WideCStr>) -> io::Result { let name_ptr = name.map_or(core::ptr::null(), |n| n.as_ptr()); - let handle = unsafe { - windows_sys::Win32::System::JobObjects::CreateJobObjectW(core::ptr::null(), name_ptr) - }; - if handle.is_null() { - Err(io::Error::last_os_error()) - } else { - Ok(handle) - } + unsafe { windows_sys::Win32::System::JobObjects::CreateJobObjectW(core::ptr::null(), name_ptr) } + .check_nonnull() } pub fn assign_process_to_job_object(job: HANDLE, process: HANDLE) -> io::Result<()> { - let ok = - unsafe { windows_sys::Win32::System::JobObjects::AssignProcessToJobObject(job, process) }; - if ok == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + unsafe { windows_sys::Win32::System::JobObjects::AssignProcessToJobObject(job, process) } + .check_win32_bool() } pub fn terminate_job_object(job: HANDLE, exit_code: u32) -> io::Result<()> { - let ok = unsafe { windows_sys::Win32::System::JobObjects::TerminateJobObject(job, exit_code) }; - if ok == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + unsafe { windows_sys::Win32::System::JobObjects::TerminateJobObject(job, exit_code) } + .check_win32_bool() } pub fn set_job_object_kill_on_close(job: HANDLE) -> io::Result<()> { @@ -808,19 +790,15 @@ pub fn set_job_object_kill_on_close(job: HANDLE) -> io::Result<()> { let mut info: JOBOBJECT_EXTENDED_LIMIT_INFORMATION = unsafe { core::mem::zeroed() }; info.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; - let ok = unsafe { + unsafe { SetInformationJobObject( job, JobObjectExtendedLimitInformation, (&info as *const JOBOBJECT_EXTENDED_LIMIT_INFORMATION).cast(), core::mem::size_of::() as u32, ) - }; - if ok == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) } + .check_win32_bool() } pub fn get_module_file_name(module: HMODULE, buffer: &mut [u16]) -> u32 { @@ -870,18 +848,14 @@ pub fn open_mutex_w( inherit_handle: bool, name: &widestring::WideCStr, ) -> io::Result { - let handle = unsafe { + unsafe { windows_sys::Win32::System::Threading::OpenMutexW( desired_access, i32::from(inherit_handle), name.as_ptr(), ) - }; - if handle.is_null() { - Err(io::Error::last_os_error()) - } else { - Ok(handle) } + .check_nonnull() } pub fn release_mutex(handle: HANDLE) -> i32 { @@ -897,7 +871,7 @@ pub fn create_named_pipe_w( in_buffer_size: u32, default_timeout: u32, ) -> io::Result { - let handle = unsafe { + unsafe { windows_sys::Win32::System::Pipes::CreateNamedPipeW( name.as_ptr(), open_mode, @@ -908,12 +882,8 @@ pub fn create_named_pipe_w( default_timeout, core::ptr::null(), ) - }; - if handle == windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE { - Err(io::Error::last_os_error()) - } else { - Ok(handle) } + .check_valid() } pub fn create_file_mapping_w( @@ -924,7 +894,7 @@ pub fn create_file_mapping_w( name: Option<&widestring::WideCStr>, ) -> io::Result { let name_ptr = name.map_or(core::ptr::null(), |n| n.as_ptr()); - let handle = unsafe { + unsafe { windows_sys::Win32::System::Memory::CreateFileMappingW( file_handle, core::ptr::null(), @@ -933,12 +903,8 @@ pub fn create_file_mapping_w( max_size_low, name_ptr, ) - }; - if handle.is_null() { - Err(io::Error::last_os_error()) - } else { - Ok(handle) } + .check_nonnull() } pub fn open_file_mapping_w( @@ -946,18 +912,14 @@ pub fn open_file_mapping_w( inherit_handle: bool, name: &widestring::WideCStr, ) -> io::Result { - let handle = unsafe { + unsafe { windows_sys::Win32::System::Memory::OpenFileMappingW( desired_access, i32::from(inherit_handle), name.as_ptr(), ) - }; - if handle.is_null() { - Err(io::Error::last_os_error()) - } else { - Ok(handle) } + .check_nonnull() } pub fn map_view_of_file( @@ -988,12 +950,7 @@ pub fn unmap_view_of_file(address: isize) -> io::Result<()> { let view = windows_sys::Win32::System::Memory::MEMORY_MAPPED_VIEW_ADDRESS { Value: address as *mut core::ffi::c_void, }; - let ok = unsafe { windows_sys::Win32::System::Memory::UnmapViewOfFile(view) }; - if ok == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + unsafe { windows_sys::Win32::System::Memory::UnmapViewOfFile(view) }.check_win32_bool() } pub fn virtual_query_size(address: isize) -> io::Result { @@ -1197,12 +1154,8 @@ pub fn connect_named_pipe(handle: HANDLE) -> io::Result<()> { } pub fn wait_named_pipe_w(name: &widestring::WideCStr, timeout: u32) -> io::Result<()> { - let ok = unsafe { windows_sys::Win32::System::Pipes::WaitNamedPipeW(name.as_ptr(), timeout) }; - if ok == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + unsafe { windows_sys::Win32::System::Pipes::WaitNamedPipeW(name.as_ptr(), timeout) } + .check_win32_bool() } pub fn peek_named_pipe(handle: HANDLE, size: Option) -> io::Result { @@ -1212,7 +1165,7 @@ pub fn peek_named_pipe(handle: HANDLE, size: Option) -> io::Result { let mut data = vec![0u8; size as usize]; let mut read = 0; - let ok = unsafe { + unsafe { windows_sys::Win32::System::Pipes::PeekNamedPipe( handle, data.as_mut_ptr().cast(), @@ -1221,10 +1174,8 @@ pub fn peek_named_pipe(handle: HANDLE, size: Option) -> io::Result) -> io::Result { - let ok = unsafe { + unsafe { windows_sys::Win32::System::Pipes::PeekNamedPipe( handle, core::ptr::null_mut(), @@ -1242,10 +1193,8 @@ pub fn peek_named_pipe(handle: HANDLE, size: Option) -> io::Result, ) -> io::Result { let name_ptr = name.map_or(core::ptr::null(), |n| n.as_ptr()); - let handle = unsafe { + unsafe { windows_sys::Win32::System::Threading::CreateMutexW( core::ptr::null(), i32::from(initial_owner), name_ptr, ) - }; - if handle.is_null() { - Err(io::Error::last_os_error()) - } else { - Ok(handle) } + .check_nonnull() } pub fn open_event_w( @@ -1362,18 +1303,14 @@ pub fn open_event_w( inherit_handle: bool, name: &widestring::WideCStr, ) -> io::Result { - let handle = unsafe { + unsafe { windows_sys::Win32::System::Threading::OpenEventW( desired_access, i32::from(inherit_handle), name.as_ptr(), ) - }; - if handle.is_null() { - Err(io::Error::last_os_error()) - } else { - Ok(handle) } + .check_nonnull() } pub fn need_current_directory_for_exe_path_w(exe_name: &widestring::WideCStr) -> bool { From a7c567a098bfc388497e9472be226b8e67cdfd6b Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sat, 16 May 2026 22:35:37 +0900 Subject: [PATCH 10/23] Apply Win32 check helpers to nt.rs (partial) --- crates/host_env/src/nt.rs | 31 ++++++++----------------------- 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/crates/host_env/src/nt.rs b/crates/host_env/src/nt.rs index 8ca34880e9a..4ca891bd246 100644 --- a/crates/host_env/src/nt.rs +++ b/crates/host_env/src/nt.rs @@ -19,7 +19,7 @@ use crate::{ StatStruct, windows::{FILE_INFO_BY_NAME_CLASS, get_file_information_by_name, stat_basic_info_to_stat}, }, - windows::ToWideString, + windows::{CheckWin32Bool, CheckWin32Handle, ToWideString}, }; use libc::intptr_t; use windows_sys::Win32::{ @@ -257,16 +257,12 @@ pub fn remove(path: &Path) -> io::Result<()> { } } - let ok = if is_directory && is_link { + if is_directory && is_link { unsafe { RemoveDirectoryW(wide_path.as_ptr()) } } else { unsafe { DeleteFileW(wide_path.as_ptr()) } - }; - if ok == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) } + .check_win32_bool() } pub fn supports_virtual_terminal() -> bool { @@ -351,17 +347,15 @@ pub fn symlink( #[allow(clippy::not_unsafe_ptr_arg_deref)] pub fn win32_hchmod(handle: HANDLE, mode: u32, write_bit: u32) -> io::Result<()> { let mut info: FILE_BASIC_INFO = unsafe { core::mem::zeroed() }; - let ret = unsafe { + unsafe { GetFileInformationByHandleEx( handle, FileBasicInfo, (&mut info as *mut FILE_BASIC_INFO).cast(), core::mem::size_of::() as u32, ) - }; - if ret == 0 { - return Err(io::Error::last_os_error()); } + .check_win32_bool()?; if mode & write_bit != 0 { info.FileAttributes &= !FILE_ATTRIBUTE_READONLY; @@ -369,19 +363,15 @@ pub fn win32_hchmod(handle: HANDLE, mode: u32, write_bit: u32) -> io::Result<()> info.FileAttributes |= FILE_ATTRIBUTE_READONLY; } - let ret = unsafe { + unsafe { SetFileInformationByHandle( handle, FileBasicInfo, (&info as *const FILE_BASIC_INFO).cast(), core::mem::size_of::() as u32, ) - }; - if ret == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) } + .check_win32_bool() } pub fn fchmod(fd: i32, mode: u32, write_bit: u32) -> io::Result<()> { @@ -401,12 +391,7 @@ pub fn win32_lchmod(path: &OsStr, mode: u32, write_bit: u32) -> io::Result<()> { } else { attr | FILE_ATTRIBUTE_READONLY }; - let ret = unsafe { SetFileAttributesW(wide.as_ptr(), new_attr) }; - if ret == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + unsafe { SetFileAttributesW(wide.as_ptr(), new_attr) }.check_win32_bool() } pub fn chmod_follow(path: &widestring::WideCStr, mode: u32, write_bit: u32) -> io::Result<()> { From 5329cc59f8d22fbdb59c2c9fa2f3fc8fcb339c59 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sun, 17 May 2026 01:00:13 +0900 Subject: [PATCH 11/23] Add CheckWin32Sentinel helper; apply to nt.rs INVALID_HANDLE_VALUE/INVALID_FILE_ATTRIBUTES patterns --- crates/host_env/src/nt.rs | 85 ++++++++++------------------------ crates/host_env/src/windows.rs | 19 ++++++++ 2 files changed, 43 insertions(+), 61 deletions(-) diff --git a/crates/host_env/src/nt.rs b/crates/host_env/src/nt.rs index 4ca891bd246..541ea771bc6 100644 --- a/crates/host_env/src/nt.rs +++ b/crates/host_env/src/nt.rs @@ -19,7 +19,7 @@ use crate::{ StatStruct, windows::{FILE_INFO_BY_NAME_CLASS, get_file_information_by_name, stat_basic_info_to_stat}, }, - windows::{CheckWin32Bool, CheckWin32Handle, ToWideString}, + windows::{CheckWin32Bool, CheckWin32Handle, CheckWin32Sentinel, ToWideString}, }; use libc::intptr_t; use windows_sys::Win32::{ @@ -382,10 +382,7 @@ pub fn fchmod(fd: i32, mode: u32, write_bit: u32) -> io::Result<()> { pub fn win32_lchmod(path: &OsStr, mode: u32, write_bit: u32) -> io::Result<()> { let wide = path.to_wide_with_nul(); - let attr = unsafe { GetFileAttributesW(wide.as_ptr()) }; - if attr == INVALID_FILE_ATTRIBUTES { - return Err(io::Error::last_os_error()); - } + let attr = unsafe { GetFileAttributesW(wide.as_ptr()) }.check_ne(INVALID_FILE_ATTRIBUTES)?; let new_attr = if mode & write_bit != 0 { attr & !FILE_ATTRIBUTE_READONLY } else { @@ -423,10 +420,7 @@ pub fn find_first_file_name(path: &Path) -> io::Result { let wide_path = path.as_os_str().to_wide_with_nul(); let mut find_data: WIN32_FIND_DATAW = unsafe { core::mem::zeroed() }; - let handle = unsafe { FindFirstFileW(wide_path.as_ptr(), &mut find_data) }; - if handle == INVALID_HANDLE_VALUE { - return Err(io::Error::last_os_error()); - } + let handle = unsafe { FindFirstFileW(wide_path.as_ptr(), &mut find_data) }.check_valid()?; unsafe { FindClose(handle) }; let len = find_data @@ -457,11 +451,8 @@ pub fn path_isdevdrive(path: &Path) -> io::Result { let wide_path = path.as_os_str().to_wide_with_nul(); let mut volume = [0u16; MAX_PATH as usize]; - let ok = - unsafe { GetVolumePathNameW(wide_path.as_ptr(), volume.as_mut_ptr(), volume.len() as _) }; - if ok == 0 { - return Err(io::Error::last_os_error()); - } + unsafe { GetVolumePathNameW(wide_path.as_ptr(), volume.as_mut_ptr(), volume.len() as _) } + .check_win32_bool()?; if unsafe { GetDriveTypeW(volume.as_ptr()) } != DRIVE_FIXED { return Ok(false); } @@ -476,10 +467,8 @@ pub fn path_isdevdrive(path: &Path) -> io::Result { FILE_FLAG_BACKUP_SEMANTICS, core::ptr::null_mut(), ) - }; - if handle == INVALID_HANDLE_VALUE { - return Err(io::Error::last_os_error()); } + .check_valid()?; let mut volume_state = FileFsPersistentVolumeInformation { volume_flags: 0, @@ -630,10 +619,7 @@ fn win32_xstat_attributes_from_dir( let wide: Vec = path.to_wide_with_nul(); let mut find_data: WIN32_FIND_DATAW = unsafe { core::mem::zeroed() }; - let handle = unsafe { FindFirstFileW(wide.as_ptr(), &mut find_data) }; - if handle == INVALID_HANDLE_VALUE { - return Err(io::Error::last_os_error()); - } + let handle = unsafe { FindFirstFileW(wide.as_ptr(), &mut find_data) }.check_valid()?; unsafe { FindClose(handle) }; let mut info: BY_HANDLE_FILE_INFORMATION = unsafe { core::mem::zeroed() }; @@ -1135,15 +1121,13 @@ pub fn pipe() -> io::Result<(i32, i32)> { let (read_handle, write_handle) = unsafe { let mut read = core::mem::MaybeUninit::::uninit(); let mut write = core::mem::MaybeUninit::::uninit(); - let ok = CreatePipe( + CreatePipe( read.as_mut_ptr() as *mut _, write.as_mut_ptr() as *mut _, &mut attr as *mut _, 0, - ); - if ok == 0 { - return Err(io::Error::last_os_error()); - } + ) + .check_win32_bool()?; (read.assume_init(), write.assume_init()) }; @@ -1186,17 +1170,15 @@ pub fn mkdir(path: &widestring::WideCStr, mode: i32) -> io::Result<()> { let sddl: Vec = "D:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;OW)\0" .encode_utf16() .collect(); - let convert_ok = unsafe { + unsafe { ConvertStringSecurityDescriptorToSecurityDescriptorW( sddl.as_ptr(), SDDL_REVISION_1, &mut sec_attr.lpSecurityDescriptor, core::ptr::null_mut(), ) - }; - if convert_ok == 0 { - return Err(io::Error::last_os_error()); } + .check_win32_bool()?; let ok = unsafe { windows_sys::Win32::Storage::FileSystem::CreateDirectoryW( path.as_ptr(), @@ -1214,11 +1196,7 @@ pub fn mkdir(path: &widestring::WideCStr, mode: i32) -> io::Result<()> { } }; - if ok == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + ok.check_win32_bool() } unsafe extern "C" { @@ -1363,18 +1341,11 @@ pub fn kill(pid: u32, sig: u32) -> io::Result<()> { Ok(()) } } else { - let handle = unsafe { Threading::OpenProcess(Threading::PROCESS_ALL_ACCESS, 0, pid) }; - if handle.is_null() { - return Err(io::Error::last_os_error()); - } - let ok = unsafe { Threading::TerminateProcess(handle, sig) }; - let err = if ok == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - }; + let handle = unsafe { Threading::OpenProcess(Threading::PROCESS_ALL_ACCESS, 0, pid) } + .check_nonnull()?; + let result = unsafe { Threading::TerminateProcess(handle, sig) }.check_win32_bool(); unsafe { CloseHandle(handle) }; - err + result } } @@ -1392,10 +1363,8 @@ pub fn getfinalpathname(path: &Path) -> io::Result { FILE_FLAG_BACKUP_SEMANTICS, core::ptr::null_mut(), ) - }; - if handle == INVALID_HANDLE_VALUE { - return Err(io::Error::last_os_error()); } + .check_valid()?; let mut buffer = vec![0u16; MAX_PATH as usize]; let result = loop { @@ -1430,10 +1399,8 @@ pub fn getfullpathname(path: &Path) -> io::Result { buffer.as_mut_ptr(), core::ptr::null_mut(), ) - }; - if ret == 0 { - return Err(io::Error::last_os_error()); } + .check_ne(0)?; if ret as usize > buffer.len() { buffer.resize(ret as usize, 0); ret = unsafe { @@ -1443,11 +1410,10 @@ pub fn getfullpathname(path: &Path) -> io::Result { buffer.as_mut_ptr(), core::ptr::null_mut(), ) - }; - if ret == 0 { - return Err(io::Error::last_os_error()); } + .check_ne(0)?; } + let _ = ret; Ok(widestring::WideCString::from_vec_truncate(buffer).to_os_string()) } @@ -1455,18 +1421,15 @@ pub fn getvolumepathname(path: &Path) -> io::Result { let wide = path.as_os_str().to_wide_with_nul(); let buflen = core::cmp::max(wide.len(), MAX_PATH as usize); let mut buffer = vec![0u16; buflen]; - let ok = unsafe { + unsafe { windows_sys::Win32::Storage::FileSystem::GetVolumePathNameW( wide.as_ptr(), buffer.as_mut_ptr(), buflen as u32, ) - }; - if ok == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(widestring::WideCString::from_vec_truncate(buffer).to_os_string()) } + .check_win32_bool()?; + Ok(widestring::WideCString::from_vec_truncate(buffer).to_os_string()) } pub fn getdiskusage(path: &Path) -> io::Result<(u64, u64)> { diff --git a/crates/host_env/src/windows.rs b/crates/host_env/src/windows.rs index 949e746c9ee..8cb9019e580 100644 --- a/crates/host_env/src/windows.rs +++ b/crates/host_env/src/windows.rs @@ -97,6 +97,25 @@ impl CheckWin32Handle for windows_sys::Win32::Foundation::HANDLE { } } +/// Generic sentinel check for Win32 return values not covered by [`CheckWin32Bool`] or +/// [`CheckWin32Handle`]. Use for APIs that signal failure with a specific value +/// (`INVALID_FILE_ATTRIBUTES`, `WAIT_FAILED`, `GetFileVersionInfoSizeW` returning `0`, etc.). +pub trait CheckWin32Sentinel: Sized + Copy + PartialEq { + /// Returns `Ok(self)` if `self != fail`, otherwise the last OS error. + #[inline] + fn check_ne(self, fail: Self) -> io::Result { + if self == fail { + Err(io::Error::last_os_error()) + } else { + Ok(self) + } + } +} + +impl CheckWin32Sentinel for u32 {} +impl CheckWin32Sentinel for i32 {} +impl CheckWin32Sentinel for u64 {} + #[derive(Clone, Debug)] pub struct WindowsVersionInfo { pub major: u32, From f1532e203361232b90e13e942e36bc7539906ba6 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sun, 17 May 2026 01:04:57 +0900 Subject: [PATCH 12/23] Add OwnedHandle / HandleToOwned helper; apply to mmap create_named_mapping leak path --- crates/host_env/src/mmap.rs | 21 ++++++++++++++++----- crates/host_env/src/windows.rs | 23 +++++++++++++++++++++++ 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/crates/host_env/src/mmap.rs b/crates/host_env/src/mmap.rs index 44e3fbecf1b..0a9adc04fbd 100644 --- a/crates/host_env/src/mmap.rs +++ b/crates/host_env/src/mmap.rs @@ -6,11 +6,13 @@ use std::io; #[cfg(windows)] -use crate::windows::{CheckWin32Bool, CheckWin32Handle}; +use crate::windows::{CheckWin32Bool, CheckWin32Handle, HandleToOwned}; #[cfg(unix)] use crate::{crt_fd, fileutils, posix}; use memmap2::{Mmap, MmapMut, MmapOptions}; #[cfg(windows)] +use std::os::windows::io::{AsRawHandle, IntoRawHandle}; +#[cfg(windows)] use windows_sys::Win32::{ Foundation::{ CloseHandle, DUPLICATE_SAME_ACCESS, DuplicateHandle, GetLastError, HANDLE, @@ -244,18 +246,27 @@ pub fn create_named_mapping( tag_wide.as_ptr(), ) } - .check_nonnull()?; + .into_owned() + .ok_or_else(io::Error::last_os_error)?; let off_hi = (offset as u64 >> 32) as u32; let off_lo = offset as u32; - let view = unsafe { MapViewOfFile(map_handle, desired_access, off_hi, off_lo, map_size) }; + let view = unsafe { + MapViewOfFile( + map_handle.as_raw_handle() as Handle, + desired_access, + off_hi, + off_lo, + map_size, + ) + }; if view.Value.is_null() { - unsafe { CloseHandle(map_handle) }; + // `map_handle` is closed automatically when dropped on this error path. return Err(io::Error::last_os_error()); } Ok(NamedMmap { - map_handle, + map_handle: map_handle.into_raw_handle() as Handle, view_ptr: view.Value as *mut u8, len: map_size, }) diff --git a/crates/host_env/src/windows.rs b/crates/host_env/src/windows.rs index 8cb9019e580..bde8d679737 100644 --- a/crates/host_env/src/windows.rs +++ b/crates/host_env/src/windows.rs @@ -116,6 +116,29 @@ impl CheckWin32Sentinel for u32 {} impl CheckWin32Sentinel for i32 {} impl CheckWin32Sentinel for u64 {} +use std::os::windows::io::FromRawHandle; +pub use std::os::windows::io::{BorrowedHandle, OwnedHandle}; + +/// Conversion of raw Win32 `HANDLE` into an [`OwnedHandle`] (RAII auto-close). +pub trait HandleToOwned: Sized { + /// Wraps the handle in an [`OwnedHandle`] that calls `CloseHandle` on drop. + /// Returns `None` if the handle is NULL or `INVALID_HANDLE_VALUE`. + fn into_owned(self) -> Option; +} + +impl HandleToOwned for windows_sys::Win32::Foundation::HANDLE { + #[inline] + fn into_owned(self) -> Option { + if self.is_null() || self == windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE { + None + } else { + // SAFETY: caller is asserting via the public `Create*`/`Open*` paths + // that this handle is owned and unaliased. + Some(unsafe { OwnedHandle::from_raw_handle(self.cast()) }) + } + } +} + #[derive(Clone, Debug)] pub struct WindowsVersionInfo { pub major: u32, From e768278e520ce1eb0d8b2b13524d3921a260d770 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sun, 17 May 2026 01:06:13 +0900 Subject: [PATCH 13/23] Use OwnedHandle RAII in nt::pipe to eliminate manual cleanup on error path --- crates/host_env/src/nt.rs | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/crates/host_env/src/nt.rs b/crates/host_env/src/nt.rs index 541ea771bc6..bb1d547dfb3 100644 --- a/crates/host_env/src/nt.rs +++ b/crates/host_env/src/nt.rs @@ -1109,6 +1109,8 @@ pub fn fd_exists(fd: crate::crt_fd::Borrowed<'_>) -> bool { } pub fn pipe() -> io::Result<(i32, i32)> { + use crate::windows::HandleToOwned; + use std::os::windows::io::{AsRawHandle, IntoRawHandle}; use windows_sys::Win32::Security::SECURITY_ATTRIBUTES; use windows_sys::Win32::System::Pipes::CreatePipe; @@ -1128,25 +1130,32 @@ pub fn pipe() -> io::Result<(i32, i32)> { 0, ) .check_win32_bool()?; - (read.assume_init(), write.assume_init()) + (read.assume_init() as HANDLE, write.assume_init() as HANDLE) }; + // RAII wrappers: both handles are auto-closed on any early return below. + let read_handle = read_handle + .into_owned() + .expect("CreatePipe returned valid read handle"); + let write_handle = write_handle + .into_owned() + .expect("CreatePipe returned valid write handle"); const O_NOINHERIT: i32 = 0x80; - let read_fd = match crate::msvcrt::open_osfhandle(read_handle, O_NOINHERIT) { - Ok(fd) => fd, - Err(err) => { - unsafe { - CloseHandle(read_handle as _); - CloseHandle(write_handle as _); - } - return Err(err); + let read_fd = crate::msvcrt::open_osfhandle(read_handle.as_raw_handle() as isize, O_NOINHERIT)?; + // Ownership of the read handle now belongs to the CRT fd. + let _ = read_handle.into_raw_handle(); + + let write_fd = match crate::msvcrt::open_osfhandle( + write_handle.as_raw_handle() as isize, + libc::O_WRONLY | O_NOINHERIT, + ) { + Ok(fd) => { + let _ = write_handle.into_raw_handle(); + fd } - }; - let write_fd = match crate::msvcrt::open_osfhandle(write_handle, libc::O_WRONLY | O_NOINHERIT) { - Ok(fd) => fd, Err(err) => { + // Close the CRT fd we already created; `write_handle` auto-closes via Drop. let _ = unsafe { crt_fd::Owned::from_raw(read_fd) }; - unsafe { CloseHandle(write_handle as _) }; return Err(err); } }; From b730f7eb65e655a87f1c214d1d91776dc361b621 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sun, 17 May 2026 01:07:57 +0900 Subject: [PATCH 14/23] Use OwnedHandle in nt::chmod_follow; hoist HandleToOwned import --- crates/host_env/src/nt.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/crates/host_env/src/nt.rs b/crates/host_env/src/nt.rs index bb1d547dfb3..805786e5458 100644 --- a/crates/host_env/src/nt.rs +++ b/crates/host_env/src/nt.rs @@ -19,7 +19,7 @@ use crate::{ StatStruct, windows::{FILE_INFO_BY_NAME_CLASS, get_file_information_by_name, stat_basic_info_to_stat}, }, - windows::{CheckWin32Bool, CheckWin32Handle, CheckWin32Sentinel, ToWideString}, + windows::{CheckWin32Bool, CheckWin32Handle, CheckWin32Sentinel, HandleToOwned, ToWideString}, }; use libc::intptr_t; use windows_sys::Win32::{ @@ -408,12 +408,9 @@ pub fn chmod_follow(path: &widestring::WideCStr, mode: u32, write_bit: u32) -> i core::ptr::null_mut(), ) }; - if handle == INVALID_HANDLE_VALUE { - return Err(io::Error::last_os_error()); - } - let result = win32_hchmod(handle, mode, write_bit); - unsafe { CloseHandle(handle) }; - result + use std::os::windows::io::AsRawHandle; + let handle = handle.into_owned().ok_or_else(io::Error::last_os_error)?; + win32_hchmod(handle.as_raw_handle() as HANDLE, mode, write_bit) } pub fn find_first_file_name(path: &Path) -> io::Result { @@ -1109,7 +1106,6 @@ pub fn fd_exists(fd: crate::crt_fd::Borrowed<'_>) -> bool { } pub fn pipe() -> io::Result<(i32, i32)> { - use crate::windows::HandleToOwned; use std::os::windows::io::{AsRawHandle, IntoRawHandle}; use windows_sys::Win32::Security::SECURITY_ATTRIBUTES; use windows_sys::Win32::System::Pipes::CreatePipe; From 67ece5b16eef4b4652990e7e68e45e68749e3df0 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sun, 17 May 2026 20:47:02 +0900 Subject: [PATCH 15/23] Drop rustix dependency from vm crate Remove unused IntoPyException impl for rustix::io::Errno and the rustix entry in crates/vm/Cargo.toml. rustix is now only depended on by host_env. --- Cargo.lock | 1 - crates/vm/Cargo.toml | 1 - crates/vm/src/exceptions.rs | 9 ++------- 3 files changed, 2 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 27940fad212..b583b654fce 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3619,7 +3619,6 @@ dependencies = [ "paste", "psm", "result-like", - "rustix", "rustpython-codegen", "rustpython-common", "rustpython-compiler", diff --git a/crates/vm/Cargo.toml b/crates/vm/Cargo.toml index b8acf23a2c0..b0028ec5489 100644 --- a/crates/vm/Cargo.toml +++ b/crates/vm/Cargo.toml @@ -87,7 +87,6 @@ icu_properties = { workspace = true } writeable = { workspace = true } [target.'cfg(unix)'.dependencies] -rustix = { workspace = true } exitcode = { workspace = true } [target.'cfg(not(target_arch = "wasm32"))'.dependencies] diff --git a/crates/vm/src/exceptions.rs b/crates/vm/src/exceptions.rs index 7f1b3cc7a14..fa7d05946ff 100644 --- a/crates/vm/src/exceptions.rs +++ b/crates/vm/src/exceptions.rs @@ -1433,13 +1433,6 @@ impl IntoPyException for std::io::Error { } } -#[cfg(unix)] -impl IntoPyException for rustix::io::Errno { - fn into_pyexception(self, vm: &VirtualMachine) -> PyBaseExceptionRef { - std::io::Error::from(self).into_pyexception(vm) - } -} - #[cfg(not(any(target_os = "wasi", target_os = "redox")))] impl ToPyException for rustpython_host_env::fcntl::LockfError { fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { @@ -1463,6 +1456,7 @@ impl ToPyException for rustpython_host_env::posix::AccessError { } } +#[cfg(all(unix, not(target_os = "redox")))] impl ToPyException for rustpython_host_env::socket::AncillaryPackError { fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { match self { @@ -1475,6 +1469,7 @@ impl ToPyException for rustpython_host_env::socket::AncillaryPackError { } } +#[cfg(any(unix, windows))] impl ToPyException for rustpython_host_env::time::CheckedTmError { fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { match self { From a61d9bd5779ae9582353e783e4eede64a96c8c93 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Tue, 19 May 2026 15:21:40 +0900 Subject: [PATCH 16/23] Fix CI failures: cross-platform regressions - winapi.rs: pass None to create_event_w; the recent Option<&WideCStr> migration left one call site still passing a raw null pointer. - exceptions.rs: gate ToPyException for LockfError with cfg(any(unix, target_os = "wasi")), matching host_env::fcntl's own cfg. The previous cfg let it compile on wasm32-unknown-unknown where host_env::fcntl does not exist. - io_unsupported.rs: derive Eq on FileMode alongside PartialEq to satisfy clippy::derive_partial_eq_without_eq. --- crates/host_env/src/io_unsupported.rs | 2 +- crates/host_env/src/winapi.rs | 2 +- crates/vm/src/exceptions.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/host_env/src/io_unsupported.rs b/crates/host_env/src/io_unsupported.rs index a9a138b7afc..e46f05af900 100644 --- a/crates/host_env/src/io_unsupported.rs +++ b/crates/host_env/src/io_unsupported.rs @@ -17,7 +17,7 @@ const O_TRUNC: i32 = 0x0400; const O_EXCL: i32 = 0x0800; bitflags::bitflags! { - #[derive(Copy, Clone, Debug, PartialEq)] + #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub struct FileMode: u8 { const CREATED = 0b0001; const READABLE = 0b0010; diff --git a/crates/host_env/src/winapi.rs b/crates/host_env/src/winapi.rs index b1f08932ae0..5557e56bc41 100644 --- a/crates/host_env/src/winapi.rs +++ b/crates/host_env/src/winapi.rs @@ -537,7 +537,7 @@ pub fn batched_wait_for_multiple_objects( }; } - let cancel_event = create_event_w(true, false, core::ptr::null()) + let cancel_event = create_event_w(true, false, None) .map_err(|err| BatchedWaitError::Os(err.raw_os_error().unwrap_or_default() as u32))?; struct BatchData { diff --git a/crates/vm/src/exceptions.rs b/crates/vm/src/exceptions.rs index fa7d05946ff..1e017e1e5cb 100644 --- a/crates/vm/src/exceptions.rs +++ b/crates/vm/src/exceptions.rs @@ -1433,7 +1433,7 @@ impl IntoPyException for std::io::Error { } } -#[cfg(not(any(target_os = "wasi", target_os = "redox")))] +#[cfg(any(unix, target_os = "wasi"))] impl ToPyException for rustpython_host_env::fcntl::LockfError { fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { match self { From 3320c2a33561759de19f7014dbb0355b972e0312 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Tue, 19 May 2026 19:29:56 +0900 Subject: [PATCH 17/23] Fix CI failures: cfg gates and unused imports - exceptions.rs: gate ToPyException for LockfError with cfg(all(unix, not(target_os = "redox"))) to match the type's own cfg in host_env/src/fcntl.rs (LockfError is not built on wasi). - signal.rs: CheckLibcResult is only used in unix-gated functions; split import so it is not pulled in for windows. - mmap.rs: remove CheckWin32Handle from imports; no longer used after switching to HandleToOwned-based RAII. - overlapped.rs: remove INVALID_HANDLE_VALUE from connect_pipe import; the call now uses .check_valid(). --- crates/host_env/src/mmap.rs | 2 +- crates/host_env/src/overlapped.rs | 2 +- crates/host_env/src/signal.rs | 4 +++- crates/vm/src/exceptions.rs | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/host_env/src/mmap.rs b/crates/host_env/src/mmap.rs index 0a9adc04fbd..62dc50f1c31 100644 --- a/crates/host_env/src/mmap.rs +++ b/crates/host_env/src/mmap.rs @@ -6,7 +6,7 @@ use std::io; #[cfg(windows)] -use crate::windows::{CheckWin32Bool, CheckWin32Handle, HandleToOwned}; +use crate::windows::{CheckWin32Bool, HandleToOwned}; #[cfg(unix)] use crate::{crt_fd, fileutils, posix}; use memmap2::{Mmap, MmapMut, MmapOptions}; diff --git a/crates/host_env/src/overlapped.rs b/crates/host_env/src/overlapped.rs index 360bf49302d..d84897d620d 100644 --- a/crates/host_env/src/overlapped.rs +++ b/crates/host_env/src/overlapped.rs @@ -790,7 +790,7 @@ pub fn start_wsa_recv_from( pub fn connect_pipe(address: &str) -> io::Result { use windows_sys::Win32::{ - Foundation::{GENERIC_READ, GENERIC_WRITE, INVALID_HANDLE_VALUE}, + Foundation::{GENERIC_READ, GENERIC_WRITE}, Storage::FileSystem::{CreateFileW, FILE_FLAG_OVERLAPPED, OPEN_EXISTING}, }; diff --git a/crates/host_env/src/signal.rs b/crates/host_env/src/signal.rs index bf1f2e975b1..b1f5dc64c62 100644 --- a/crates/host_env/src/signal.rs +++ b/crates/host_env/src/signal.rs @@ -3,7 +3,9 @@ use std::io; use std::sync::Once; #[cfg(any(unix, windows))] -use crate::os::{CheckLibcResult, CheckLibcZero}; +use crate::os::CheckLibcZero; +#[cfg(unix)] +use crate::os::CheckLibcResult; #[cfg(any(unix, windows))] pub use libc::sighandler_t; diff --git a/crates/vm/src/exceptions.rs b/crates/vm/src/exceptions.rs index 1e017e1e5cb..a15d970826d 100644 --- a/crates/vm/src/exceptions.rs +++ b/crates/vm/src/exceptions.rs @@ -1433,7 +1433,7 @@ impl IntoPyException for std::io::Error { } } -#[cfg(any(unix, target_os = "wasi"))] +#[cfg(all(unix, not(target_os = "redox")))] impl ToPyException for rustpython_host_env::fcntl::LockfError { fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { match self { From e9ec6e8810ab05f21fc8ceb2c506766a382cfd99 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Tue, 19 May 2026 20:09:55 +0900 Subject: [PATCH 18/23] Fix CI failures: rustfmt and windows unused import - signal.rs: reorder cfg-gated imports per rustfmt. - socket.rs: gate ToPyException import to cfg(all(unix, not(target_os = "redox"))); it is only used inside sendmsg which has the same gate, so it was unused on windows. --- crates/host_env/src/signal.rs | 4 ++-- crates/stdlib/src/socket.rs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/host_env/src/signal.rs b/crates/host_env/src/signal.rs index b1f5dc64c62..f9f2a8206b9 100644 --- a/crates/host_env/src/signal.rs +++ b/crates/host_env/src/signal.rs @@ -2,10 +2,10 @@ use std::io; #[cfg(windows)] use std::sync::Once; -#[cfg(any(unix, windows))] -use crate::os::CheckLibcZero; #[cfg(unix)] use crate::os::CheckLibcResult; +#[cfg(any(unix, windows))] +use crate::os::CheckLibcZero; #[cfg(any(unix, windows))] pub use libc::sighandler_t; diff --git a/crates/stdlib/src/socket.rs b/crates/stdlib/src/socket.rs index f1ba092ab08..55aae79df61 100644 --- a/crates/stdlib/src/socket.rs +++ b/crates/stdlib/src/socket.rs @@ -8,15 +8,15 @@ pub(super) use _socket::{PySocket, SelectKind, sock_select, timeout_error_msg}; #[pymodule] mod _socket { use crate::common::lock::{PyMappedRwLockReadGuard, PyRwLock, PyRwLockReadGuard}; + #[cfg(all(unix, not(target_os = "redox")))] + use crate::vm::convert::ToPyException; use crate::vm::{ AsObject, Py, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, builtins::{ PyBaseExceptionRef, PyListRef, PyModule, PyOSError, PyStrRef, PyTupleRef, PyTypeRef, PyUtf8StrRef, }, - convert::{ - IntoPyException, ToPyException, ToPyObject, TryFromBorrowedObject, TryFromObject, - }, + convert::{IntoPyException, ToPyObject, TryFromBorrowedObject, TryFromObject}, function::{ ArgBytesLike, ArgIntoFloat, ArgMemoryBuffer, ArgStrOrBytesLike, Either, FsPath, OptionalArg, OptionalOption, From ba3f4fa579ffa869271684041e8e68b5a038c9e7 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Wed, 20 May 2026 00:35:13 +0900 Subject: [PATCH 19/23] Push remaining libc/extern callsites from vm into host_env Add host_env wrappers and replace the corresponding vm call sites: - host_env::errno::strerror_string for libc::strerror - host_env::io::write_stderr_raw for libc::write(STDERR_FILENO,...) - host_env::locale::localeconv_data reused from vm::format - host_env::os::abort for the inline abort extern - host_env::os::urandom wraps getrandom; getrandom moves from vm to host_env - host_env::posix::lchmod for the macOS/BSD lchmod extern - host_env::posix::fcopyfile for the macOS fcopyfile extern - host_env::nt::wputenv for the Windows _wputenv extern vm/format.rs's get_locale_info now uses host_env on both unix and windows instead of the unix-only libc::localeconv path. --- Cargo.lock | 2 +- crates/host_env/Cargo.toml | 1 + crates/host_env/src/errno.rs | 13 ++++++++ crates/host_env/src/io.rs | 10 ++++++ crates/host_env/src/nt.rs | 12 +++++++ crates/host_env/src/os.rs | 15 +++++++++ crates/host_env/src/posix.rs | 30 +++++++++++++++++ crates/vm/Cargo.toml | 1 - crates/vm/src/exceptions.rs | 24 ++++--------- crates/vm/src/format.rs | 63 +++++++++-------------------------- crates/vm/src/stdlib/os.rs | 28 +++++----------- crates/vm/src/stdlib/posix.rs | 31 +++-------------- crates/vm/src/vm/mod.rs | 4 +-- 13 files changed, 116 insertions(+), 118 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b583b654fce..671350409ce 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3352,6 +3352,7 @@ name = "rustpython-host_env" version = "0.5.0" dependencies = [ "bitflags 2.11.1", + "getrandom 0.3.4", "junction", "libc", "libffi", @@ -3596,7 +3597,6 @@ dependencies = [ "exitcode", "flame", "flamer", - "getrandom 0.3.4", "glob", "half", "hex", diff --git a/crates/host_env/Cargo.toml b/crates/host_env/Cargo.toml index 399c3a79fdf..718e3f41aab 100644 --- a/crates/host_env/Cargo.toml +++ b/crates/host_env/Cargo.toml @@ -12,6 +12,7 @@ license.workspace = true rustpython-wtf8 = { workspace = true } bitflags = { workspace = true } +getrandom = { workspace = true } libc = { workspace = true } num-traits = { workspace = true } parking_lot = { workspace = true } diff --git a/crates/host_env/src/errno.rs b/crates/host_env/src/errno.rs index da8edf3e41a..e816a04a55f 100644 --- a/crates/host_env/src/errno.rs +++ b/crates/host_env/src/errno.rs @@ -1,5 +1,18 @@ // spell-checker:disable +/// Return the platform `strerror(errno)` message as an owned `String`. +/// Returns `None` when the runtime gives no description for `errno`. +#[cfg(any(unix, windows))] +#[must_use] +pub fn strerror_string(errno: i32) -> Option { + let ptr = unsafe { libc::strerror(errno) }; + if ptr.is_null() { + return None; + } + let s = unsafe { core::ffi::CStr::from_ptr(ptr) }.to_string_lossy(); + Some(s.into_owned()) +} + #[cfg(any(unix, windows, target_os = "wasi"))] pub mod errors { pub use libc::*; diff --git a/crates/host_env/src/io.rs b/crates/host_env/src/io.rs index 49eaeddcb62..4ae1e4b3641 100644 --- a/crates/host_env/src/io.rs +++ b/crates/host_env/src/io.rs @@ -252,3 +252,13 @@ pub fn write_once(fd: crt_fd::Borrowed<'_>, buf: &[u8]) -> io::Result { pub fn close_owned_fd(fd: crt_fd::Owned) -> io::Result<()> { crt_fd::close(fd) } + +/// Async-signal-safe raw write to the platform stderr file descriptor. +/// Avoids `std::io::stderr()` locking so it is safe to call from fork +/// children and signal handlers. +#[cfg(unix)] +pub fn write_stderr_raw(buf: &[u8]) { + unsafe { + let _ = libc::write(libc::STDERR_FILENO, buf.as_ptr().cast(), buf.len()); + } +} diff --git a/crates/host_env/src/nt.rs b/crates/host_env/src/nt.rs index 805786e5458..4c77b30e616 100644 --- a/crates/host_env/src/nt.rs +++ b/crates/host_env/src/nt.rs @@ -1206,6 +1206,7 @@ pub fn mkdir(path: &widestring::WideCStr, mode: i32) -> io::Result<()> { unsafe extern "C" { fn _umask(mask: i32) -> i32; + fn _wputenv(envstring: *const u16) -> libc::c_int; } pub fn umask(mask: i32) -> io::Result { @@ -1217,6 +1218,17 @@ pub fn umask(mask: i32) -> io::Result { } } +/// Update the CRT environment via `_wputenv`. +/// `envstring` must point to a nul-terminated wide string of the form `KEY=value`. +pub fn wputenv(envstring: &widestring::WideCStr) -> io::Result<()> { + let result = unsafe { crate::suppress_iph!(_wputenv(envstring.as_ptr())) }; + if result != 0 { + Err(crate::os::errno_io_error()) + } else { + Ok(()) + } +} + fn set_fd_inheritable(fd: i32, inheritable: bool) -> io::Result<()> { let borrowed = unsafe { crt_fd::Borrowed::borrow_raw(fd) }; let handle = crt_fd::as_handle(borrowed)?; diff --git a/crates/host_env/src/os.rs b/crates/host_env/src/os.rs index ab94b6b9ad7..8afa07aea45 100644 --- a/crates/host_env/src/os.rs +++ b/crates/host_env/src/os.rs @@ -223,6 +223,21 @@ pub fn exit(code: i32) -> ! { std::process::exit(code) } +/// Wrapper around the C `abort()` call: terminates the process abnormally. +pub fn abort() -> ! { + unsafe extern "C" { + fn abort() -> !; + } + unsafe { abort() } +} + +/// Read `size` cryptographically random bytes from the OS. +pub fn urandom(size: usize) -> io::Result> { + let mut buf = vec![0u8; size]; + getrandom::fill(&mut buf).map_err(io::Error::from)?; + Ok(buf) +} + #[cfg(any(unix, windows, target_os = "wasi"))] pub fn isatty(fd: i32) -> bool { unsafe { suppress_iph!(libc::isatty(fd)) != 0 } diff --git a/crates/host_env/src/posix.rs b/crates/host_env/src/posix.rs index 1133026beb3..f10e99ac938 100644 --- a/crates/host_env/src/posix.rs +++ b/crates/host_env/src/posix.rs @@ -144,6 +144,36 @@ pub fn unlinkat(dir_fd: i32, path: &CStr) -> std::io::Result<()> { } } +#[cfg(any(target_os = "macos", target_os = "freebsd", target_os = "netbsd"))] +pub fn lchmod(path: &CStr, mode: libc::mode_t) -> std::io::Result<()> { + unsafe extern "C" { + fn lchmod(path: *const libc::c_char, mode: libc::mode_t) -> libc::c_int; + } + if unsafe { lchmod(path.as_ptr(), mode) } == 0 { + Ok(()) + } else { + Err(std::io::Error::last_os_error()) + } +} + +#[cfg(target_os = "macos")] +pub fn fcopyfile(in_fd: i32, out_fd: i32, flags: u32) -> std::io::Result<()> { + unsafe extern "C" { + fn fcopyfile( + in_fd: libc::c_int, + out_fd: libc::c_int, + state: *mut libc::c_void, + flags: u32, + ) -> libc::c_int; + } + let ret = unsafe { fcopyfile(in_fd, out_fd, core::ptr::null_mut(), flags) }; + if ret < 0 { + Err(std::io::Error::last_os_error()) + } else { + Ok(()) + } +} + #[cfg(not(windows))] pub fn make_dir(path: &CStr, mode: u32) -> std::io::Result<()> { let ret = unsafe { libc::mkdir(path.as_ptr(), mode as _) }; diff --git a/crates/vm/Cargo.toml b/crates/vm/Cargo.toml index b0028ec5489..8ff078b818f 100644 --- a/crates/vm/Cargo.toml +++ b/crates/vm/Cargo.toml @@ -51,7 +51,6 @@ crossbeam-utils = { workspace = true } chrono = { workspace = true } constant_time_eq = { workspace = true } flame = { workspace = true, optional = true } -getrandom = { workspace = true } hex = { workspace = true } indexmap = { workspace = true } itertools = { workspace = true } diff --git a/crates/vm/src/exceptions.rs b/crates/vm/src/exceptions.rs index a15d970826d..eb39d13fe40 100644 --- a/crates/vm/src/exceptions.rs +++ b/crates/vm/src/exceptions.rs @@ -1385,28 +1385,16 @@ impl ToOSErrorBuilder for std::io::Error { // Use C runtime's strerror for POSIX errno values. // For Windows-specific error codes, fall back to FormatMessage. const MAX_POSIX_ERRNO: i32 = 127; - if errno > 0 && errno <= MAX_POSIX_ERRNO { - let ptr = unsafe { libc::strerror(errno) }; - if !ptr.is_null() { - let s = unsafe { core::ffi::CStr::from_ptr(ptr) }.to_string_lossy(); - if !s.starts_with("Unknown error") { - break 'msg s.into_owned(); - } - } + if errno > 0 && errno <= MAX_POSIX_ERRNO + && let Some(s) = crate::host_env::errno::strerror_string(errno) + && !s.starts_with("Unknown error") + { + break 'msg s; } self.to_string() }; #[cfg(unix)] - let msg = { - let ptr = unsafe { libc::strerror(errno) }; - if !ptr.is_null() { - unsafe { core::ffi::CStr::from_ptr(ptr) } - .to_string_lossy() - .into_owned() - } else { - self.to_string() - } - }; + let msg = crate::host_env::errno::strerror_string(errno).unwrap_or_else(|| self.to_string()); #[cfg(not(any(windows, unix)))] let msg = self.to_string(); diff --git a/crates/vm/src/format.rs b/crates/vm/src/format.rs index 77322d0629a..2e3d6ad48ad 100644 --- a/crates/vm/src/format.rs +++ b/crates/vm/src/format.rs @@ -10,34 +10,25 @@ use crate::common::format::*; use crate::common::wtf8::{Wtf8, Wtf8Buf}; /// Get locale information from C `localeconv()` for the 'n' format specifier. -#[cfg(unix)] +#[cfg(any(unix, windows))] pub(crate) fn get_locale_info() -> LocaleInfo { - use core::ffi::CStr; - unsafe { - let lc = libc::localeconv(); - if lc.is_null() { - return LocaleInfo { - thousands_sep: String::new(), - decimal_point: ".".to_string(), - grouping: vec![], - }; - } - let thousands_sep = CStr::from_ptr((*lc).thousands_sep) - .to_string_lossy() - .into_owned(); - let decimal_point = CStr::from_ptr((*lc).decimal_point) - .to_string_lossy() - .into_owned(); - let grouping = parse_grouping((*lc).grouping); - LocaleInfo { - thousands_sep, - decimal_point, - grouping, - } + let lc = crate::host_env::locale::localeconv_data(); + let mut grouping: Vec = lc.grouping.iter().map(|&c| c as u8).collect(); + if !grouping.is_empty() { + grouping.push(0); + } + LocaleInfo { + thousands_sep: String::from_utf8_lossy(&lc.thousands_sep).into_owned(), + decimal_point: if lc.decimal_point.is_empty() { + ".".to_string() + } else { + String::from_utf8_lossy(&lc.decimal_point).into_owned() + }, + grouping, } } -#[cfg(not(unix))] +#[cfg(not(any(unix, windows)))] pub(crate) fn get_locale_info() -> LocaleInfo { LocaleInfo { thousands_sep: String::new(), @@ -46,30 +37,6 @@ pub(crate) fn get_locale_info() -> LocaleInfo { } } -/// Parse C `lconv.grouping` into a `Vec`. -/// Reads bytes until 0 or CHAR_MAX, then appends 0 (meaning "repeat last group"). -#[cfg(unix)] -unsafe fn parse_grouping(grouping: *const libc::c_char) -> Vec { - let mut result = Vec::new(); - if grouping.is_null() { - return result; - } - - unsafe { - let mut ptr = grouping; - while ![0, libc::c_char::MAX].contains(&*ptr) { - result.push(*ptr as _); - ptr = ptr.add(1); - } - } - - if !result.is_empty() { - result.push(0); - } - - result -} - impl IntoPyException for FormatSpecError { fn into_pyexception(self, vm: &VirtualMachine) -> PyBaseExceptionRef { match self { diff --git a/crates/vm/src/stdlib/os.rs b/crates/vm/src/stdlib/os.rs index f17add06899..3ce80ef6a75 100644 --- a/crates/vm/src/stdlib/os.rs +++ b/crates/vm/src/stdlib/os.rs @@ -466,11 +466,6 @@ pub(super) mod _os { } } - #[cfg(windows)] - unsafe extern "C" { - fn _wputenv(envstring: *const u16) -> libc::c_int; - } - /// Check if wide string length exceeds Windows environment variable limit. #[cfg(windows)] fn check_env_var_len(wide_len: usize, vm: &VirtualMachine) -> PyResult<()> { @@ -498,15 +493,13 @@ pub(super) mod _os { return Err(vm.new_value_error("illegal environment variable name")); } let env_str = format!("{key_str}={value_str}"); - let wide = env_str.to_wide_with_nul(); - check_env_var_len(wide.len(), vm)?; + // env_str is guaranteed nul-free by the checks above. + let wide = widestring::WideCString::from_str(&env_str) + .expect("env_str validated to contain no NUL"); + check_env_var_len(wide.len() + 1, vm)?; - // Use _wputenv like CPython (not SetEnvironmentVariableW) to update CRT environ - let result = unsafe { rustpython_host_env::suppress_iph!(_wputenv(wide.as_ptr())) }; - if result != 0 { - return Err(vm.new_last_errno_error()); - } - Ok(()) + // Use _wputenv (not SetEnvironmentVariableW) to update CRT environ. + rustpython_host_env::nt::wputenv(&wide).map_err(|e| e.into_pyexception(vm)) } #[cfg(not(windows))] @@ -1434,10 +1427,7 @@ pub(super) mod _os { #[pyfunction] fn abort() { - unsafe extern "C" { - fn abort(); - } - unsafe { abort() } + crate::host_env::os::abort() } #[pyfunction] @@ -1445,9 +1435,7 @@ pub(super) mod _os { if size < 0 { return Err(vm.new_value_error("negative argument not allowed")); } - let mut buf = vec![0u8; size as usize]; - getrandom::fill(&mut buf).map_err(|e| io::Error::from(e).into_pyexception(vm))?; - Ok(buf) + crate::host_env::os::urandom(size as usize).map_err(|e| e.into_pyexception(vm)) } #[pyfunction] diff --git a/crates/vm/src/stdlib/posix.rs b/crates/vm/src/stdlib/posix.rs index 51723e59d6c..b45f31821c7 100644 --- a/crates/vm/src/stdlib/posix.rs +++ b/crates/vm/src/stdlib/posix.rs @@ -999,16 +999,9 @@ pub mod module { #[cfg(any(target_os = "macos", target_os = "freebsd", target_os = "netbsd",))] #[pyfunction] fn lchmod(path: OsPath, mode: u32, vm: &VirtualMachine) -> PyResult<()> { - unsafe extern "C" { - fn lchmod(path: *const libc::c_char, mode: libc::mode_t) -> libc::c_int; - } let c_path = path.clone().into_cstring(vm)?; - if unsafe { lchmod(c_path.as_ptr(), mode as libc::mode_t) } == 0 { - Ok(()) - } else { - let err = std::io::Error::last_os_error(); - Err(OSErrorBuilder::with_filename(&err, path, vm)) - } + rustpython_host_env::posix::lchmod(&c_path, mode as libc::mode_t) + .map_err(|err| OSErrorBuilder::with_filename(&err, path, vm)) } #[pyfunction] @@ -1660,27 +1653,11 @@ pub mod module { Ok(_os::TerminalSizeData { columns, lines }) } - // from libstd: - // https://github.com/rust-lang/rust/blob/daecab3a784f28082df90cebb204998051f3557d/src/libstd/sys/unix/fs.rs#L1251 - #[cfg(target_os = "macos")] - unsafe extern "C" { - fn fcopyfile( - in_fd: libc::c_int, - out_fd: libc::c_int, - state: *mut libc::c_void, // copyfile_state_t (unused) - flags: u32, // copyfile_flags_t - ) -> libc::c_int; - } - #[cfg(target_os = "macos")] #[pyfunction] fn _fcopyfile(in_fd: i32, out_fd: i32, flags: i32, vm: &VirtualMachine) -> PyResult<()> { - let ret = unsafe { fcopyfile(in_fd, out_fd, core::ptr::null_mut(), flags as u32) }; - if ret < 0 { - Err(vm.new_last_errno_error()) - } else { - Ok(()) - } + rustpython_host_env::posix::fcopyfile(in_fd, out_fd, flags as u32) + .map_err(|err| err.into_pyexception(vm)) } #[pyfunction] diff --git a/crates/vm/src/vm/mod.rs b/crates/vm/src/vm/mod.rs index 5e82c1b6d0c..85416a09129 100644 --- a/crates/vm/src/vm/mod.rs +++ b/crates/vm/src/vm/mod.rs @@ -572,9 +572,7 @@ pub(super) fn stw_trace(msg: core::fmt::Arguments<'_>) { crate::stdlib::_thread::get_ident(), msg ); - unsafe { - let _ = libc::write(libc::STDERR_FILENO, out.buf.as_ptr().cast(), out.len); - } + crate::host_env::io::write_stderr_raw(&out.buf[..out.len]); } } From 45c83dd3f704803c111062a97f63f6a6680d2f45 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Wed, 20 May 2026 00:45:46 +0900 Subject: [PATCH 20/23] Move time tz state and winsound FFI into host_env - host_env::time::tz: wraps the libc tzset/timezone/daylight/tzname globals on non-msvc, non-wasm32 targets. vm::stdlib::time now reads these via the typed wrappers instead of declaring its own externs. - host_env::winsound (windows): exposes PlaySoundW (via a typed PlaySoundSource enum), Beep, and MessageBeep. vm::stdlib::winsound drops its inline FFI block and routes through host_env. --- crates/host_env/src/lib.rs | 2 ++ crates/host_env/src/time.rs | 48 ++++++++++++++++++++++++++ crates/host_env/src/winsound.rs | 58 ++++++++++++++++++++++++++++++++ crates/vm/src/stdlib/time.rs | 33 +++--------------- crates/vm/src/stdlib/winsound.rs | 52 +++++++++------------------- 5 files changed, 129 insertions(+), 64 deletions(-) create mode 100644 crates/host_env/src/winsound.rs diff --git a/crates/host_env/src/lib.rs b/crates/host_env/src/lib.rs index 66016345abd..99f67b2b496 100644 --- a/crates/host_env/src/lib.rs +++ b/crates/host_env/src/lib.rs @@ -83,4 +83,6 @@ pub mod winapi; #[cfg(windows)] pub mod winreg; #[cfg(windows)] +pub mod winsound; +#[cfg(windows)] pub mod wmi; diff --git a/crates/host_env/src/time.rs b/crates/host_env/src/time.rs index cac3c795b2b..3e3227d78de 100644 --- a/crates/host_env/src/time.rs +++ b/crates/host_env/src/time.rs @@ -15,6 +15,54 @@ pub const SEC_TO_NS: i64 = SEC_TO_MS * MS_TO_NS; pub const NS_TO_MS: i64 = 1000 * 1000; pub const NS_TO_US: i64 = 1000; +/// Access to the C runtime's `tzset` / `timezone` / `daylight` / `tzname` +/// globals used by Python's `time` module. +/// +/// Not available under MSVC (which exposes these only via the +/// `_get_tzname`-style helpers) or on `wasm32` (no libc tz state). +#[cfg(all(not(target_env = "msvc"), not(target_arch = "wasm32")))] +pub mod tz { + unsafe extern "C" { + #[cfg(not(target_os = "freebsd"))] + #[link_name = "daylight"] + static c_daylight: core::ffi::c_int; + #[link_name = "timezone"] + static c_timezone: core::ffi::c_long; + #[link_name = "tzname"] + static c_tzname: [*const core::ffi::c_char; 2]; + #[link_name = "tzset"] + fn c_tzset(); + } + + pub fn tzset() { + unsafe { c_tzset() } + } + + #[must_use] + pub fn timezone() -> core::ffi::c_long { + unsafe { c_timezone } + } + + #[cfg(not(target_os = "freebsd"))] + #[must_use] + pub fn daylight() -> core::ffi::c_int { + unsafe { c_daylight } + } + + /// Snapshot of `tzname[0]` / `tzname[1]` as owned `String`s. + /// Reads the C globals once and copies the bytes out so callers don't + /// have to handle the raw pointers themselves. + #[must_use] + pub fn tzname_strings() -> (String, String) { + unsafe fn to_str(s: *const core::ffi::c_char) -> String { + unsafe { core::ffi::CStr::from_ptr(s) } + .to_string_lossy() + .into_owned() + } + unsafe { (to_str(c_tzname[0]), to_str(c_tzname[1])) } + } +} + pub fn duration_since_system_now() -> Result { SystemTime::now().duration_since(UNIX_EPOCH) } diff --git a/crates/host_env/src/winsound.rs b/crates/host_env/src/winsound.rs new file mode 100644 index 00000000000..cf40881391b --- /dev/null +++ b/crates/host_env/src/winsound.rs @@ -0,0 +1,58 @@ +// spell-checker:ignore pszSound fdwSound winmm + +use std::io; + +#[link(name = "winmm")] +unsafe extern "system" { + fn PlaySoundW(pszSound: *const u16, hmod: isize, fdwSound: u32) -> i32; +} + +unsafe extern "system" { + fn Beep(dwFreq: u32, dwDuration: u32) -> i32; + fn MessageBeep(uType: u32) -> i32; +} + +/// Source for a `PlaySound` call. +pub enum PlaySoundSource<'a> { + /// Stop currently playing sound (NULL `pszSound`). + Stop, + /// Play sound data from memory; pass with `SND_MEMORY` set in `flags`. + Memory(&'a [u8]), + /// Play sound by filename or system alias. + Name(&'a widestring::WideCStr), +} + +/// Returns `Ok(())` when `PlaySoundW` returns non-zero, an error otherwise. +pub fn play_sound(source: PlaySoundSource<'_>, flags: u32) -> Result<(), PlaySoundError> { + let ptr: *const u16 = match source { + PlaySoundSource::Stop => core::ptr::null(), + PlaySoundSource::Memory(buf) => buf.as_ptr().cast(), + PlaySoundSource::Name(s) => s.as_ptr(), + }; + let ok = unsafe { PlaySoundW(ptr, 0, flags) }; + if ok == 0 { + Err(PlaySoundError) + } else { + Ok(()) + } +} + +/// `PlaySoundW` returned 0; there is no documented errno for this path. +#[derive(Debug, Clone, Copy)] +pub struct PlaySoundError; + +/// `Beep(freq, duration)`. `false` on failure. +#[must_use] +pub fn beep(frequency: u32, duration_ms: u32) -> bool { + unsafe { Beep(frequency, duration_ms) != 0 } +} + +/// `MessageBeep(type)`. On failure returns `Err` populated from `GetLastError`. +pub fn message_beep(beep_type: u32) -> io::Result<()> { + let ok = unsafe { MessageBeep(beep_type) }; + if ok == 0 { + Err(io::Error::last_os_error()) + } else { + Ok(()) + } +} diff --git a/crates/vm/src/stdlib/time.rs b/crates/vm/src/stdlib/time.rs index e2cc8536d88..93d04341d26 100644 --- a/crates/vm/src/stdlib/time.rs +++ b/crates/vm/src/stdlib/time.rs @@ -8,21 +8,6 @@ pub use decl::time; pub(crate) use decl::module_def; -#[cfg(not(target_env = "msvc"))] -#[cfg(not(target_arch = "wasm32"))] -unsafe extern "C" { - #[cfg(not(target_os = "freebsd"))] - #[link_name = "daylight"] - static c_daylight: core::ffi::c_int; - // pub static dstbias: std::ffi::c_int; - #[link_name = "timezone"] - static c_timezone: core::ffi::c_long; - #[link_name = "tzname"] - static c_tzname: [*const core::ffi::c_char; 2]; - #[link_name = "tzset"] - fn c_tzset(); -} - #[pymodule(name = "time", with(#[cfg(any(unix, windows))] platform))] mod decl { #![allow(unreachable_pub)] @@ -222,7 +207,7 @@ mod decl { #[pyattr] fn altzone(_vm: &VirtualMachine) -> core::ffi::c_long { // TODO: RUSTPYTHON; Add support for using the C altzone - unsafe { super::c_timezone - 3600 } + crate::host_env::time::tz::timezone() - 3600 } #[cfg(target_env = "msvc")] @@ -238,7 +223,7 @@ mod decl { #[cfg(not(target_arch = "wasm32"))] #[pyattr] fn timezone(_vm: &VirtualMachine) -> core::ffi::c_long { - unsafe { super::c_timezone } + crate::host_env::time::tz::timezone() } #[cfg(target_env = "msvc")] @@ -255,7 +240,7 @@ mod decl { #[cfg(not(target_arch = "wasm32"))] #[pyattr] fn daylight(_vm: &VirtualMachine) -> core::ffi::c_int { - unsafe { super::c_daylight } + crate::host_env::time::tz::daylight() } #[cfg(target_env = "msvc")] @@ -272,13 +257,7 @@ mod decl { #[pyattr] fn tzname(vm: &VirtualMachine) -> crate::builtins::PyTupleRef { use crate::builtins::tuple::IntoPyTuple; - - unsafe fn to_str(s: *const core::ffi::c_char) -> String { - unsafe { core::ffi::CStr::from_ptr(s) } - .to_string_lossy() - .into_owned() - } - unsafe { (to_str(super::c_tzname[0]), to_str(super::c_tzname[1])) }.into_pytuple(vm) + crate::host_env::time::tz::tzname_strings().into_pytuple(vm) } #[cfg(target_env = "msvc")] @@ -917,9 +896,7 @@ mod decl { ) -> PyResult<()> { #[cfg(not(target_env = "msvc"))] #[cfg(not(target_arch = "wasm32"))] - unsafe { - super::c_tzset() - }; + crate::host_env::time::tz::tzset(); __module_exec(vm, module); Ok(()) diff --git a/crates/vm/src/stdlib/winsound.rs b/crates/vm/src/stdlib/winsound.rs index 359d3967a99..b121dd6a934 100644 --- a/crates/vm/src/stdlib/winsound.rs +++ b/crates/vm/src/stdlib/winsound.rs @@ -3,18 +3,6 @@ pub(crate) use winsound::module_def; -mod win32 { - #[link(name = "winmm")] - unsafe extern "system" { - pub(super) fn PlaySoundW(pszSound: *const u16, hmod: isize, fdwSound: u32) -> i32; - } - - unsafe extern "system" { - pub(super) fn Beep(dwFreq: u32, dwDuration: u32) -> i32; - pub(super) fn MessageBeep(uType: u32) -> i32; - } -} - #[pymodule] mod winsound { use crate::builtins::{PyBytes, PyStr}; @@ -22,6 +10,7 @@ mod winsound { use crate::host_env::windows::ToWideString; use crate::protocol::PyBuffer; use crate::{AsObject, PyObjectRef, PyResult, VirtualMachine}; + use rustpython_host_env::winsound::{PlaySoundSource, play_sound}; // PlaySound flags #[pyattr] @@ -79,17 +68,18 @@ mod winsound { flags: i32, } + fn map_play_err(vm: &VirtualMachine) -> impl FnOnce(rustpython_host_env::winsound::PlaySoundError) -> crate::builtins::PyBaseExceptionRef + '_ + { + |_| vm.new_runtime_error("Failed to play sound") + } + #[pyfunction] fn PlaySound(args: PlaySoundArgs, vm: &VirtualMachine) -> PyResult<()> { let sound = args.sound; let flags = args.flags as u32; if vm.is_none(&sound) { - let ok = unsafe { super::win32::PlaySoundW(core::ptr::null(), 0, flags) }; - if ok == 0 { - return Err(vm.new_runtime_error("Failed to play sound")); - } - return Ok(()); + return play_sound(PlaySoundSource::Stop, flags).map_err(map_play_err(vm)); } if flags & SND_MEMORY != 0 { @@ -100,11 +90,7 @@ mod winsound { let buf = buffer .as_contiguous() .ok_or_else(|| vm.new_type_error("a bytes-like object is required, not 'str'"))?; - let ok = unsafe { super::win32::PlaySoundW(buf.as_ptr() as *const u16, 0, flags) }; - if ok == 0 { - return Err(vm.new_runtime_error("Failed to play sound")); - } - return Ok(()); + return play_sound(PlaySoundSource::Memory(&*buf), flags).map_err(map_play_err(vm)); } if sound.downcastable::() { @@ -157,11 +143,9 @@ mod winsound { } let wide = path.to_wide_with_nul(); - let ok = unsafe { super::win32::PlaySoundW(wide.as_ptr(), 0, flags) }; - if ok == 0 { - return Err(vm.new_runtime_error("Failed to play sound")); - } - Ok(()) + let wide_cstr = widestring::WideCStr::from_slice_truncate(&wide) + .map_err(|_| vm.new_value_error("embedded null character"))?; + play_sound(PlaySoundSource::Name(wide_cstr), flags).map_err(map_play_err(vm)) } #[derive(FromArgs)] @@ -178,11 +162,11 @@ mod winsound { return Err(vm.new_value_error("frequency must be in 37 thru 32767")); } - let ok = unsafe { super::win32::Beep(args.frequency as u32, args.duration as u32) }; - if ok == 0 { - return Err(vm.new_runtime_error("Failed to beep")); + if rustpython_host_env::winsound::beep(args.frequency as u32, args.duration as u32) { + Ok(()) + } else { + Err(vm.new_runtime_error("Failed to beep")) } - Ok(()) } #[derive(FromArgs)] @@ -193,10 +177,6 @@ mod winsound { #[pyfunction] fn MessageBeep(args: MessageBeepArgs, vm: &VirtualMachine) -> PyResult<()> { - let ok = unsafe { super::win32::MessageBeep(args.r#type) }; - if ok == 0 { - return Err(std::io::Error::last_os_error().into_pyexception(vm)); - } - Ok(()) + rustpython_host_env::winsound::message_beep(args.r#type).map_err(|e| e.into_pyexception(vm)) } } From 49c13f8f0f8be963d199cb81893e6944b15109ca Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Wed, 20 May 2026 10:12:33 +0900 Subject: [PATCH 21/23] Migrate unsetenv to host_env::nt::wputenv; rustfmt - vm::stdlib::os::unsetenv had a second _wputenv call site that still referenced the removed inline extern. Route it through host_env::nt::wputenv like putenv. - rustfmt fixups in exceptions.rs (boolean chain layout) and the two winsound files. --- crates/host_env/src/winsound.rs | 6 +----- crates/vm/src/exceptions.rs | 6 ++++-- crates/vm/src/stdlib/os.rs | 14 ++++++-------- crates/vm/src/stdlib/winsound.rs | 8 ++++++-- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/crates/host_env/src/winsound.rs b/crates/host_env/src/winsound.rs index cf40881391b..f00f9e20890 100644 --- a/crates/host_env/src/winsound.rs +++ b/crates/host_env/src/winsound.rs @@ -30,11 +30,7 @@ pub fn play_sound(source: PlaySoundSource<'_>, flags: u32) -> Result<(), PlaySou PlaySoundSource::Name(s) => s.as_ptr(), }; let ok = unsafe { PlaySoundW(ptr, 0, flags) }; - if ok == 0 { - Err(PlaySoundError) - } else { - Ok(()) - } + if ok == 0 { Err(PlaySoundError) } else { Ok(()) } } /// `PlaySoundW` returned 0; there is no documented errno for this path. diff --git a/crates/vm/src/exceptions.rs b/crates/vm/src/exceptions.rs index eb39d13fe40..df199801b8a 100644 --- a/crates/vm/src/exceptions.rs +++ b/crates/vm/src/exceptions.rs @@ -1385,7 +1385,8 @@ impl ToOSErrorBuilder for std::io::Error { // Use C runtime's strerror for POSIX errno values. // For Windows-specific error codes, fall back to FormatMessage. const MAX_POSIX_ERRNO: i32 = 127; - if errno > 0 && errno <= MAX_POSIX_ERRNO + if errno > 0 + && errno <= MAX_POSIX_ERRNO && let Some(s) = crate::host_env::errno::strerror_string(errno) && !s.starts_with("Unknown error") { @@ -1394,7 +1395,8 @@ impl ToOSErrorBuilder for std::io::Error { self.to_string() }; #[cfg(unix)] - let msg = crate::host_env::errno::strerror_string(errno).unwrap_or_else(|| self.to_string()); + let msg = + crate::host_env::errno::strerror_string(errno).unwrap_or_else(|| self.to_string()); #[cfg(not(any(windows, unix)))] let msg = self.to_string(); diff --git a/crates/vm/src/stdlib/os.rs b/crates/vm/src/stdlib/os.rs index 3ce80ef6a75..c9db2ebf106 100644 --- a/crates/vm/src/stdlib/os.rs +++ b/crates/vm/src/stdlib/os.rs @@ -538,15 +538,13 @@ pub(super) mod _os { } // "key=" to unset (empty value removes the variable) let env_str = format!("{key_str}="); - let wide = env_str.to_wide_with_nul(); - check_env_var_len(wide.len(), vm)?; + // env_str is guaranteed nul-free by the checks above. + let wide = widestring::WideCString::from_str(&env_str) + .expect("env_str validated to contain no NUL"); + check_env_var_len(wide.len() + 1, vm)?; - // Use _wputenv like CPython (not SetEnvironmentVariableW) to update CRT environ - let result = unsafe { rustpython_host_env::suppress_iph!(_wputenv(wide.as_ptr())) }; - if result != 0 { - return Err(vm.new_last_errno_error()); - } - Ok(()) + // Use _wputenv (not SetEnvironmentVariableW) to update CRT environ. + rustpython_host_env::nt::wputenv(&wide).map_err(|e| e.into_pyexception(vm)) } #[cfg(not(windows))] diff --git a/crates/vm/src/stdlib/winsound.rs b/crates/vm/src/stdlib/winsound.rs index b121dd6a934..6e3e43a5507 100644 --- a/crates/vm/src/stdlib/winsound.rs +++ b/crates/vm/src/stdlib/winsound.rs @@ -68,8 +68,12 @@ mod winsound { flags: i32, } - fn map_play_err(vm: &VirtualMachine) -> impl FnOnce(rustpython_host_env::winsound::PlaySoundError) -> crate::builtins::PyBaseExceptionRef + '_ - { + fn map_play_err( + vm: &VirtualMachine, + ) -> impl FnOnce( + rustpython_host_env::winsound::PlaySoundError, + ) -> crate::builtins::PyBaseExceptionRef + + '_ { |_| vm.new_runtime_error("Failed to play sound") } From 61e0fd4edde9aab7e6c614e0dda66b5303d617e5 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Wed, 20 May 2026 10:22:00 +0900 Subject: [PATCH 22/23] Address PR review comments - host_env::winapi::create_process: assert that the command_line buffer is NUL-terminated and that the env block ends with a double-NUL, matching the Win32 CreateProcessW contract. - stdlib::overlapped CreateEvent: replace WideCString::from_str_truncate with the fallible from_str(), so embedded NULs in the event name surface as ValueError instead of being silently truncated. - vm::exceptions::ReadlinkError::NotSymbolicLink now maps to OSError (matches Win32 ERROR_NOT_A_REPARSE_POINT semantics) rather than ValueError. - winreg::ConnectRegistry: route the non-zero return through the existing os_error_from_windows_code helper so the resulting exception carries the real winerror/message instead of a generic OSError. --- crates/host_env/src/winapi.rs | 30 ++++++++++++++++++++++++++++++ crates/stdlib/src/overlapped.rs | 7 ++++--- crates/vm/src/exceptions.rs | 2 +- crates/vm/src/stdlib/winreg.rs | 4 ++-- 4 files changed, 37 insertions(+), 6 deletions(-) diff --git a/crates/host_env/src/winapi.rs b/crates/host_env/src/winapi.rs index 5557e56bc41..bb6c2721da3 100644 --- a/crates/host_env/src/winapi.rs +++ b/crates/host_env/src/winapi.rs @@ -215,6 +215,29 @@ unsafe fn create_process_w_raw( Ok(unsafe { procinfo.assume_init() }) } +/// Win32 `CreateProcessW` requires `lpCommandLine` to be NUL-terminated. +/// The buffer is passed `&mut [u16]` because `CreateProcessW` may modify it +/// in place. +#[inline] +fn assert_command_line_terminated(buf: &[u16]) { + assert!( + buf.last() == Some(&0), + "command_line buffer passed to create_process must be NUL-terminated", + ); +} + +/// Win32 `CreateProcessW` with `CREATE_UNICODE_ENVIRONMENT` requires +/// `lpEnvironment` to be a sequence of `KEY=value\0` strings followed by a +/// final terminating `\0` — i.e. the block ends with two consecutive zero +/// `u16`s. +#[inline] +fn assert_environment_block_terminated(buf: &[u16]) { + assert!( + buf.len() >= 2 && buf[buf.len() - 2..] == [0, 0], + "env block passed to create_process must end with a double NUL terminator", + ); +} + #[allow( clippy::too_many_arguments, reason = "This is the semantic host wrapper for Win32 CreateProcess parameters." @@ -229,6 +252,13 @@ pub fn create_process( startup_info: StartupInfoData, handle_list: Option>, ) -> io::Result { + if let Some(cmd) = command_line.as_deref() { + assert_command_line_terminated(cmd); + } + if let Some(env_block) = env { + assert_environment_block_terminated(env_block); + } + let mut si: windows_sys::Win32::System::Threading::STARTUPINFOEXW = unsafe { core::mem::zeroed() }; si.StartupInfo.cb = core::mem::size_of_val(&si) as _; diff --git a/crates/stdlib/src/overlapped.rs b/crates/stdlib/src/overlapped.rs index 130d68e0d84..4ce3d3ba830 100644 --- a/crates/stdlib/src/overlapped.rs +++ b/crates/stdlib/src/overlapped.rs @@ -11,7 +11,7 @@ mod _overlapped { AsObject, Py, PyObjectRef, PyPayload, PyResult, VirtualMachine, builtins::{PyBaseExceptionRef, PyBytesRef, PyModule, PyStrRef, PyTupleRef, PyType}, common::lock::PyMutex, - convert::ToPyObject, + convert::{ToPyException, ToPyObject}, function::OptionalArg, object::{Traverse, TraverseFn}, protocol::PyBuffer, @@ -1251,8 +1251,9 @@ mod _overlapped { return Err(vm.new_value_error("EventAttributes must be None")); } - let name_wide: Option = - name.map(|n| widestring::WideCString::from_str_truncate(&n)); + let name_wide: Option = name + .map(|n| widestring::WideCString::from_str(&n).map_err(|err| err.to_pyexception(vm))) + .transpose()?; host_winapi::create_event_w(manual_reset, initial_state, name_wide.as_deref()) .map(|h| h as isize) .map_err(|err| set_from_windows_err(err.raw_os_error().unwrap_or(0) as u32, vm)) diff --git a/crates/vm/src/exceptions.rs b/crates/vm/src/exceptions.rs index df199801b8a..3e27fff54ab 100644 --- a/crates/vm/src/exceptions.rs +++ b/crates/vm/src/exceptions.rs @@ -1510,7 +1510,7 @@ impl ToPyException for rustpython_host_env::nt::ReadlinkError { fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { match self { Self::Io(err) => err.to_pyexception(vm), - Self::NotSymbolicLink => vm.new_value_error("not a symbolic link"), + Self::NotSymbolicLink => vm.new_os_error("The file or directory is not a reparse point"), Self::InvalidReparseData => vm.new_os_error("Invalid reparse data"), } } diff --git a/crates/vm/src/stdlib/winreg.rs b/crates/vm/src/stdlib/winreg.rs index 7aed53ab882..468767e9d38 100644 --- a/crates/vm/src/stdlib/winreg.rs +++ b/crates/vm/src/stdlib/winreg.rs @@ -267,7 +267,7 @@ mod winreg { if res == 0 { Ok(PyHkey::new(ret_key)) } else { - Err(vm.new_os_error(format!("error code: {res}"))) + Err(os_error_from_windows_code(vm, res as i32)) } } @@ -279,7 +279,7 @@ mod winreg { if res == 0 { Ok(PyHkey::new(out_key)) } else { - Err(vm.new_os_error(format!("error code: {res}"))) + Err(os_error_from_windows_code(vm, res as i32)) } } From 783c6b5064978907b1b4a4cf7121d13eeaafcde7 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Wed, 20 May 2026 14:16:50 +0900 Subject: [PATCH 23/23] Fix CI failures and address review follow-ups CI failures: - rustfmt cleanup in exceptions.rs after the ReadlinkError change. - vm/stdlib/os.rs: drop unused ToWideString import that the wputenv migration left behind. - vm/stdlib/winsound.rs: replace explicit `&*buf` with `&buf` to satisfy clippy::explicit_auto_deref. - Lib/test/test_format.py, Lib/test/test_types.py: drop the now-stale expectedFailureIfWindows decorators on the locale-format tests; the Windows path now reads real `localeconv` data via host_env so these tests pass. Review follow-ups: - host_env::winapi::create_process: switch the new buffer terminator checks from `assert!` to fallible validators returning `io::ErrorKind::InvalidInput`, so bad inputs stay recoverable at the API boundary. - host_env::winsound::play_sound: reject `Memory(_)` together with `SND_ASYNC` (lifetime-unsafe) and `SND_MEMORY` without a `Memory(_)` source. Expand `PlaySoundError` into a variant enum. - vm::stdlib::_winapi::CreateProcess: route the Win32 path/argv strings through `as_wtf8().to_wide_cstring()` like the rest of the Windows API surface; `expect_str()` could panic on Python strings containing lone surrogates. --- Lib/test/test_format.py | 1 - Lib/test/test_types.py | 2 -- crates/host_env/src/winapi.rs | 32 +++++++++++++++++++------------ crates/host_env/src/winsound.rs | 33 +++++++++++++++++++++++++++++--- crates/vm/src/exceptions.rs | 4 +++- crates/vm/src/stdlib/_winapi.rs | 14 ++++++-------- crates/vm/src/stdlib/os.rs | 2 -- crates/vm/src/stdlib/winsound.rs | 11 ++++++----- 8 files changed, 65 insertions(+), 34 deletions(-) diff --git a/Lib/test/test_format.py b/Lib/test/test_format.py index d53a3f46134..6868c87171d 100644 --- a/Lib/test/test_format.py +++ b/Lib/test/test_format.py @@ -423,7 +423,6 @@ def test_non_ascii(self): self.assertEqual(format(1+2j, "\u2007^8"), "\u2007(1+2j)\u2007") self.assertEqual(format(0j, "\u2007^4"), "\u20070j\u2007") - @unittest.expectedFailureIfWindows("TODO: RUSTPYTHON; AssertionError: ',' not found in '123456789'") def test_locale(self): try: oldloc = locale.setlocale(locale.LC_ALL) diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py index 272e01f15d2..01da70b4c68 100644 --- a/Lib/test/test_types.py +++ b/Lib/test/test_types.py @@ -431,7 +431,6 @@ def test(i, format_spec, result): test(123456, "1=20", '11111111111111123456') test(123456, "*=20", '**************123456') - @unittest.expectedFailureIfWindows("TODO: RUSTPYTHON; AssertionError: '1,234.57' != '1234.57'") @run_with_locale('LC_NUMERIC', 'en_US.UTF8', '') def test_float__format__locale(self): # test locale support for __format__ code 'n' @@ -441,7 +440,6 @@ def test_float__format__locale(self): self.assertEqual(locale.format_string('%g', x, grouping=True), format(x, 'n')) self.assertEqual(locale.format_string('%.10g', x, grouping=True), format(x, '.10n')) - @unittest.expectedFailureIfWindows("TODO: RUSTPYTHON; AssertionError: '123,456,789,012,345,678,901,234,567,890' != '123456789012345678901234567890'") @run_with_locale('LC_NUMERIC', 'en_US.UTF8', '') def test_int__format__locale(self): # test locale support for __format__ code 'n' for integers diff --git a/crates/host_env/src/winapi.rs b/crates/host_env/src/winapi.rs index bb6c2721da3..4e4536c3518 100644 --- a/crates/host_env/src/winapi.rs +++ b/crates/host_env/src/winapi.rs @@ -219,11 +219,15 @@ unsafe fn create_process_w_raw( /// The buffer is passed `&mut [u16]` because `CreateProcessW` may modify it /// in place. #[inline] -fn assert_command_line_terminated(buf: &[u16]) { - assert!( - buf.last() == Some(&0), - "command_line buffer passed to create_process must be NUL-terminated", - ); +fn validate_command_line_terminated(buf: &[u16]) -> io::Result<()> { + if buf.last() == Some(&0) { + Ok(()) + } else { + Err(io::Error::new( + io::ErrorKind::InvalidInput, + "command_line buffer passed to create_process must be NUL-terminated", + )) + } } /// Win32 `CreateProcessW` with `CREATE_UNICODE_ENVIRONMENT` requires @@ -231,11 +235,15 @@ fn assert_command_line_terminated(buf: &[u16]) { /// final terminating `\0` — i.e. the block ends with two consecutive zero /// `u16`s. #[inline] -fn assert_environment_block_terminated(buf: &[u16]) { - assert!( - buf.len() >= 2 && buf[buf.len() - 2..] == [0, 0], - "env block passed to create_process must end with a double NUL terminator", - ); +fn validate_environment_block_terminated(buf: &[u16]) -> io::Result<()> { + if buf.len() >= 2 && buf[buf.len() - 2..] == [0, 0] { + Ok(()) + } else { + Err(io::Error::new( + io::ErrorKind::InvalidInput, + "env block passed to create_process must end with a double NUL terminator", + )) + } } #[allow( @@ -253,10 +261,10 @@ pub fn create_process( handle_list: Option>, ) -> io::Result { if let Some(cmd) = command_line.as_deref() { - assert_command_line_terminated(cmd); + validate_command_line_terminated(cmd)?; } if let Some(env_block) = env { - assert_environment_block_terminated(env_block); + validate_environment_block_terminated(env_block)?; } let mut si: windows_sys::Win32::System::Threading::STARTUPINFOEXW = diff --git a/crates/host_env/src/winsound.rs b/crates/host_env/src/winsound.rs index f00f9e20890..07fc5751df1 100644 --- a/crates/host_env/src/winsound.rs +++ b/crates/host_env/src/winsound.rs @@ -12,6 +12,11 @@ unsafe extern "system" { fn MessageBeep(uType: u32) -> i32; } +/// `SND_ASYNC` flag value from `mmsystem.h`. +const SND_ASYNC: u32 = 0x0001; +/// `SND_MEMORY` flag value from `mmsystem.h`. +const SND_MEMORY: u32 = 0x0004; + /// Source for a `PlaySound` call. pub enum PlaySoundSource<'a> { /// Stop currently playing sound (NULL `pszSound`). @@ -23,19 +28,41 @@ pub enum PlaySoundSource<'a> { } /// Returns `Ok(())` when `PlaySoundW` returns non-zero, an error otherwise. +/// +/// Rejects `Memory(_)` together with `SND_ASYNC` because async playback +/// requires the buffer to outlive the call; combining them with a borrowed +/// slice would let WinMM read freed memory. pub fn play_sound(source: PlaySoundSource<'_>, flags: u32) -> Result<(), PlaySoundError> { + if matches!(source, PlaySoundSource::Memory(_)) && flags & SND_ASYNC != 0 { + return Err(PlaySoundError::MemoryAsyncRejected); + } + // `SND_MEMORY` requires a `Memory(_)` source; an empty pointer would + // dereference garbage. + if !matches!(source, PlaySoundSource::Memory(_)) && flags & SND_MEMORY != 0 { + return Err(PlaySoundError::MemoryFlagWithoutBuffer); + } let ptr: *const u16 = match source { PlaySoundSource::Stop => core::ptr::null(), PlaySoundSource::Memory(buf) => buf.as_ptr().cast(), PlaySoundSource::Name(s) => s.as_ptr(), }; let ok = unsafe { PlaySoundW(ptr, 0, flags) }; - if ok == 0 { Err(PlaySoundError) } else { Ok(()) } + if ok == 0 { + Err(PlaySoundError::CallFailed) + } else { + Ok(()) + } } -/// `PlaySoundW` returned 0; there is no documented errno for this path. #[derive(Debug, Clone, Copy)] -pub struct PlaySoundError; +pub enum PlaySoundError { + /// `PlaySoundW` returned 0; there is no documented errno for this path. + CallFailed, + /// `Memory(_)` source cannot be combined with `SND_ASYNC` in the safe API. + MemoryAsyncRejected, + /// `SND_MEMORY` set in `flags` but no `Memory(_)` buffer supplied. + MemoryFlagWithoutBuffer, +} /// `Beep(freq, duration)`. `false` on failure. #[must_use] diff --git a/crates/vm/src/exceptions.rs b/crates/vm/src/exceptions.rs index 3e27fff54ab..b91e567e731 100644 --- a/crates/vm/src/exceptions.rs +++ b/crates/vm/src/exceptions.rs @@ -1510,7 +1510,9 @@ impl ToPyException for rustpython_host_env::nt::ReadlinkError { fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { match self { Self::Io(err) => err.to_pyexception(vm), - Self::NotSymbolicLink => vm.new_os_error("The file or directory is not a reparse point"), + Self::NotSymbolicLink => { + vm.new_os_error("The file or directory is not a reparse point") + } Self::InvalidReparseData => vm.new_os_error("Invalid reparse data"), } } diff --git a/crates/vm/src/stdlib/_winapi.rs b/crates/vm/src/stdlib/_winapi.rs index 6f7a338f26e..f6faf6a8a95 100644 --- a/crates/vm/src/stdlib/_winapi.rs +++ b/crates/vm/src/stdlib/_winapi.rs @@ -230,11 +230,8 @@ mod _winapi { let handle_list = get_handle_list(args.startup_info.get_attr("lpAttributeList", vm)?, vm)?; - let wcstring = |s: PyStrRef| { - widestring::WideCString::from_str(s.expect_str()).map_err(|err| err.to_pyexception(vm)) - }; - // Validate no embedded null bytes in command name and command line + // before handing the strings off; to_wide_cstring truncates at NUL. if let Some(ref name) = args.name && name.as_bytes().contains(&0) { @@ -246,12 +243,13 @@ mod _winapi { return Err(crate::exceptions::cstring_error(vm)); } - let app_name = args.name.map(wcstring).transpose()?; - let current_dir = args.current_dir.map(wcstring).transpose()?; + let wcstring = |s: PyStrRef| s.as_wtf8().to_wide_cstring(); + let app_name = args.name.as_ref().map(|s| wcstring(s.clone())); + let current_dir = args.current_dir.as_ref().map(|s| wcstring(s.clone())); let mut command_line = args .command_line - .map(|s| wcstring(s).map(|w| w.into_vec_with_nul())) - .transpose()?; + .as_ref() + .map(|s| wcstring(s.clone()).into_vec_with_nul()); let procinfo = host_winapi::create_process( app_name.as_deref(), diff --git a/crates/vm/src/stdlib/os.rs b/crates/vm/src/stdlib/os.rs index c9db2ebf106..917d9a71978 100644 --- a/crates/vm/src/stdlib/os.rs +++ b/crates/vm/src/stdlib/os.rs @@ -156,8 +156,6 @@ impl ToPyObject for crt_fd::Borrowed<'_> { pub(super) mod _os { use super::{DirFd, FollowSymlinks, SupportFunc}; use crate::host_env::fileutils::StatStruct; - #[cfg(windows)] - use crate::host_env::windows::ToWideString; #[cfg(any(unix, windows))] use crate::utils::ToCString; use crate::{ diff --git a/crates/vm/src/stdlib/winsound.rs b/crates/vm/src/stdlib/winsound.rs index 6e3e43a5507..e8214a8fd19 100644 --- a/crates/vm/src/stdlib/winsound.rs +++ b/crates/vm/src/stdlib/winsound.rs @@ -74,7 +74,11 @@ mod winsound { rustpython_host_env::winsound::PlaySoundError, ) -> crate::builtins::PyBaseExceptionRef + '_ { - |_| vm.new_runtime_error("Failed to play sound") + use rustpython_host_env::winsound::PlaySoundError::*; + |err| match err { + MemoryAsyncRejected => vm.new_runtime_error("Cannot play asynchronously from memory"), + MemoryFlagWithoutBuffer | CallFailed => vm.new_runtime_error("Failed to play sound"), + } } #[pyfunction] @@ -87,14 +91,11 @@ mod winsound { } if flags & SND_MEMORY != 0 { - if flags & SND_ASYNC != 0 { - return Err(vm.new_runtime_error("Cannot play asynchronously from memory")); - } let buffer = PyBuffer::try_from_borrowed_object(vm, &sound)?; let buf = buffer .as_contiguous() .ok_or_else(|| vm.new_type_error("a bytes-like object is required, not 'str'"))?; - return play_sound(PlaySoundSource::Memory(&*buf), flags).map_err(map_play_err(vm)); + return play_sound(PlaySoundSource::Memory(&buf), flags).map_err(map_play_err(vm)); } if sound.downcastable::() {