From 6be27def807c018b8c7573d000e5abc890b12aa9 Mon Sep 17 00:00:00 2001 From: yyc12345 Date: Mon, 27 Oct 2025 20:49:04 +0800 Subject: [PATCH] refactor(windows): update Windows API calls and error handling - Replace `Default::default()` with `std::ptr::null_mut()` or `std::ptr::null()` for FFI calls - Change `WideCStr::from_ptr` to `WideStr::from_ptr` for better safety - Remove redundant error variants in `LoadStrRcError` and `ExpandEnvVarError` - Reorder imports for better readability - Add test cases for `StrRc` and `ExpandString` utilities - Improve documentation comments for icon loading functions --- wfassoc/src/extra/windows.rs | 50 ++++++++++++++++++---------------- wfassoc/src/utilities.rs | 4 +-- wfassoc/tests/extra_windows.rs | 18 +++++++++++- 3 files changed, 46 insertions(+), 26 deletions(-) diff --git a/wfassoc/src/extra/windows.rs b/wfassoc/src/extra/windows.rs index 8ab5e9b..238563e 100644 --- a/wfassoc/src/extra/windows.rs +++ b/wfassoc/src/extra/windows.rs @@ -9,7 +9,7 @@ use std::path::Path; use std::str::FromStr; use std::sync::LazyLock; use thiserror::Error as TeError; -use widestring::{WideCStr, WideCString, WideChar}; +use widestring::{WideCString, WideChar, WideStr}; use windows_sys::Win32::UI::WindowsAndMessaging::HICON; // region: Windows Resource Reference String @@ -217,7 +217,7 @@ pub struct IconRc { impl IconRc { /// Load icon from executable or `.ico` file. - /// + /// /// If you want to extract icon from `.ico` file, please pass `0` to `index` parameter. /// Otherwise `index` is the icon resource index located in executable. pub fn new(file: &Path, index: u32, kind: IconSizeKind) -> Result { @@ -231,10 +231,10 @@ impl IconRc { let rv = unsafe { match kind { IconSizeKind::Small => { - ExtractIconExW(file.as_ptr(), index, Default::default(), icon_ptr, 1) + ExtractIconExW(file.as_ptr(), index, std::ptr::null_mut(), icon_ptr, 1) } IconSizeKind::Large => { - ExtractIconExW(file.as_ptr(), index, icon_ptr, Default::default(), 1) + ExtractIconExW(file.as_ptr(), index, icon_ptr, std::ptr::null_mut(), 1) } } }; @@ -247,7 +247,7 @@ impl IconRc { } /// An alias to default constructor. - /// It automatically handle the index parameter for you + /// It automatically handle the index parameter for you /// when loading `.ico` file, rather than executable file. pub fn with_ico_file(file: &Path, kind: IconSizeKind) -> Result { Self::new(file, 0, kind) @@ -290,8 +290,6 @@ impl Drop for IconRc { pub enum LoadStrRcError { /// Given path has embedded NUL. EmbeddedNul(#[from] widestring::error::ContainsNul), - /// The C-format string is invalid. - BadString(#[from] widestring::error::NulError), /// The encoding of string is invalid. BadEncoding(#[from] widestring::error::Utf16Error), /// Error when casting integer @@ -303,33 +301,41 @@ pub enum LoadStrRcError { } pub struct StrRc { - inner: String + inner: String, } impl StrRc { pub fn new(file: &Path, index: u32) -> Result { - use windows_sys::core::PWSTR; - use windows_sys::Win32::UI::WindowsAndMessaging::LoadStringW; use windows_sys::Win32::Foundation::FreeLibrary; - use windows_sys::Win32::System::LibraryLoader::{LoadLibraryExW, LOAD_LIBRARY_AS_DATAFILE, LOAD_LIBRARY_AS_IMAGE_RESOURCE}; + use windows_sys::Win32::System::LibraryLoader::{ + LOAD_LIBRARY_AS_DATAFILE, LOAD_LIBRARY_AS_IMAGE_RESOURCE, LoadLibraryExW, + }; + use windows_sys::Win32::UI::WindowsAndMessaging::LoadStringW; + use windows_sys::core::PWSTR; // Load library first let file = WideCString::from_os_str(file.as_os_str())?; - let hmodule = unsafe { LoadLibraryExW(file.as_ptr(), Default::default(), LOAD_LIBRARY_AS_DATAFILE | LOAD_LIBRARY_AS_IMAGE_RESOURCE) }; + let hmodule = unsafe { + LoadLibraryExW( + file.as_ptr(), + std::ptr::null_mut(), + LOAD_LIBRARY_AS_DATAFILE | LOAD_LIBRARY_AS_IMAGE_RESOURCE, + ) + }; if hmodule.is_null() { return Err(LoadStrRcError::LoadLibrary); } // Load string - let mut buffer: *const u16 = Default::default(); - let buffer_ptr= &mut buffer as *mut *const u16 as PWSTR; + let mut buffer: *const u16 = std::ptr::null(); + let buffer_ptr = &mut buffer as *mut *const u16 as PWSTR; let char_count = unsafe { LoadStringW(hmodule, index, buffer_ptr, 0) }; // We write this function to make sure following "FreeLibrary" must be executed. fn load_string(buffer: *const u16, char_count: i32) -> Result { if char_count == 0 { Err(LoadStrRcError::LoadString) } else { - let buffer = unsafe { WideCStr::from_ptr(buffer, char_count.try_into()?)? }; + let buffer = unsafe { WideStr::from_ptr(buffer, char_count.try_into()?) }; Ok(buffer.to_string()?) } } @@ -372,16 +378,14 @@ impl ParseExpandStrError { pub enum ExpandEnvVarError { /// Given string has embedded NUL. EmbeddedNul(#[from] widestring::error::ContainsNul), - /// Error occurs when executing Win32 expand function. - ExpandFunction, + /// The encoding of string is invalid. + BadEncoding(#[from] widestring::error::Utf16Error), /// Error occurs when int type casting. BadIntCast(#[from] std::num::TryFromIntError), /// Integeral arithmatic downflow. Underflow, - /// The C-format string is invalid. - BadString(#[from] widestring::error::NulError), - /// The encoding of string is invalid. - BadEncoding(#[from] widestring::error::Utf16Error), + /// Error occurs when executing Win32 expand function. + ExpandFunction, /// Some environment vairable are not expanded. NoEnvVar, } @@ -410,7 +414,7 @@ impl ExpandString { // Fetch the size of expand result let source = WideCString::from_str(self.inner.as_str())?; - let size = unsafe { ExpandEnvironmentStringsW(source.as_ptr(), Default::default(), 0) }; + let size = unsafe { ExpandEnvironmentStringsW(source.as_ptr(), std::ptr::null_mut(), 0) }; if size == 0 { return Err(ExpandEnvVarError::ExpandFunction); } @@ -428,7 +432,7 @@ impl ExpandString { } // Cast result as Rust string - let wstr = unsafe { WideCStr::from_ptr(buffer.as_ptr(), len_no_nul)? }; + let wstr = unsafe { WideStr::from_ptr(buffer.as_ptr(), len_no_nul) }; let rv = wstr.to_string()?; // If the final string still has environment variable, diff --git a/wfassoc/src/utilities.rs b/wfassoc/src/utilities.rs index 7e4dfbc..aaba153 100644 --- a/wfassoc/src/utilities.rs +++ b/wfassoc/src/utilities.rs @@ -86,8 +86,8 @@ pub fn notify_assoc_changed() -> () { SHChangeNotify( SHCNE_ASSOCCHANGED as i32, SHCNF_IDLIST, - Default::default(), - Default::default(), + std::ptr::null(), + std::ptr::null(), ) } } diff --git a/wfassoc/tests/extra_windows.rs b/wfassoc/tests/extra_windows.rs index 3b542f0..032623e 100644 --- a/wfassoc/tests/extra_windows.rs +++ b/wfassoc/tests/extra_windows.rs @@ -48,16 +48,32 @@ fn test_icon_rc() { // We pick it from "jpegfile" ProgId tester("imageres.dll", 72); + tester("notepad.exe", 0); } #[test] fn test_str_rc() { + fn tester(file: &str, index: u32) { + let rv = StrRc::new(Path::new(file), index); + assert!(rv.is_ok()); + } + // We pick from "jpegfile" ProgId + tester("shell32.dll", 30596); } #[test] fn test_expand_string() { - + fn tester(s: &str) { + let rv = ExpandString::new(s); + assert!(rv.is_ok()); + let rv = rv.unwrap(); + + let rv = rv.expand_string(); + assert!(rv.is_ok()); + } + + tester(r#"%SystemRoot%\System32\shell32.dll"#); } #[test]