From cc060cb594bad6302dfb647c2699bb6b0e01a103 Mon Sep 17 00:00:00 2001 From: JamesLinYJ <110664404+JamesLinYJ@users.noreply.github.com> Date: Sun, 9 Aug 2026 20:30:09 +0800 Subject: [PATCH 1/3] Fix open issue root causes --- localization/de.toml | 1 + localization/en_us.toml | 1 + localization/es.toml | 1 + localization/fr.toml | 1 + localization/pt.toml | 1 + localization/ru.toml | 1 + localization/zh_cn.toml | 1 + localization/zh_tw.toml | 1 + src/app/controllers.rs | 34 +- src/app/mod.rs | 234 ++++- src/app/page_host.rs | 110 ++- src/app/single_instance.rs | 219 +++-- src/capabilities.rs | 2 +- src/infrastructure/diagnostics/crash.rs | 133 ++- src/infrastructure/diagnostics/secure_fs.rs | 231 ++++- src/infrastructure/native/errors.rs | 21 +- src/infrastructure/native/handles.rs | 90 +- src/infrastructure/native/mod.rs | 7 +- src/infrastructure/native/safety.rs | 8 +- src/infrastructure/native/ui.rs | 90 +- src/pages/applications/icons.rs | 218 +++-- src/pages/applications/mod.rs | 29 +- src/pages/cpu/mod.rs | 9 +- src/pages/network.rs | 480 +++++----- src/pages/processes/actions.rs | 949 ++++++++++---------- src/pages/processes/mod.rs | 293 +++--- src/pages/processes/sampler.rs | 25 +- src/pages/users.rs | 51 +- src/system/cpu_sets.rs | 67 +- src/system/process_identity.rs | 7 +- src/ui/assets.rs | 36 +- src/ui/localization/text_key.rs | 1 + 32 files changed, 2033 insertions(+), 1319 deletions(-) diff --git a/localization/de.toml b/localization/de.toml index a55a42f..da84052 100644 --- a/localization/de.toml +++ b/localization/de.toml @@ -154,6 +154,7 @@ Maximize = "Ma&ximieren" Cascade = "&Ueberlappend" BringToFront = "In den &Vordergrund" HelpTopics = "Task-Manager-&Hilfethemen" +HelpOpenFailed = "Die Task-Manager-Hilfe konnte nicht geöffnet werden." DiagnosticLogs = "&Diagnoseprotokolle..." DiagnosticLogsTitle = "Diagnoseprotokolle" DiagnosticStatusLabel = "Status:" diff --git a/localization/en_us.toml b/localization/en_us.toml index e511d97..4268616 100644 --- a/localization/en_us.toml +++ b/localization/en_us.toml @@ -161,6 +161,7 @@ Maximize = "Ma&ximize" Cascade = "&Cascade" BringToFront = "&Bring to Front" HelpTopics = "Task Manager &Help Topics" +HelpOpenFailed = "Task Manager Help could not be opened." DiagnosticLogs = "&Diagnostic Logs..." DiagnosticLogsTitle = "Diagnostic Logs" DiagnosticStatusLabel = "Status:" diff --git a/localization/es.toml b/localization/es.toml index 4fb4ef3..5aef75f 100644 --- a/localization/es.toml +++ b/localization/es.toml @@ -154,6 +154,7 @@ Maximize = "Ma&ximizar" Cascade = "&Cascada" BringToFront = "Traer al &frente" HelpTopics = "Temas de a&yuda del Administrador de tareas" +HelpOpenFailed = "No se pudo abrir la ayuda del Administrador de tareas." DiagnosticLogs = "Registros de &diagnóstico..." DiagnosticLogsTitle = "Registros de diagnóstico" DiagnosticStatusLabel = "Estado:" diff --git a/localization/fr.toml b/localization/fr.toml index fd16a8c..5160606 100644 --- a/localization/fr.toml +++ b/localization/fr.toml @@ -154,6 +154,7 @@ Maximize = "Ma&ximiser" Cascade = "&Cascade" BringToFront = "&Mettre au premier plan" HelpTopics = "&Rubriques d'aide du Gestionnaire des tâches" +HelpOpenFailed = "Impossible d’ouvrir l’aide du Gestionnaire des tâches." DiagnosticLogs = "Journaux de &diagnostic..." DiagnosticLogsTitle = "Journaux de diagnostic" DiagnosticStatusLabel = "État :" diff --git a/localization/pt.toml b/localization/pt.toml index 5fb25b1..3dab7d4 100644 --- a/localization/pt.toml +++ b/localization/pt.toml @@ -154,6 +154,7 @@ Maximize = "Ma&ximizar" Cascade = "&Em cascata" BringToFront = "Trazer para &frente" HelpTopics = "Tópicos de &ajuda do Gerenciador de Tarefas" +HelpOpenFailed = "Não foi possível abrir a ajuda do Gerenciador de Tarefas." DiagnosticLogs = "Logs de &diagnóstico..." DiagnosticLogsTitle = "Logs de diagnóstico" DiagnosticStatusLabel = "Status:" diff --git a/localization/ru.toml b/localization/ru.toml index 0cd458e..e006b2d 100644 --- a/localization/ru.toml +++ b/localization/ru.toml @@ -154,6 +154,7 @@ Maximize = "Ра&звернуть" Cascade = "&Каскадом" BringToFront = "На &передний план" HelpTopics = "&Разделы справки диспетчера задач" +HelpOpenFailed = "Не удалось открыть справку диспетчера задач." DiagnosticLogs = "Журналы &диагностики..." DiagnosticLogsTitle = "Журналы диагностики" DiagnosticStatusLabel = "Состояние:" diff --git a/localization/zh_cn.toml b/localization/zh_cn.toml index 5460ec3..716dcbc 100644 --- a/localization/zh_cn.toml +++ b/localization/zh_cn.toml @@ -154,6 +154,7 @@ Maximize = "最大化(&X)" Cascade = "层叠(&C)" BringToFront = "切换到前台(&B)" HelpTopics = "任务管理器帮助主题(&H)" +HelpOpenFailed = "无法打开任务管理器帮助。" DiagnosticLogs = "诊断日志(&D)..." DiagnosticLogsTitle = "诊断日志" DiagnosticStatusLabel = "状态:" diff --git a/localization/zh_tw.toml b/localization/zh_tw.toml index 979f7af..1d1cad4 100644 --- a/localization/zh_tw.toml +++ b/localization/zh_tw.toml @@ -154,6 +154,7 @@ Maximize = "最大化(&X)" Cascade = "重疊顯示(&C)" BringToFront = "帶到前景(&B)" HelpTopics = "工作管理員說明主題(&H)" +HelpOpenFailed = "無法開啟工作管理員說明。" DiagnosticLogs = "診斷記錄(&D)..." DiagnosticLogsTitle = "診斷記錄" DiagnosticStatusLabel = "狀態:" diff --git a/src/app/controllers.rs b/src/app/controllers.rs index 5d5cf10..7e32f92 100644 --- a/src/app/controllers.rs +++ b/src/app/controllers.rs @@ -27,7 +27,7 @@ use windows_sys::Win32::UI::Shell::{ use windows_sys::Win32::UI::WindowsAndMessaging::{HICON, HMENU, LR_DEFAULTCOLOR, LR_DEFAULTSIZE}; use crate::infrastructure::native::{ - destroy_icon_handle, format_resource_string, record_win32_error, to_wide_null, + OwnedIcon, format_resource_string, record_win32_error, to_wide_null, }; use crate::system::sampler::SystemSample; use crate::ui::assets::{TRAY_CPU_ICON_RESOURCES, load_icon_resource}; @@ -59,7 +59,7 @@ impl RuntimeStatsController { /// 托盘图标控制器。 /// 管理 12 级 CPU 占用图标和通知区域提示文本。 pub struct TrayController { - icons: Vec, + icons: Vec, registered: Cell, last_error: Cell>, } @@ -78,20 +78,12 @@ impl TrayController { pub fn load_icons(&mut self) -> Result<(), u32> { let mut loaded = Vec::with_capacity(TRAY_CPU_ICON_RESOURCES.len()); for resource_name in TRAY_CPU_ICON_RESOURCES { - let icon_handle = - load_icon_resource(resource_name, 0, 0, LR_DEFAULTCOLOR | LR_DEFAULTSIZE); - if icon_handle.is_null() { - let error = unsafe { windows_sys::Win32::Foundation::GetLastError() }; - for icon in loaded { - destroy_icon_handle(icon); - } - return Err(if error == 0 { - windows_sys::Win32::Foundation::ERROR_RESOURCE_DATA_NOT_FOUND - } else { - error - }); - } - loaded.push(icon_handle); + loaded.push(load_icon_resource( + resource_name, + 0, + 0, + LR_DEFAULTCOLOR | LR_DEFAULTSIZE, + )?); } self.clear_icons(); self.icons = loaded; @@ -99,7 +91,7 @@ impl TrayController { } pub fn first_icon(&self) -> Option { - self.icons.first().copied() + self.icons.first().map(OwnedIcon::as_raw) } pub fn update_tray(&self, main_hwnd: HWND, command: u32, icon: HICON, tip: &str) { @@ -163,17 +155,13 @@ impl TrayController { self.update_tray( main_hwnd, windows_sys::Win32::UI::Shell::NIM_MODIFY, - self.icons[icon_index], + self.icons[icon_index].as_raw(), &tooltip, ); } pub fn clear_icons(&mut self) { - for icon in self.icons.drain(..) { - if !icon.is_null() { - destroy_icon_handle(icon); - } - } + self.icons.clear(); } } diff --git a/src/app/mod.rs b/src/app/mod.rs index 8e48ce5..179e164 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -29,7 +29,8 @@ use windows::Win32::System::Com::{ use windows::Win32::UI::Shell::{IShellDispatch, Shell}; use windows_sys::Win32::Foundation::{ CloseHandle, ERROR_ALREADY_EXISTS, ERROR_GEN_FAILURE, ERROR_INVALID_PARAMETER, ERROR_TIMEOUT, - HANDLE, HINSTANCE, HWND, LPARAM, POINT, RECT, TRUE, WAIT_TIMEOUT, WPARAM, + GetLastError, HANDLE, HINSTANCE, HWND, LPARAM, POINT, RECT, SetLastError, TRUE, WAIT_TIMEOUT, + WPARAM, }; use windows_sys::Win32::Graphics::Gdi::{ COLOR_3DFACE, CreateRectRgn, DCX_CACHE, DCX_CLIPSIBLINGS, DCX_INTERSECTRGN, DeleteObject, @@ -55,14 +56,14 @@ use windows_sys::Win32::UI::Controls::{ use windows_sys::Win32::UI::Input::KeyboardAndMouse::{ GetAsyncKeyState, ReleaseCapture, SetCapture, VK_CONTROL, }; -use windows_sys::Win32::UI::Shell::{NIM_ADD, NIM_DELETE, ShellAboutW, ShellExecuteW, WinHelpW}; +use windows_sys::Win32::UI::Shell::{NIM_ADD, NIM_DELETE, ShellAboutW, ShellExecuteW}; use windows_sys::Win32::UI::WindowsAndMessaging::{ CheckMenuItem, CheckMenuRadioItem, CreateWindowExW, DefWindowProcW, DeleteMenu, DestroyAcceleratorTable, DestroyWindow, DispatchMessageW, DrawMenuBar, EnableMenuItem, GWL_STYLE, GetClassInfoW, GetClientRect, GetCursorPos, GetDlgItem, GetForegroundWindow, GetMenu, GetMenuItemInfoW, GetMessageW, GetShellWindow, GetWindowLongW, GetWindowPlacement, - GetWindowRect, HACCEL, HELP_FINDER, HICON, HMENU, HTCAPTION, HTCLIENT, HWND_NOTOPMOST, - HWND_TOP, HWND_TOPMOST, IDCANCEL, IsDialogMessageW, IsIconic, IsWindowVisible, IsZoomed, + GetWindowRect, HACCEL, HICON, HMENU, HTCAPTION, HTCLIENT, HWND_NOTOPMOST, HWND_TOP, HWND_TOPMOST, + IDCANCEL, IsDialogMessageW, IsIconic, IsWindowVisible, IsZoomed, KillTimer, LR_DEFAULTCOLOR, LR_DEFAULTSIZE, MB_ICONSTOP, MB_OK, MENUITEMINFOW, MF_BYCOMMAND, MF_CHECKED, MF_ENABLED, MF_GRAYED, MF_POPUP, MF_SEPARATOR, MF_SYSMENU, MF_UNCHECKED, MIIM_ID, MINMAXINFO, MSG, MessageBoxW, OpenIcon, PostMessageW, PostQuitMessage, RegisterClassW, @@ -87,10 +88,10 @@ use self::page_registry::{MinimumSizePolicy, PageId}; use crate::config::options::{Options, update_speed_timer_interval}; use crate::infrastructure::diagnostics::{self, Field, Level}; use crate::infrastructure::native::{ - call_window_proc, destroy_icon_handle, enable_debug_privilege, format_resource_string, height, - hiword, loword, process_is_elevated, record_hresult_error, record_startup_timing, - record_win32_error, sanitize_task_manager_menu, set_dialog_msg_result, set_style, - set_window_userdata_ptr, to_wide_null, width, window_userdata_non_null, + OwnedIcon, call_window_proc, enable_debug_privilege, format_resource_string, height, hiword, + loword, process_is_elevated, record_hresult_error, record_startup_timing, record_win32_error, + sanitize_task_manager_menu, set_dialog_msg_result, set_style, set_window_userdata_ptr, + to_wide_null, width, window_userdata_non_null, }; use crate::system::cpu_topology::CpuTopologyError; use crate::system::process_identity::ProcIdentity; @@ -104,6 +105,10 @@ use crate::ui::runtime_menu::PopupMenu; const FINDME_TIMEOUT: u32 = 10_000; const STARTUP_MUTEX_WAIT_TIMEOUT: u32 = 500; +const HELP_DOCUMENTATION_URL: &str = "https://github.com/JamesLinYJ/taskmgr-rs#readme"; +// CoInitializeEx reports this when the thread already belongs to a different apartment. +// The thread is still COM-initialized, but this call acquired no reference to release. +const RPC_E_CHANGED_MODE_HRESULT: i32 = 0x8001_0106_u32 as i32; static FRAME_BASE_WNDPROC: OnceLock< Option isize>, > = OnceLock::new(); @@ -121,7 +126,7 @@ struct ComApartment; impl ComApartment { fn initialize() -> Result { - // The main window thread owns this apartment for the duration of the system Run request. + // The returned guard owns exactly the successful CoInitializeEx call made here. let result = unsafe { CoInitializeEx(None, COINIT_APARTMENTTHREADED) }; if result.is_ok() { Ok(Self) @@ -129,6 +134,16 @@ impl ComApartment { Err(result.0) } } + + fn initialize_for_shell() -> Result, i32> { + match Self::initialize() { + Ok(apartment) => Ok(Some(apartment)), + // A host may have initialized the UI thread with another apartment model. In that + // case COM is already available and there is no successful call for us to balance. + Err(RPC_E_CHANGED_MODE_HRESULT) => Ok(None), + Err(error) => Err(error), + } + } } impl Drop for ComApartment { @@ -166,7 +181,7 @@ pub struct App { startup_mutex: HANDLE, startup_mutex_owned: bool, accelerator_table: HACCEL, - main_icon: HICON, + main_icon: Option, menu: MenuController, tray: TrayController, strings: GlobalStrings, @@ -375,7 +390,7 @@ impl App { startup_mutex: null_mut(), startup_mutex_owned: false, accelerator_table: null_mut(), - main_icon: null_mut(), + main_icon: None, menu: MenuController::default(), tray: TrayController::default(), strings: GlobalStrings::default(), @@ -644,9 +659,12 @@ impl App { page.destroy(); } self.tray.clear_icons(); - if !self.main_icon.is_null() { - destroy_icon_handle(self.main_icon); - self.main_icon = null_mut(); + if self.main_icon.is_some() { + if !self.main_hwnd.is_null() { + // 安全性: detach the borrowed icon from the live window before its owner drops. + unsafe { SendMessageW(self.main_hwnd, WM_SETICON, 1, 0) }; + } + self.main_icon = None; } if !self.accelerator_table.is_null() { unsafe { DestroyAcceleratorTable(self.accelerator_table) }; @@ -1854,9 +1872,61 @@ impl App { } fn show_help(&self, hwnd: HWND) { - let help_path = to_wide_null("taskmgr.hlp"); - // 安全性: `help_path` is a NUL-terminated UTF-16 buffer valid for the duration of call. - unsafe { WinHelpW(hwnd, help_path.as_ptr(), HELP_FINDER, 0) }; + let _apartment = match ComApartment::initialize_for_shell() { + Ok(apartment) => apartment, + Err(error) => { + record_hresult_error("initializing COM for online help", error); + self.show_hresult_failure(text(TextKey::HelpOpenFailed), error); + return; + } + }; + let verb = to_wide_null("open"); + let target = to_wide_null(help_documentation_url()); + // Safety: the verb and fixed HTTPS target are NUL-terminated and live through the + // synchronous call. Clear and capture last-error around ShellExecuteW so failures keep + // their documented extended diagnostic without consulting any file search path. + let (result, last_error) = unsafe { + SetLastError(0); + let result = ShellExecuteW( + hwnd, + verb.as_ptr(), + target.as_ptr(), + null(), + null(), + SW_SHOWNORMAL, + ) as isize; + let last_error = if result <= 32 { GetLastError() } else { 0 }; + (result, last_error) + }; + if let Some(failure) = shell_execute_failure(result, last_error) { + let title = to_wide_null(text(TextKey::HelpOpenFailed)); + let body = match failure { + ShellExecuteFailure::Win32(error) => { + record_win32_error("opening online help", error); + format!("{} 0x{error:08X}", text(TextKey::Win32ErrorPrefix)) + } + ShellExecuteFailure::Shell(code) => { + diagnostics::event( + Level::Error, + "help.open_failed", + "app", + "ShellExecuteW returned a documented failure code for online help", + &[Field::signed("shell_execute_code", code as i64)], + ); + format!("ShellExecuteW: {code}") + } + }; + let body = to_wide_null(&body); + // Safety: the owner HWND is live and both UTF-16 buffers remain valid for the call. + unsafe { + MessageBoxW( + hwnd, + body.as_ptr(), + title.as_ptr(), + MB_OK | MB_ICONSTOP, + ); + } + } } fn on_menu_select(&mut self, wparam: WPARAM, lparam: LPARAM) -> isize { @@ -2140,15 +2210,16 @@ impl App { } IDM_ABOUT => { let title = to_wide_null(&self.strings.app_title); - let icon = load_icon_resource( + match load_icon_resource( APPLICATION_ICON_RESOURCE, 0, 0, LR_DEFAULTCOLOR | LR_DEFAULTSIZE, - ); - if !icon.is_null() { - ShellAboutW(hwnd, title.as_ptr(), null(), icon); - destroy_icon_handle(icon); + ) { + Ok(icon) => { + ShellAboutW(hwnd, title.as_ptr(), null(), icon.as_raw()); + } + Err(error) => record_win32_error("about dialog icon loading", error), } } IDM_TASK_CASCADE @@ -2274,12 +2345,23 @@ impl App { } } - fn on_notify(&mut self, lparam: LPARAM) -> isize { - // 安全性: this function is a safe facade over Win32/FFI work; all callers run it on the owning UI thread and the existing body preserves its original handle/pointer invariants. + /// Handles a notification sent to the main dialog. + /// + /// # Safety + /// + /// `lparam` must point to the live `WM_NOTIFY` payload supplied by Win32 for the duration of + /// this synchronous dispatch. + unsafe fn on_notify(&mut self, lparam: LPARAM) -> isize { unsafe { - let header = &*(lparam as *const NMHDR); - if header.idFrom as i32 == IDC_TABS && header.code == TCN_SELCHANGE { - let tabs_hwnd = GetDlgItem(self.main_hwnd, IDC_TABS); + let Some(header) = (lparam as *const NMHDR).as_ref() else { + return 0; + }; + let tabs_hwnd = GetDlgItem(self.main_hwnd, IDC_TABS); + if !tabs_hwnd.is_null() + && header.idFrom == IDC_TABS as usize + && header.hwndFrom == tabs_hwnd + && header.code == TCN_SELCHANGE + { let selected = SendMessageW(tabs_hwnd, TCM_GETCURSEL, 0, 0); let Ok(selected) = usize::try_from(selected) else { return 0; @@ -2372,10 +2454,9 @@ impl App { } self.tray.clear_icons(); - if !self.main_icon.is_null() { + if self.main_icon.is_some() { SendMessageW(self.main_hwnd, WM_SETICON, 1, 0); - destroy_icon_handle(self.main_icon); - self.main_icon = null_mut(); + self.main_icon = None; } if !self.accelerator_table.is_null() { DestroyAcceleratorTable(self.accelerator_table); @@ -2396,6 +2477,16 @@ impl App { } } +impl Drop for App { + fn drop(&mut self) { + if self.main_icon.is_some() && !self.main_hwnd.is_null() { + // 安全性: App is confined to the window thread. Detaching the borrowed window icon + // synchronously ensures the following field drop cannot destroy an icon still in use. + unsafe { SendMessageW(self.main_hwnd, WM_SETICON, 1, 0) }; + } + } +} + fn active_page_id(current_page: i32) -> Option { PageId::from_persisted(current_page) } @@ -2451,6 +2542,27 @@ fn clamped_window_size( (width_px.max(min_width), height_px.max(min_height)) } +fn help_documentation_url() -> &'static str { + // The help target is application-owned build input, never a filename or environment lookup. + HELP_DOCUMENTATION_URL +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum ShellExecuteFailure { + Win32(u32), + Shell(isize), +} + +fn shell_execute_failure(result: isize, last_error: u32) -> Option { + if result > 32 { + return None; + } + if last_error != 0 { + return Some(ShellExecuteFailure::Win32(last_error)); + } + Some(ShellExecuteFailure::Shell(result)) +} + unsafe extern "system" fn main_window_proc( hwnd: HWND, msg: u32, @@ -2517,25 +2629,19 @@ unsafe extern "system" fn main_window_proc( record_win32_error("tray icon loading", error); } // 安全性: main HWND is live; icon and tray setup after deferred icon loading. - let icon = load_icon_resource( + match load_icon_resource( APPLICATION_ICON_RESOURCE, 0, 0, LR_DEFAULTCOLOR | LR_DEFAULTSIZE, - ); - if !icon.is_null() { - let old_owned_icon = application.main_icon; - SendMessageW(hwnd, WM_SETICON, 1, icon as LPARAM); - if !old_owned_icon.is_null() { - destroy_icon_handle(old_owned_icon); + ) { + Ok(icon) => { + SendMessageW(hwnd, WM_SETICON, 1, icon.as_raw() as LPARAM); + // WM_SETICON synchronously replaces the window's borrowed icon, so + // assigning the new owner may now safely drop the previous owner. + application.main_icon = Some(icon); } - application.main_icon = icon; - } else { - let error = windows_sys::Win32::Foundation::GetLastError(); - record_win32_error( - "main window icon loading", - if error == 0 { ERROR_GEN_FAILURE } else { error }, - ); + Err(error) => record_win32_error("main window icon loading", error), } if let Some(first_icon) = application.tray.first_icon() { application.update_tray(NIM_ADD, first_icon, ""); @@ -2617,9 +2723,47 @@ mod tests { use super::page_registry::PageId; use super::{ active_page_id, active_page_uses_normal_minimum, clamped_window_size, is_active_page, - page_uses_normal_minimum, + ShellExecuteFailure, help_documentation_url, page_uses_normal_minimum, + shell_execute_failure, }; + #[test] + fn help_uses_fixed_https_project_documentation() { + let target = help_documentation_url(); + assert_eq!(target, "https://github.com/JamesLinYJ/taskmgr-rs#readme"); + assert!(target.starts_with("https://")); + assert!(!target.ends_with(".hlp")); + } + + #[test] + fn shell_execute_help_result_ignores_stale_last_error_after_success() { + assert_eq!(shell_execute_failure(33, 5), None); + } + + #[test] + fn shell_execute_help_failure_preserves_extended_error() { + assert_eq!( + shell_execute_failure(31, 5), + Some(ShellExecuteFailure::Win32(5)) + ); + } + + #[test] + fn shell_execute_help_failure_preserves_shell_error_domain() { + assert_eq!( + shell_execute_failure(31, 0), + Some(ShellExecuteFailure::Shell(31)) + ); + assert_eq!( + shell_execute_failure(0, 0), + Some(ShellExecuteFailure::Shell(0)) + ); + assert_eq!( + shell_execute_failure(-1, 0), + Some(ShellExecuteFailure::Shell(-1)) + ); + } + #[test] fn active_page_id_rejects_invalid_values() { assert_eq!(active_page_id(-1), None); diff --git a/src/app/page_host.rs b/src/app/page_host.rs index 49efc14..30f1610 100644 --- a/src/app/page_host.rs +++ b/src/app/page_host.rs @@ -131,15 +131,15 @@ impl PageState { match self { Self::Task(state) => state.complete_initialize()?, - Self::Process(state) => unsafe { + Self::Process(state) => { state.initialize(hinstance, hwnd, main_hwnd)?; - }, + } Self::Performance(state) => state.complete_initialize(hwnd)?, Self::Cpu(state) => state.initialize(hwnd)?, Self::Gpu(state) => state.initialize(hwnd, main_hwnd, hwnd_tabs)?, - Self::Network(state) => unsafe { + Self::Network(state) => { state.initialize(hwnd, main_hwnd, hwnd_tabs)?; - }, + } Self::Users(state) => state.initialize(hwnd)?, } Ok(()) @@ -157,10 +157,10 @@ impl PageState { state.apply_options(options); false } - Self::Process(state) => unsafe { + Self::Process(state) => { state.apply_options(options, processor_count); false - }, + } Self::Performance(state) => { if state.apply_options(hwnd, options, processor_count) { state.size_page(hwnd, main_hwnd); @@ -183,10 +183,10 @@ impl PageState { false } } - Self::Network(state) => unsafe { + Self::Network(state) => { state.apply_options(options); false - }, + } Self::Users(state) => { state.apply_options(options); false @@ -201,20 +201,20 @@ impl PageState { let force = reason.forces_list_refresh(); match self { Self::Task(state) => state.timer_event(options, force), - Self::Process(state) => unsafe { + Self::Process(state) => { state.apply_options(options, processor_count); state.timer_event(options, force); - }, + } Self::Performance(_) => {} Self::Cpu(state) => state.timer_event(reason.cpu_detail_refresh()), Self::Gpu(state) => { let _ = state.apply_options(options); state.timer_event(); } - Self::Network(state) => unsafe { + Self::Network(state) => { state.apply_options(options); state.timer_event(); - }, + } Self::Users(state) => { state.apply_options(options); state.timer_event(); @@ -224,20 +224,18 @@ impl PageState { fn deactivate(&mut self, options: &mut Options) { if let Self::Process(state) = self { - unsafe { - state.deactivate(options); - } + state.deactivate(options); } } fn destroy(&mut self) { match self { Self::Task(state) => state.destroy(), - Self::Process(state) => unsafe { state.destroy() }, + Self::Process(state) => state.destroy(), Self::Performance(state) => state.destroy(), Self::Cpu(state) => state.destroy(), Self::Gpu(state) => state.destroy(), - Self::Network(state) => unsafe { state.destroy() }, + Self::Network(state) => state.destroy(), Self::Users(state) => state.destroy(), } } @@ -263,10 +261,10 @@ impl PageState { fn handle_process_command(&mut self, command_id: u16, options: Option<&mut Options>) -> bool { match self { - Self::Process(state) => unsafe { + Self::Process(state) => { state.handle_command(command_id, options); true - }, + } _ => false, } } @@ -287,7 +285,7 @@ impl PageState { fn find_process(&mut self, identity: ProcIdentity) -> bool { match self { - Self::Process(state) => unsafe { state.find_process(identity) }, + Self::Process(state) => state.find_process(identity), _ => false, } } @@ -295,11 +293,11 @@ impl PageState { fn no_title(&self) -> bool { match self { Self::Task(state) => state.no_title(), - Self::Process(state) => unsafe { state.no_title() }, + Self::Process(state) => state.no_title(), Self::Performance(state) => state.no_title(), Self::Cpu(state) => state.no_title(), Self::Gpu(state) => state.no_title(), - Self::Network(state) => unsafe { state.no_title() }, + Self::Network(state) => state.no_title(), Self::Users(state) => state.no_title(), } } @@ -328,21 +326,27 @@ impl PageState { state.handle_command(command_id); 1 } - Self::Process(state) => unsafe { + Self::Process(state) => { state.handle_command(command_id, None); 1 - }, + } Self::Users(state) => isize::from(state.handle_command(command_id)), _ => 0, } } - fn handle_notify(&mut self, lparam: LPARAM) -> isize { + /// Forwards a `WM_NOTIFY` payload to the page that owns the originating control. + /// + /// # Safety + /// + /// `lparam` must be the live payload supplied with the current synchronous `WM_NOTIFY` + /// dispatch and must satisfy the selected page handler's notification contract. + unsafe fn handle_notify(&mut self, lparam: LPARAM) -> isize { match self { - Self::Task(state) => state.handle_notify(lparam), + Self::Task(state) => unsafe { state.handle_notify(lparam) }, Self::Process(state) => unsafe { state.handle_notify(lparam) }, - Self::Cpu(state) => state.handle_notify(lparam), - Self::Users(state) => state.handle_notify(lparam), + Self::Cpu(state) => unsafe { state.handle_notify(lparam) }, + Self::Users(state) => unsafe { state.handle_notify(lparam) }, _ => 0, } } @@ -357,12 +361,10 @@ impl PageState { Some(1) } Self::Process(state) if wparam as HWND == unsafe { GetDlgItem(hwnd, IDC_PROCLIST) } => { - unsafe { - state.show_context_menu( - i32::from((lparam & 0xFFFF) as i16), - i32::from(((lparam >> 16) & 0xFFFF) as i16), - ); - } + state.show_context_menu( + i32::from((lparam & 0xFFFF) as i16), + i32::from(((lparam >> 16) & 0xFFFF) as i16), + ); Some(1) } Self::Users(state) if wparam as HWND == unsafe { GetDlgItem(hwnd, IDC_USERLIST) } => { @@ -382,20 +384,20 @@ impl PageState { state.size_page(); false } - Self::Process(state) => unsafe { + Self::Process(state) => { state.size_page(); false - }, + } Self::Performance(state) => { state.size_page(hwnd, main_hwnd); true } Self::Cpu(state) => state.size_page(), Self::Gpu(state) => state.size_page(), - Self::Network(state) => unsafe { + Self::Network(state) => { state.size_page(); false - }, + } Self::Users(state) => { state.size_page(); false @@ -426,7 +428,7 @@ impl PageState { !matches!(self, Self::Task(_)) } - unsafe fn initialize_dialog_host( + fn initialize_dialog_host( &mut self, hinstance: HINSTANCE, hwnd: HWND, @@ -443,6 +445,13 @@ impl PageState { self.handle_init_dialog(hinstance, hwnd, main_hwnd, hwnd_tabs) } + /// Dispatches one raw Win32 message to the active page state. + /// + /// # Safety + /// + /// `hwnd`, `wparam`, and `lparam` must be the live parameters supplied by Win32 for `msg`. + /// Message-specific pointer payloads must remain valid for the duration of this synchronous + /// call. unsafe fn handle_message( &mut self, hwnd: HWND, @@ -460,7 +469,7 @@ impl PageState { }, WM_NOTIFY => match self { Self::Task(_) | Self::Process(_) | Self::Cpu(_) | Self::Users(_) => { - Some(self.handle_notify(lparam)) + Some(unsafe { self.handle_notify(lparam) }) } _ => None, }, @@ -531,12 +540,12 @@ impl PageState { } WM_VSCROLL => match self { Self::Gpu(state) => Some(state.handle_vscroll(wparam)), - Self::Network(state) => Some(unsafe { state.handle_vscroll(wparam) }), + Self::Network(state) => Some(state.handle_vscroll(wparam)), _ => None, }, WM_MOUSEWHEEL => match self { Self::Gpu(state) => Some(state.handle_mouse_wheel(wparam)), - Self::Network(state) => Some(unsafe { state.handle_mouse_wheel(wparam) }), + Self::Network(state) => Some(state.handle_mouse_wheel(wparam)), _ => None, }, PWM_TASK_WORKER_COMPLETE => match self { @@ -548,9 +557,7 @@ impl PageState { }, PWM_PROC_WORKER_COMPLETE => match self { Self::Process(state) => { - unsafe { - state.handle_worker_completion(); - } + state.handle_worker_completion(); Some(1) } _ => None, @@ -585,9 +592,7 @@ impl PageState { }, PWM_NET_WORKER_COMPLETE => match self { Self::Network(state) => { - unsafe { - state.handle_worker_completion(); - } + state.handle_worker_completion(); Some(0) } _ => None, @@ -872,7 +877,14 @@ fn page_from_hwnd(hwnd: HWND, msg: u32, lparam: LPARAM) -> *mut DialogPage { page_pointer_for_message(msg, get_window_userdata(hwnd), lparam) } -fn bind_page(hwnd: HWND, page: *mut DialogPage) { +/// Binds the initialization pointer supplied to the dialog manager to its page HWND. +/// +/// # Safety +/// +/// A non-null `page` must be the live, uniquely accessible `DialogPage` pointer originally passed +/// to `CreateDialogParamW` for `hwnd`. The page allocation must outlive the HWND and remain on the +/// owning UI thread until `WM_NCDESTROY` clears the stored pointer. +unsafe fn bind_page(hwnd: HWND, page: *mut DialogPage) { if !page.is_null() { // 安全性: WM_INITDIALOG supplies the DialogPage pointer passed to CreateDialogParam. unsafe { diff --git a/src/app/single_instance.rs b/src/app/single_instance.rs index 2b1a0f0..ea426ce 100644 --- a/src/app/single_instance.rs +++ b/src/app/single_instance.rs @@ -44,7 +44,7 @@ use windows_sys::Win32::UI::WindowsAndMessaging::{ }; use crate::infrastructure::diagnostics::{self, Field, Level}; -use crate::infrastructure::native::to_wide_null; +use crate::infrastructure::native::{OwnedHandle, to_wide_null}; const MAX_IMAGE_PATH_UNITS: usize = 32_768; @@ -76,7 +76,7 @@ fn create_startup_mutex_with_suffix(suffix: &str) -> Result { GetCurrentProcessId() })?; let name = startup_mutex_name(&identity, suffix); - let sid = sid_to_string(identity.user_sid.as_ptr().cast_mut().cast())?; + let sid = identity.user_sid.to_string_sid()?; let mandatory_label = if identity.elevated { "S:(ML;;NW;;;HI)" } else { @@ -103,7 +103,7 @@ fn create_startup_mutex_with_suffix(suffix: &str) -> Result { fn startup_mutex_name(identity: &ProcessIdentity, suffix: &str) -> Vec { let mut hasher = Sha256::new(); - hasher.update(&identity.user_sid); + hasher.update(identity.user_sid.as_bytes()); let digest = hasher.finalize(); let user_hash = digest[..8] .iter() @@ -188,8 +188,10 @@ impl AuthenticatedWindow { process_id, ) }; - let process = OwnedHandle::new(process)?; - let peer = query_process_identity(process.raw(), process_id).ok()?; + // Safety: successful OpenProcess returns one owned process handle released by CloseHandle; + // no other owner is retained after this transfer. + let process = unsafe { OwnedHandle::from_raw(process) }?; + let peer = query_process_identity(process.as_raw(), process_id).ok()?; if !same_instance_identity(current, &peer) { diagnostics::event( Level::Trace, @@ -212,7 +214,8 @@ impl AuthenticatedWindow { // Safety: the PID output is writable and the retained process handle is live. let owner_exists = unsafe { GetWindowThreadProcessId(self.hwnd, &mut observed_process_id) } != 0; - let process_running = unsafe { WaitForSingleObject(self.process.raw(), 0) } == WAIT_TIMEOUT; + let process_running = + unsafe { WaitForSingleObject(self.process.as_raw(), 0) } == WAIT_TIMEOUT; window_binding_is_current( self.process_id, observed_process_id, @@ -226,7 +229,7 @@ struct ProcessIdentity { session_id: u32, elevated: bool, integrity_rid: u32, - user_sid: Vec, + user_sid: OwnedSid, image_path: String, } @@ -249,21 +252,23 @@ fn query_process_identity(process: HANDLE, process_id: u32) -> Result()) < size_of::() { return Err(ERROR_GEN_FAILURE); } // Safety: token_information returns a suitably aligned byte allocation and length was checked. let elevated = unsafe { (*(elevation.as_ptr().cast::())).TokenIsElevated != 0 }; - let user = token_information(token.raw(), TokenUser)?; + let user = token_information(token.as_raw(), TokenUser)?; if user.len().saturating_mul(size_of::()) < size_of::() { return Err(ERROR_GEN_FAILURE); } // Safety: the TOKEN_USER header and referenced SID stay inside `user` for its lifetime. let user_sid = unsafe { (*(user.as_ptr().cast::())).User.Sid }; - let user_sid = copy_sid(user_sid)?; - let integrity = token_information(token.raw(), TokenIntegrityLevel)?; + // Safety: TOKEN_USER came from the live, successfully populated token-information buffer and + // its SID remains readable while this function copies it. + let user_sid = unsafe { OwnedSid::copy_from_raw(user_sid) }?; + let integrity = token_information(token.as_raw(), TokenIntegrityLevel)?; if integrity.len().saturating_mul(size_of::()) < size_of::() { return Err(ERROR_GEN_FAILURE); } @@ -273,7 +278,10 @@ fn query_process_identity(process: HANDLE, process_id: u32) -> Result Result { if unsafe { OpenProcessToken(process, TOKEN_QUERY, &mut token) } == 0 { Err(last_error()) } else { - OwnedHandle::new(token).ok_or(ERROR_GEN_FAILURE) + // Safety: successful OpenProcessToken returns one owned token handle released by + // CloseHandle; ownership moves directly into the guard. + unsafe { OwnedHandle::from_raw(token) }.ok_or(ERROR_GEN_FAILURE) } } @@ -325,34 +335,106 @@ fn token_information(token: HANDLE, class: i32) -> Result, u32> { } } -fn copy_sid(sid: PSID) -> Result, u32> { - if sid.is_null() { - return Err(ERROR_GEN_FAILURE); +#[derive(Clone, Debug, PartialEq, Eq)] +struct OwnedSid { + // TOKEN_* buffers are u64-aligned. Retaining that alignment avoids rebuilding a PSID from a + // `Vec`, whose type-level alignment would not satisfy native SID field accesses. + storage: Vec, + byte_len: usize, +} + +impl OwnedSid { + /// Copies a native SID into self-contained, suitably aligned storage. + /// + /// # Safety + /// + /// `sid` must point to a live, structurally valid SID that remains readable for the complete + /// byte length reported by `GetLengthSid` during this call. + unsafe fn copy_from_raw(sid: PSID) -> Result { + if sid.is_null() { + return Err(ERROR_GEN_FAILURE); + } + // Safety: validity and readability are required by the function-level contract. + let byte_len = unsafe { GetLengthSid(sid) } as usize; + if byte_len == 0 { + return Err(last_error()); + } + let mut storage = vec![0u64; byte_len.div_ceil(size_of::())]; + // Safety: destination storage is aligned and at least `byte_len` bytes; the source range + // is readable and non-overlapping by the function-level contract. + unsafe { + std::ptr::copy_nonoverlapping( + sid.cast::(), + storage.as_mut_ptr().cast::(), + byte_len, + ); + } + Ok(Self { storage, byte_len }) } - // Safety: the SID came from a validated token-information buffer. - let length = unsafe { GetLengthSid(sid) }; - if length == 0 { - return Err(last_error()); + + fn as_bytes(&self) -> &[u8] { + // Safety: `storage` contains at least `byte_len` initialized bytes copied above. + unsafe { std::slice::from_raw_parts(self.storage.as_ptr().cast::(), self.byte_len) } } - // Safety: the SID reports its own byte length and remains live for this copy. - Ok(unsafe { std::slice::from_raw_parts(sid.cast::(), length as usize) }.to_vec()) -} -fn sid_last_subauthority(sid: PSID) -> Result { - if sid.is_null() { - return Err(ERROR_GEN_FAILURE); + fn as_psid(&self) -> PSID { + self.storage.as_ptr().cast_mut().cast() } - // Safety: the SID came from a validated token-information buffer. - let count = unsafe { GetSidSubAuthorityCount(sid) }; - if count.is_null() || unsafe { *count } == 0 { - return Err(ERROR_GEN_FAILURE); + + fn last_subauthority(&self) -> Result { + let sid = self.as_psid(); + // Safety: this owner contains a complete, aligned SID and keeps it live through both calls. + let count = unsafe { GetSidSubAuthorityCount(sid) }; + if count.is_null() || unsafe { *count } == 0 { + return Err(ERROR_GEN_FAILURE); + } + // Safety: the validated nonzero count indexes the final subauthority of the same SID. + let value = unsafe { GetSidSubAuthority(sid, u32::from(*count) - 1) }; + if value.is_null() { + Err(ERROR_GEN_FAILURE) + } else { + Ok(unsafe { *value }) + } } - // Safety: count is nonzero and indexes the final subauthority of the same SID. - let value = unsafe { GetSidSubAuthority(sid, u32::from(*count) - 1) }; - if value.is_null() { - Err(ERROR_GEN_FAILURE) - } else { - Ok(unsafe { *value }) + + fn to_string_sid(&self) -> Result { + let mut value = null_mut::(); + // Safety: this owner supplies a complete aligned SID, and `value` is a writable out slot. + if unsafe { ConvertSidToStringSidW(self.as_psid(), &mut value) } == 0 { + return Err(last_error()); + } + let mut length = 0usize; + // Safety: the conversion API returns a NUL-terminated LocalAlloc string. + unsafe { + while *value.add(length) != 0 { + length += 1; + } + } + // Safety: the discovered range precedes the terminating NUL. + let output = + String::from_utf16_lossy(unsafe { std::slice::from_raw_parts(value, length) }); + // Safety: ConvertSidToStringSidW transfers a LocalAlloc allocation to the caller. + unsafe { + LocalFree(value.cast()); + } + Ok(output) + } + + #[cfg(test)] + fn from_identity_bytes(bytes: &[u8]) -> Self { + let mut storage = vec![0u64; bytes.len().div_ceil(size_of::())]; + // Safety: destination has at least `bytes.len()` bytes and cannot overlap the input. + unsafe { + std::ptr::copy_nonoverlapping( + bytes.as_ptr(), + storage.as_mut_ptr().cast::(), + bytes.len(), + ); + } + Self { + storage, + byte_len: bytes.len(), + } } } @@ -367,27 +449,6 @@ fn query_image_path(process: HANDLE) -> Result { Ok(OsString::from_wide(&buffer).to_string_lossy().into_owned()) } -fn sid_to_string(sid: PSID) -> Result { - let mut value = null_mut::(); - // Safety: `sid` points at the copied, self-contained SID bytes owned by the caller. - if unsafe { ConvertSidToStringSidW(sid, &mut value) } == 0 { - return Err(last_error()); - } - let mut length = 0usize; - // Safety: the conversion API returns a NUL-terminated LocalAlloc string. - unsafe { - while *value.add(length) != 0 { - length += 1; - } - } - // Safety: the discovered range precedes the terminating NUL. - let output = String::from_utf16_lossy(unsafe { std::slice::from_raw_parts(value, length) }); - unsafe { - LocalFree(value.cast()); - } - Ok(output) -} - struct OwnedSecurityDescriptor(PSECURITY_DESCRIPTOR); impl OwnedSecurityDescriptor { @@ -425,28 +486,6 @@ impl Drop for OwnedSecurityDescriptor { } } -struct OwnedHandle(HANDLE); - -impl OwnedHandle { - fn new(handle: HANDLE) -> Option { - (!handle.is_null()).then_some(Self(handle)) - } - - const fn raw(&self) -> HANDLE { - self.0 - } -} - -impl Drop for OwnedHandle { - fn drop(&mut self) { - if !self.0.is_null() { - unsafe { - CloseHandle(self.0); - } - } - } -} - fn last_error() -> u32 { let error = unsafe { GetLastError() }; if error == 0 { ERROR_GEN_FAILURE } else { error } @@ -465,7 +504,7 @@ mod tests { session_id: 3, elevated: true, integrity_rid: 0x3000, - user_sid: vec![1, 2, 3], + user_sid: OwnedSid::from_identity_bytes(&[1, 2, 3]), image_path: r"C:\Program Files\taskmgr-rs\taskmgr.exe".to_string(), } } @@ -521,20 +560,28 @@ mod tests { "current session should be queryable" ); let token = open_process_token(process).expect("current token should open"); - token_information(token.raw(), TokenElevation).expect("elevation should be queryable"); - let user = token_information(token.raw(), TokenUser).expect("user should be queryable"); + token_information(token.as_raw(), TokenElevation).expect("elevation should be queryable"); + let user = token_information(token.as_raw(), TokenUser).expect("user should be queryable"); let user_sid = unsafe { (*(user.as_ptr().cast::())).User.Sid }; - let user_sid = copy_sid(user_sid).expect("user SID should copy"); - let integrity = token_information(token.raw(), TokenIntegrityLevel) + // Safety: the SID is backed by the live TOKEN_USER buffer for this copy. + let user_sid = unsafe { OwnedSid::copy_from_raw(user_sid) } + .expect("user SID should copy"); + let integrity = token_information(token.as_raw(), TokenIntegrityLevel) .expect("integrity should be queryable"); let integrity_sid = unsafe { (*(integrity.as_ptr().cast::())) .Label .Sid }; - sid_last_subauthority(integrity_sid).expect("integrity SID should be valid"); + // Safety: the SID is backed by the live TOKEN_MANDATORY_LABEL buffer for this copy. + let integrity_sid = unsafe { OwnedSid::copy_from_raw(integrity_sid) } + .expect("integrity SID should copy"); + integrity_sid + .last_subauthority() + .expect("integrity SID should be valid"); query_image_path(process).expect("current image path should be queryable"); - let sid = sid_to_string(user_sid.as_ptr().cast_mut().cast()) + let sid = user_sid + .to_string_sid() .expect("copied user SID should remain valid"); OwnedSecurityDescriptor::from_sddl(&format!("D:P(A;;GA;;;SY)(A;;GA;;;BA)(A;;GA;;;{sid})")) .expect("mutex security descriptor should parse"); diff --git a/src/capabilities.rs b/src/capabilities.rs index 98bf0ca..4e2054a 100644 --- a/src/capabilities.rs +++ b/src/capabilities.rs @@ -179,7 +179,7 @@ fn cpu_capability_report() -> Value { let process = unsafe { GetCurrentProcess() }; let cpu_sets = match CpuSetTopology::query(process) { Ok(topology) => { - let default_sets = unsafe { query_process_default_cpu_sets(process) }; + let default_sets = query_process_default_cpu_sets(process); json!({ "status": "supported", "groups": topology.groups().iter().map(|group| json!({ diff --git a/src/infrastructure/diagnostics/crash.rs b/src/infrastructure/diagnostics/crash.rs index aeb5ef5..a343581 100644 --- a/src/infrastructure/diagnostics/crash.rs +++ b/src/infrastructure/diagnostics/crash.rs @@ -17,6 +17,7 @@ use std::env; use std::ffi::OsStr; +use std::marker::PhantomData; use std::os::windows::ffi::OsStrExt; use std::path::Path; use std::ptr::{null, null_mut}; @@ -49,6 +50,54 @@ const TEST_CRASH_ENVIRONMENT: &str = "TASKMGR_RS_DIAGNOSTIC_TEST_CRASH"; static CRASH_DIRECTORY: OnceLock> = OnceLock::new(); static MINIDUMP_ENABLED: AtomicBool = AtomicBool::new(false); +#[derive(Clone, Copy)] +struct NulTerminatedWide<'a> { + units: &'a [u16], +} + +impl<'a> NulTerminatedWide<'a> { + fn new(units: &'a [u16]) -> Option { + let (&terminator, body) = units.split_last()?; + (terminator == 0 && !body.contains(&0)).then_some(Self { units }) + } + + fn as_ptr(self) -> *const u16 { + self.units.as_ptr() + } + + #[cfg(test)] + fn without_terminator(self) -> &'a [u16] { + &self.units[..self.units.len() - 1] + } +} + +/// Borrowed exception state supplied for one invocation of the top-level Windows filter. +struct ExceptionContext<'a> { + raw: *const EXCEPTION_POINTERS, + _callback_lifetime: PhantomData<&'a EXCEPTION_POINTERS>, +} + +impl<'a> ExceptionContext<'a> { + /// Establishes the raw exception-pointer contract at the callback boundary. + /// + /// # Safety + /// + /// `raw` must be null or the live `EXCEPTION_POINTERS` value supplied by Windows to the + /// current unhandled-exception callback. Its nested exception/context records must remain + /// valid for all reads performed by this module and by `MiniDumpWriteDump`, and the returned + /// value must not outlive that callback. + unsafe fn from_callback(raw: *const EXCEPTION_POINTERS) -> Self { + Self { + raw, + _callback_lifetime: PhantomData, + } + } + + fn as_raw(&self) -> *const EXCEPTION_POINTERS { + self.raw + } +} + pub(super) fn install(directory: &Path, minidump_enabled: bool) -> Result<(), String> { let mut wide = OsStr::new(directory.as_os_str()) .encode_wide() @@ -114,16 +163,19 @@ pub(super) fn trigger_test_access_violation() -> ! { unsafe extern "system" fn unhandled_exception_filter( exception_info: *const EXCEPTION_POINTERS, ) -> i32 { + // Safety: Windows owns the raw exception graph for the complete synchronous filter callback. + // This is the sole conversion from that raw graph into the module's typed borrowed boundary. + let exception = unsafe { ExceptionContext::from_callback(exception_info) }; let pid = unsafe { GetCurrentProcessId() }; let tid = unsafe { GetCurrentThreadId() }; - let (exception_code, exception_address) = exception_details(exception_info); + let (exception_code, exception_address) = exception_details(&exception); let module_base = module_base_for_address(exception_address); let module_offset = exception_address.saturating_sub(module_base); let mut path = [0u16; MAX_CRASH_PATH_UNITS]; - if build_crash_path(&mut path, pid, tid, ".crash.json") { + if let Some(path) = build_crash_path(&mut path, pid, tid, ".crash.json") { write_crash_record( - path.as_ptr(), + path, pid, tid, exception_code, @@ -133,19 +185,22 @@ unsafe extern "system" fn unhandled_exception_filter( ); } - if MINIDUMP_ENABLED.load(Ordering::Acquire) && build_crash_path(&mut path, pid, tid, ".dmp") { - write_minidump(path.as_ptr(), pid, tid, exception_info); + if MINIDUMP_ENABLED.load(Ordering::Acquire) + && let Some(path) = build_crash_path(&mut path, pid, tid, ".dmp") + { + write_minidump(path, pid, tid, &exception); } EXCEPTION_CONTINUE_SEARCH } -fn exception_details(exception_info: *const EXCEPTION_POINTERS) -> (u32, usize) { - if exception_info.is_null() { +fn exception_details(exception: &ExceptionContext<'_>) -> (u32, usize) { + if exception.as_raw().is_null() { return (0, 0); } - // Safety: Windows supplies EXCEPTION_POINTERS and its record for the duration of the filter. - let record = unsafe { (*exception_info).ExceptionRecord }; + // Safety: `ExceptionContext` establishes that the outer record and its nested pointers remain + // live for this callback. + let record = unsafe { (*exception.as_raw()).ExceptionRecord }; if record.is_null() { return (0, 0); } @@ -174,36 +229,39 @@ fn module_base_for_address(address: usize) -> usize { if found == 0 { 0 } else { module as usize } } -fn build_crash_path( - output: &mut [u16; MAX_CRASH_PATH_UNITS], +fn build_crash_path<'a>( + output: &'a mut [u16; MAX_CRASH_PATH_UNITS], pid: u32, tid: u32, extension: &str, -) -> bool { +) -> Option> { let Some(directory) = CRASH_DIRECTORY.get() else { - return false; + return None; }; let mut offset = 0usize; if !push_wide(output, &mut offset, directory) { - return false; + return None; } for byte in b"crash-" { if !push_wide_unit(output, &mut offset, u16::from(*byte)) { - return false; + return None; } } if !push_decimal(output, &mut offset, pid) || !push_wide_unit(output, &mut offset, b'-' as u16) || !push_decimal(output, &mut offset, tid) { - return false; + return None; } for byte in extension.as_bytes() { if !push_wide_unit(output, &mut offset, u16::from(*byte)) { - return false; + return None; } } - push_wide_unit(output, &mut offset, 0) + if !push_wide_unit(output, &mut offset, 0) { + return None; + } + NulTerminatedWide::new(&output[..offset]) } fn push_wide(output: &mut [u16], offset: &mut usize, value: &[u16]) -> bool { @@ -245,7 +303,7 @@ fn push_decimal(output: &mut [u16], offset: &mut usize, value: u32) -> bool { } fn write_crash_record( - path: *const u16, + path: NulTerminatedWide<'_>, pid: u32, tid: u32, exception_code: u32, @@ -253,10 +311,10 @@ fn write_crash_record( module_base: usize, module_offset: usize, ) { - // Safety: `path` points at the NUL-terminated stack buffer built above. + // Safety: `path` is a typed NUL-terminated borrow that remains live through CreateFileW. let file = unsafe { CreateFileW( - path, + path.as_ptr(), GENERIC_WRITE, FILE_SHARE_READ, null(), @@ -300,11 +358,16 @@ fn write_crash_record( } } -fn write_minidump(path: *const u16, pid: u32, tid: u32, exception_info: *const EXCEPTION_POINTERS) { - // Safety: `path` points at the NUL-terminated stack buffer built above. +fn write_minidump( + path: NulTerminatedWide<'_>, + pid: u32, + tid: u32, + exception: &ExceptionContext<'_>, +) { + // Safety: `path` is a typed NUL-terminated borrow that remains live through CreateFileW. let file = unsafe { CreateFileW( - path, + path.as_ptr(), GENERIC_WRITE, FILE_SHARE_READ, null(), @@ -319,7 +382,7 @@ fn write_minidump(path: *const u16, pid: u32, tid: u32, exception_info: *const E let info = MINIDUMP_EXCEPTION_INFORMATION { ThreadId: tid, - ExceptionPointers: exception_info.cast_mut(), + ExceptionPointers: exception.as_raw().cast_mut(), ClientPointers: 0, }; let dump_type = MiniDumpNormal | MiniDumpWithThreadInfo | MiniDumpWithUnloadedModules; @@ -422,14 +485,28 @@ mod tests { fn crash_file_name_is_process_and_thread_specific() { let _ = CRASH_DIRECTORY.set(vec![b'C' as u16, b':' as u16, b'\\' as u16].into()); let mut path = [0u16; MAX_CRASH_PATH_UNITS]; - assert!(build_crash_path(&mut path, 12, 34, ".dmp")); - let end = path.iter().position(|unit| *unit == 0).unwrap(); + let path = build_crash_path(&mut path, 12, 34, ".dmp") + .expect("the configured crash directory should fit in the path buffer"); assert_eq!( - String::from_utf16_lossy(&path[..end]), + String::from_utf16_lossy(path.without_terminator()), "C:\\crash-12-34.dmp" ); } + #[test] + fn nul_terminated_wide_rejects_missing_or_interior_terminators() { + assert!(NulTerminatedWide::new(&[b'C' as u16, 0]).is_some()); + assert!(NulTerminatedWide::new(&[b'C' as u16]).is_none()); + assert!(NulTerminatedWide::new(&[b'C' as u16, 0, b'x' as u16, 0]).is_none()); + } + + #[test] + fn null_exception_context_has_no_details() { + // Safety: null is explicitly accepted by the callback-boundary contract. + let exception = unsafe { ExceptionContext::from_callback(null()) }; + assert_eq!(exception_details(&exception), (0, 0)); + } + #[test] fn controlled_crash_requires_the_exact_opt_in_value() { assert!(test_crash_value_requested(Some(OsStr::new( diff --git a/src/infrastructure/diagnostics/secure_fs.rs b/src/infrastructure/diagnostics/secure_fs.rs index 0696333..610c8cf 100644 --- a/src/infrastructure/diagnostics/secure_fs.rs +++ b/src/infrastructure/diagnostics/secure_fs.rs @@ -22,7 +22,7 @@ use std::mem::{offset_of, size_of}; use std::os::windows::ffi::{OsStrExt, OsStringExt}; use std::os::windows::io::{AsRawHandle, FromRawHandle}; use std::path::{Component, Path, PathBuf, Prefix}; -use std::ptr::{copy_nonoverlapping, null, null_mut, read_unaligned}; +use std::ptr::{copy_nonoverlapping, null, null_mut}; use windows_sys::Wdk::Foundation::OBJECT_ATTRIBUTES; use windows_sys::Wdk::Storage::FileSystem::{ @@ -174,7 +174,7 @@ impl SecureDirectory { disposition, FILE_ATTRIBUTE_DIRECTORY, FILE_DIRECTORY_FILE | FILE_OPEN_REPARSE_POINT | FILE_SYNCHRONOUS_IO_NONALERT, - security.as_ref().map(SecurityDescriptor::as_ptr), + security.as_ref(), )?; validate_handle(&file, ExpectedKind::Directory, false)?; Ok(Self { @@ -239,7 +239,7 @@ impl SecureDirectory { FILE_CREATE, FILE_ATTRIBUTE_NORMAL, FILE_NON_DIRECTORY_FILE | FILE_OPEN_REPARSE_POINT | FILE_SYNCHRONOUS_IO_NONALERT, - Some(security.as_ptr()), + Some(&security), )?; validate_handle(&file, ExpectedKind::RegularFile, true)?; Ok(file) @@ -302,11 +302,12 @@ impl SecureDirectory { if status < 0 && status != STATUS_BUFFER_OVERFLOW { return Err(ntstatus_error(status)); } - let returned = status_block.Information.min(DIRECTORY_QUERY_BUFFER_BYTES); + let returned = status_block.Information; if returned == 0 { break; } - parse_directory_entries(storage.as_ptr().cast::(), returned, &mut entries)?; + let bytes = directory_query_bytes(&storage, returned)?; + parse_directory_entries(bytes, &mut entries)?; restart_scan = false; } Ok(entries) @@ -443,7 +444,7 @@ fn open_relative( disposition: u32, file_attributes: u32, options: u32, - security_descriptor: Option<*const SECURITY_DESCRIPTOR>, + security_descriptor: Option<&SecurityDescriptor>, ) -> io::Result { let mut name = name.encode_wide().collect::>(); let byte_length = name @@ -461,7 +462,7 @@ fn open_relative( RootDirectory: parent, ObjectName: &raw const unicode, Attributes: OBJ_CASE_INSENSITIVE, - SecurityDescriptor: security_descriptor.unwrap_or(null()), + SecurityDescriptor: security_descriptor.map_or(null(), SecurityDescriptor::as_ptr), SecurityQualityOfService: null(), }; let mut status_block = IO_STATUS_BLOCK::default(); @@ -551,63 +552,125 @@ fn validate_handle(file: &File, expected: ExpectedKind, reject_hard_links: bool) Ok(()) } +fn directory_query_bytes(storage: &[u64], initialized_len: usize) -> io::Result<&[u8]> { + let capacity = storage + .len() + .checked_mul(size_of::()) + .ok_or_else(|| invalid_directory_data("directory query buffer size overflow"))?; + if initialized_len > capacity { + return Err(invalid_directory_data( + "directory enumeration returned more bytes than the query buffer", + )); + } + + // Safety: `storage` is a live, initialized allocation and `initialized_len` was bounded by + // its byte capacity above. The returned slice cannot outlive the borrowed storage. + Ok(unsafe { std::slice::from_raw_parts(storage.as_ptr().cast::(), initialized_len) }) +} + fn parse_directory_entries( - buffer: *const u8, - length: usize, + buffer: &[u8], output: &mut Vec, ) -> io::Result<()> { let fixed = offset_of!(FILE_DIRECTORY_INFORMATION, FileName); let mut offset = 0usize; loop { - if length.saturating_sub(offset) < fixed { - return Err(io::Error::new( - io::ErrorKind::InvalidData, + let remaining = buffer + .get(offset..) + .ok_or_else(|| invalid_directory_data("directory record offset is out of bounds"))?; + if remaining.len() < fixed { + return Err(invalid_directory_data( "directory enumeration returned a truncated record", )); } - // Safety: bounds above cover the fixed header; unaligned reads avoid layout assumptions. - let information = - unsafe { read_unaligned(buffer.add(offset).cast::()) }; - let name_bytes = information.FileNameLength as usize; + + let next = read_u32_field( + remaining, + offset_of!(FILE_DIRECTORY_INFORMATION, NextEntryOffset), + ) + .ok_or_else(|| invalid_directory_data("directory record is missing its next offset"))? + as usize; + let record_len = if next == 0 { + remaining.len() + } else { + if next < fixed || next > remaining.len() { + return Err(invalid_directory_data( + "directory enumeration returned an invalid next-entry offset", + )); + } + next + }; + + let name_bytes = read_u32_field( + remaining, + offset_of!(FILE_DIRECTORY_INFORMATION, FileNameLength), + ) + .ok_or_else(|| invalid_directory_data("directory record is missing its name length"))? + as usize; if !name_bytes.is_multiple_of(size_of::()) || fixed .checked_add(name_bytes) - .is_none_or(|record| record > length.saturating_sub(offset)) + .is_none_or(|name_end| name_end > record_len) { - return Err(io::Error::new( - io::ErrorKind::InvalidData, + return Err(invalid_directory_data( "directory enumeration returned an invalid file name", )); } - let units = name_bytes / size_of::(); - // Safety: the validated record contains `units` UTF-16 code units after the fixed header. - let name = unsafe { - let pointer = buffer.add(offset + fixed).cast::(); - OsString::from_wide(std::slice::from_raw_parts(pointer, units)) - }; + + let name_end = fixed + name_bytes; + let name_units = remaining[fixed..name_end] + .chunks_exact(size_of::()) + .map(|bytes| u16::from_ne_bytes([bytes[0], bytes[1]])) + .collect::>(); + let name = OsString::from_wide(&name_units); if name != OsStr::new(".") && name != OsStr::new("..") { validate_component(&name)?; output.push(SecureDirectoryEntry { name, - attributes: information.FileAttributes, - last_write_time: information.LastWriteTime, + attributes: read_u32_field( + remaining, + offset_of!(FILE_DIRECTORY_INFORMATION, FileAttributes), + ) + .ok_or_else(|| { + invalid_directory_data("directory record is missing its attributes") + })?, + last_write_time: read_i64_field( + remaining, + offset_of!(FILE_DIRECTORY_INFORMATION, LastWriteTime), + ) + .ok_or_else(|| { + invalid_directory_data("directory record is missing its write time") + })?, }); } - if information.NextEntryOffset == 0 { + if next == 0 { break; } - let next = information.NextEntryOffset as usize; - if next < fixed || next > length.saturating_sub(offset) { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - "directory enumeration returned an invalid next-entry offset", - )); - } offset += next; } Ok(()) } +fn read_u32_field(buffer: &[u8], offset: usize) -> Option { + let bytes: [u8; size_of::()] = buffer + .get(offset..offset.checked_add(size_of::())?)? + .try_into() + .ok()?; + Some(u32::from_ne_bytes(bytes)) +} + +fn read_i64_field(buffer: &[u8], offset: usize) -> Option { + let bytes: [u8; size_of::()] = buffer + .get(offset..offset.checked_add(size_of::())?)? + .try_into() + .ok()?; + Some(i64::from_ne_bytes(bytes)) +} + +fn invalid_directory_data(message: &'static str) -> io::Error { + io::Error::new(io::ErrorKind::InvalidData, message) +} + fn split_absolute_path(path: &Path) -> io::Result<(PathBuf, Vec)> { let mut components = path.components(); let prefix = match components.next() { @@ -790,7 +853,9 @@ fn current_user_sid_string() -> io::Result { if unsafe { OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &mut token) } == 0 { return Err(io::Error::last_os_error()); } - let token = OwnedHandle::new(token) + // Safety: successful OpenProcessToken transferred one owned token handle whose documented + // release function is CloseHandle; no other owner is retained. + let token = unsafe { OwnedHandle::from_raw(token) } .ok_or_else(|| io::Error::other("OpenProcessToken returned an invalid handle"))?; let mut required = 0u32; @@ -847,6 +912,100 @@ fn current_user_sid_string() -> io::Result { mod tests { use super::*; + fn write_u32_field(buffer: &mut [u8], offset: usize, value: u32) { + buffer[offset..offset + size_of::()].copy_from_slice(&value.to_ne_bytes()); + } + + fn write_i64_field(buffer: &mut [u8], offset: usize, value: i64) { + buffer[offset..offset + size_of::()].copy_from_slice(&value.to_ne_bytes()); + } + + fn directory_record(name: &str, attributes: u32, last_write_time: i64) -> Vec { + let fixed = offset_of!(FILE_DIRECTORY_INFORMATION, FileName); + let name = name.encode_utf16().collect::>(); + let mut record = vec![0u8; fixed + name.len() * size_of::()]; + write_u32_field( + &mut record, + offset_of!(FILE_DIRECTORY_INFORMATION, FileNameLength), + (name.len() * size_of::()) as u32, + ); + write_u32_field( + &mut record, + offset_of!(FILE_DIRECTORY_INFORMATION, FileAttributes), + attributes, + ); + write_i64_field( + &mut record, + offset_of!(FILE_DIRECTORY_INFORMATION, LastWriteTime), + last_write_time, + ); + for (index, unit) in name.into_iter().enumerate() { + let start = fixed + index * size_of::(); + record[start..start + size_of::()].copy_from_slice(&unit.to_ne_bytes()); + } + record + } + + #[test] + fn directory_query_length_is_bounded_by_typed_storage() { + let storage = [0u64; 2]; + assert_eq!( + directory_query_bytes(&storage, 7) + .expect("in-range byte count should be accepted") + .len(), + 7 + ); + assert_eq!( + directory_query_bytes(&storage, 17) + .expect_err("out-of-range byte count must be rejected") + .kind(), + io::ErrorKind::InvalidData + ); + } + + #[test] + fn directory_parser_reads_a_typed_record_slice() { + let record = directory_record("task.log", FILE_ATTRIBUTE_NORMAL, 42); + let mut entries = Vec::new(); + parse_directory_entries(&record, &mut entries).expect("record should parse"); + assert_eq!(entries.len(), 1); + assert_eq!(entries[0].name, OsStr::new("task.log")); + assert_eq!(entries[0].attributes, FILE_ATTRIBUTE_NORMAL); + assert_eq!(entries[0].last_write_time, 42); + } + + #[test] + fn directory_parser_rejects_a_name_crossing_the_record_boundary() { + let mut record = directory_record("task.log", FILE_ATTRIBUTE_NORMAL, 42); + write_u32_field( + &mut record, + offset_of!(FILE_DIRECTORY_INFORMATION, NextEntryOffset), + offset_of!(FILE_DIRECTORY_INFORMATION, FileName) as u32, + ); + let mut entries = Vec::new(); + assert_eq!( + parse_directory_entries(&record, &mut entries) + .expect_err("name must stay within its own record") + .kind(), + io::ErrorKind::InvalidData + ); + assert!(entries.is_empty()); + } + + #[test] + fn directory_parser_rejects_a_truncated_header() { + let fixed = offset_of!(FILE_DIRECTORY_INFORMATION, FileName); + let truncated = vec![0u8; fixed - 1]; + let mut entries = Vec::new(); + assert_eq!( + parse_directory_entries(&truncated, &mut entries) + .expect_err("truncated header must be rejected") + .kind(), + io::ErrorKind::InvalidData + ); + assert!(entries.is_empty()); + } + #[test] fn security_descriptors_name_the_token_user_instead_of_owner_rights() { let user_sid = diff --git a/src/infrastructure/native/errors.rs b/src/infrastructure/native/errors.rs index 46cb012..f5bd539 100644 --- a/src/infrastructure/native/errors.rs +++ b/src/infrastructure/native/errors.rs @@ -12,6 +12,7 @@ use std::ptr::null; +use windows_sys::Win32::Foundation::HMODULE; use windows_sys::Win32::System::Diagnostics::Debug::{ FORMAT_MESSAGE_FROM_HMODULE, FORMAT_MESSAGE_FROM_SYSTEM, FORMAT_MESSAGE_IGNORE_INSERTS, FormatMessageW, @@ -165,7 +166,7 @@ fn error_identity(domain: ErrorDomain, raw: u32) -> ErrorIdentity { } fn format_system_message(code: u32) -> Option { - format_message(code, null(), FORMAT_MESSAGE_FROM_SYSTEM) + format_message(code, None) } fn format_module_message(module_name: &str, code: u32) -> Option { @@ -178,16 +179,20 @@ fn format_module_message(module_name: &str, code: u32) -> Option { if module.is_null() { return None; } - format_message( - code, - module.cast_const(), - FORMAT_MESSAGE_FROM_HMODULE | FORMAT_MESSAGE_FROM_SYSTEM, - ) + format_message(code, Some(module)) } -fn format_message(code: u32, source: *const core::ffi::c_void, flags: u32) -> Option { +fn format_message(code: u32, module: Option) -> Option { + let (source, flags) = match module { + Some(module) => ( + module.cast_const(), + FORMAT_MESSAGE_FROM_HMODULE | FORMAT_MESSAGE_FROM_SYSTEM, + ), + None => (null(), FORMAT_MESSAGE_FROM_SYSTEM), + }; let mut buffer = [0u16; 1024]; - // Safety: `buffer` is writable for the supplied length and no insert arguments are requested. + // Safety: a module source comes only from successful GetModuleHandleW and remains loaded for + // this synchronous query; the buffer is writable and no insert arguments are requested. let count = unsafe { FormatMessageW( flags | FORMAT_MESSAGE_IGNORE_INSERTS, diff --git a/src/infrastructure/native/handles.rs b/src/infrastructure/native/handles.rs index 8475a26..1ca69f8 100644 --- a/src/infrastructure/native/handles.rs +++ b/src/infrastructure/native/handles.rs @@ -8,29 +8,51 @@ // 作者: OpenAI Codex // -------------------------------------------------------------------------- -//! Provides unique owners for Win32 and WTS allocations plus explicit owned-icon destruction. +//! Provides unique owners for Win32 handles, WTS allocations, and non-shared icons. +//! +//! Raw ownership can only enter these types through `unsafe` constructors. A non-null raw value +//! does not prove allocator provenance, unique ownership, or compatibility with the destructor. + +use std::cell::Cell; +use std::marker::PhantomData; +use std::num::NonZeroIsize; use windows_sys::Win32::Foundation::{CloseHandle, HANDLE, INVALID_HANDLE_VALUE}; use windows_sys::Win32::System::RemoteDesktop::WTSFreeMemory; use windows_sys::Win32::UI::WindowsAndMessaging::{DestroyIcon, HICON}; +#[must_use = "dropping the owner closes the Win32 handle"] pub struct OwnedHandle { handle: HANDLE, } +#[must_use = "dropping the owner frees the WTS allocation"] pub struct OwnedWtsMemory { ptr: *mut T, } -pub fn destroy_icon_handle(icon: HICON) { - if !icon.is_null() { - // 安全性: callers pass an icon handle they own and want to release. - unsafe { DestroyIcon(icon) }; - } +/// Unique owner of a non-shared icon that must be released with `DestroyIcon`. +/// +/// Win32 USER handles are not tied to the thread that created them, so the owner may move between +/// threads. `Cell` deliberately keeps shared references from being `Sync`: callers only borrow the +/// raw `HICON` for the duration of a synchronous Win32 call. +#[must_use = "dropping the owner destroys the icon"] +pub struct OwnedIcon { + icon: NonZeroIsize, + _not_sync: PhantomData>, } impl OwnedWtsMemory { - pub fn new(ptr: *mut T) -> Option { + /// Takes ownership of a WTS allocation. + /// + /// Null is accepted and returns `None`. + /// + /// # Safety + /// + /// A non-null `ptr` must identify a live allocation returned by a WTS API whose documented + /// release function is `WTSFreeMemory`. The caller must transfer unique ownership and must not + /// use or free the allocation after this call succeeds. + pub unsafe fn from_raw(ptr: *mut T) -> Option { (!ptr.is_null()).then_some(Self { ptr }) } @@ -49,7 +71,16 @@ impl Drop for OwnedWtsMemory { } impl OwnedHandle { - pub fn new(handle: HANDLE) -> Option { + /// Takes ownership of a Win32 kernel handle. + /// + /// Null and `INVALID_HANDLE_VALUE` are accepted and return `None`. + /// + /// # Safety + /// + /// Any other `handle` must be a live, uniquely owned handle whose documented release function + /// is `CloseHandle`. The caller must not use or close it after this call succeeds. Pseudo + /// handles and handles released by a different API must not be passed. + pub unsafe fn from_raw(handle: HANDLE) -> Option { (!handle.is_null() && handle != INVALID_HANDLE_VALUE).then_some(Self { handle }) } @@ -58,6 +89,49 @@ impl OwnedHandle { } } +impl OwnedIcon { + /// Takes ownership of a non-shared icon. + /// + /// Null is accepted and returns `None`. + /// + /// # Safety + /// + /// A non-null `icon` must be a live icon that the caller uniquely owns and is required to + /// release with `DestroyIcon` (for example, a successful `CopyIcon` result or `LoadImageW` + /// result loaded without `LR_SHARED`). Shared, class-owned, or window-owned icons must not be + /// passed. The caller must not use or destroy the icon after this call succeeds. + pub unsafe fn from_raw(icon: HICON) -> Option { + NonZeroIsize::new(icon as isize).map(|icon| Self { + icon, + _not_sync: PhantomData, + }) + } + + /// Borrows the icon handle for a synchronous Win32 call. + pub fn as_raw(&self) -> HICON { + self.icon.get() as HICON + } +} + +impl Drop for OwnedIcon { + fn drop(&mut self) { + // 安全性: construction requires unique ownership of a non-shared icon compatible with + // `DestroyIcon`; the non-zero handle is released exactly once here. + unsafe { DestroyIcon(self.as_raw()) }; + } +} + +#[cfg(test)] +mod tests { + use super::OwnedIcon; + + #[test] + fn owned_icon_can_transfer_between_threads() { + fn assert_send() {} + assert_send::(); + } +} + impl Drop for OwnedHandle { fn drop(&mut self) { if !self.handle.is_null() && self.handle != INVALID_HANDLE_VALUE { diff --git a/src/infrastructure/native/mod.rs b/src/infrastructure/native/mod.rs index 46fba07..8fc4ecf 100644 --- a/src/infrastructure/native/mod.rs +++ b/src/infrastructure/native/mod.rs @@ -23,11 +23,12 @@ pub(crate) use errors::{ record_hresult_error_with_fields, record_ntstatus_error_with_fields, record_pdh_error_with_fields, record_win32_error_with_fields, }; -pub use handles::{OwnedHandle, OwnedWtsMemory, destroy_icon_handle}; +pub use handles::{OwnedHandle, OwnedIcon, OwnedWtsMemory}; pub use safety::{enable_debug_privilege, process_is_elevated, process_needs_32_bit_suffix_handle}; pub use ui::{ - append_32_bit_suffix, call_window_proc, copy_text_to_callback_buffer, finish_list_view_update, - format_resource_string, get_window_userdata, height, hiword, loword, + append_32_bit_suffix, call_window_proc, copy_text_to_callback_buffer, + copy_text_to_utf16_buffer, finish_list_view_update, format_resource_string, + get_window_userdata, height, hiword, loword, pause_redraw_for_visible_windows, redraw_window_tree, resume_redraw_for_windows, sanitize_task_manager_menu, set_dialog_msg_result, set_style, set_window_userdata, set_window_userdata_ptr, subclass_list_view, to_wide_null, widestr_ptr_to_string, width, diff --git a/src/infrastructure/native/safety.rs b/src/infrastructure/native/safety.rs index e26dad4..561145a 100644 --- a/src/infrastructure/native/safety.rs +++ b/src/infrastructure/native/safety.rs @@ -45,7 +45,9 @@ pub fn enable_debug_privilege() -> Result<(), u32> { { return Err(GetLastError()); } - let Some(token) = OwnedHandle::new(raw_token) else { + // 安全性: successful OpenProcessToken returns one owned token handle released by + // CloseHandle; ownership moves directly into this guard. + let Some(token) = OwnedHandle::from_raw(raw_token) else { return Err(ERROR_NOT_ALL_ASSIGNED); }; @@ -82,7 +84,9 @@ pub fn process_is_elevated() -> Result { error }); } - let Some(token) = OwnedHandle::new(raw_token) else { + // 安全性: successful OpenProcessToken returns one owned token handle released by + // CloseHandle; ownership moves directly into this guard. + let Some(token) = OwnedHandle::from_raw(raw_token) else { return Err(ERROR_NOT_ALL_ASSIGNED); }; diff --git a/src/infrastructure/native/ui.rs b/src/infrastructure/native/ui.rs index 85e4ee7..0bdc2d2 100644 --- a/src/infrastructure/native/ui.rs +++ b/src/infrastructure/native/ui.rs @@ -351,6 +351,13 @@ pub fn append_32_bit_suffix(label: &str, show_suffix: bool) -> Cow<'_, str> { Cow::Owned(format!("{label} {}", text(TextKey::Bitness32Suffix))) } +/// Forwards a message to a raw window procedure obtained from Win32. +/// +/// # Safety +/// +/// When `wndproc` is `Some`, it must be a live procedure pointer returned or registered by Win32 +/// for the supplied window. `hwnd`, `msg`, `wparam`, and `lparam` must satisfy that procedure's +/// message-specific contract and remain valid for the synchronous call. pub unsafe fn call_window_proc( wndproc: WNDPROC, hwnd: HWND, @@ -507,22 +514,55 @@ pub fn window_rect_relative_to_page(hwnd: HWND, page_hwnd: HWND) -> RECT { } } -pub fn copy_text_to_callback_buffer(buffer: *mut u16, capacity: usize, text: &str) { - if buffer.is_null() || capacity == 0 { +/// Copies text into a caller-provided UTF-16 buffer and always terminates a non-empty buffer. +pub fn copy_text_to_utf16_buffer(buffer: &mut [u16], text: &str) { + if buffer.is_empty() { return; } - let max_len = capacity.saturating_sub(1); + let max_len = buffer.len() - 1; let mut written = 0usize; - for code_unit in text.encode_utf16().take(max_len) { - // 安全性: `written` is bounded by capacity - 1. - unsafe { *buffer.add(written) = code_unit }; - written += 1; + for character in text.chars() { + let mut encoded = [0u16; 2]; + let code_units = character.encode_utf16(&mut encoded); + if written + code_units.len() > max_len { + break; + } + buffer[written..written + code_units.len()].copy_from_slice(code_units); + written += code_units.len(); + } + buffer[written] = 0; +} + +/// Copies text into a raw callback buffer supplied synchronously by Win32. +/// +/// Null and zero-capacity buffers are ignored. +/// +/// # Safety +/// +/// When `buffer` is non-null and `capacity` is non-zero, it must be valid and properly aligned for +/// writes of `capacity` consecutive `u16` values. The allocation must remain live and exclusively +/// writable for this call, with no references aliasing the written region. +pub unsafe fn copy_text_to_callback_buffer(buffer: *mut u16, capacity: usize, text: &str) { + if buffer.is_null() || capacity == 0 { + return; } - // 安全性: one slot was reserved for the terminator. - unsafe { *buffer.add(written) = 0 }; -} + // 安全性: the function-level contract supplies validity, alignment, size, and exclusive + // access; the slice cannot outlive this synchronous call. + let buffer = unsafe { std::slice::from_raw_parts_mut(buffer, capacity) }; + copy_text_to_utf16_buffer(buffer, text); +} + +/// Copies a NUL-terminated UTF-16 string from a raw Win32 pointer. +/// +/// Null is accepted and produces an empty string. +/// +/// # Safety +/// +/// A non-null `ptr` must be valid and properly aligned for reads through the first NUL code unit, +/// with that terminator occurring within 32,768 `u16` elements. The allocation must remain live +/// and must not be mutated for the duration of this call. pub unsafe fn widestr_ptr_to_string(ptr: *const u16) -> String { unsafe { // 安全性: 调用方必须传入有效的、以 NUL 结尾的 UTF-16 字符串指针。 @@ -540,3 +580,33 @@ pub unsafe fn widestr_ptr_to_string(ptr: *const u16) -> String { } } } + +#[cfg(test)] +mod tests { + use super::copy_text_to_utf16_buffer; + + #[test] + fn callback_text_is_nul_terminated_and_leaves_unused_tail_untouched() { + let mut buffer = [0xAAAA; 8]; + copy_text_to_utf16_buffer(&mut buffer, "Task"); + assert_eq!(&buffer[..5], &[b'T' as u16, b'a' as u16, b's' as u16, b'k' as u16, 0]); + assert_eq!(&buffer[5..], &[0xAAAA; 3]); + } + + #[test] + fn callback_text_truncation_does_not_split_surrogate_pairs() { + let mut buffer = [0xAAAA; 3]; + copy_text_to_utf16_buffer(&mut buffer, "A😀"); + assert_eq!(buffer, [b'A' as u16, 0, 0xAAAA]); + } + + #[test] + fn callback_text_handles_empty_and_terminator_only_buffers() { + let mut empty = []; + copy_text_to_utf16_buffer(&mut empty, "ignored"); + + let mut terminator = [0xAAAA]; + copy_text_to_utf16_buffer(&mut terminator, "ignored"); + assert_eq!(terminator, [0]); + } +} diff --git a/src/pages/applications/icons.rs b/src/pages/applications/icons.rs index 7f6e460..a96bdca 100644 --- a/src/pages/applications/icons.rs +++ b/src/pages/applications/icons.rs @@ -21,7 +21,7 @@ use std::sync::{ use std::thread::{self, JoinHandle}; use crossbeam_channel::{Receiver, Sender, bounded}; -use windows_sys::Win32::Foundation::{ERROR_INVALID_DATA, HWND}; +use windows_sys::Win32::Foundation::{ERROR_INVALID_DATA, HWND, SetLastError}; use windows_sys::Win32::UI::Controls::{ HIMAGELIST, ImageList_Create, ImageList_Destroy, ImageList_Remove, ImageList_ReplaceIcon, }; @@ -35,7 +35,7 @@ use windows_sys::Win32::UI::WindowsAndMessaging::{ }; use super::{TaskIdentity, last_error_or_gen_failure, window_matches_identity}; -use crate::infrastructure::native::{destroy_icon_handle, record_win32_error}; +use crate::infrastructure::native::{OwnedIcon, record_win32_error}; use crate::ui::assets::{DEFAULT_PROCESS_ICON_RESOURCE, load_icon_resource}; const MAX_TASK_ICON_WORKERS: usize = 8; @@ -52,8 +52,8 @@ pub(super) struct TaskIconRequest { pub(super) struct TaskIconResult { pub(super) identity: TaskIdentity, - small_icon: isize, - large_icon: isize, + small_icon: Option, + large_icon: Option, } pub(super) struct TaskIconCompletion { @@ -68,27 +68,12 @@ pub(super) struct TaskIconBatchRequest { } impl TaskIconResult { - pub(super) fn take_small_icon(&mut self) -> HICON { - let icon = self.small_icon as HICON; - self.small_icon = 0; - icon + pub(super) fn take_small_icon(&mut self) -> Option { + self.small_icon.take() } - pub(super) fn take_large_icon(&mut self) -> HICON { - let icon = self.large_icon as HICON; - self.large_icon = 0; - icon - } -} - -impl Drop for TaskIconResult { - fn drop(&mut self) { - if self.small_icon != 0 { - destroy_icon_handle(self.small_icon as HICON); - } - if self.large_icon != 0 { - destroy_icon_handle(self.large_icon as HICON); - } + pub(super) fn take_large_icon(&mut self) -> Option { + self.large_icon.take() } } @@ -96,8 +81,8 @@ impl Drop for TaskIconResult { pub(super) struct TaskIconStore { small: HIMAGELIST, large: HIMAGELIST, - default_small: HICON, - default_large: HICON, + default_small: Option, + default_large: Option, pub(super) free_slots: Vec, } @@ -131,7 +116,7 @@ impl TaskIconStore { return Err(last_error_or_gen_failure()); } - next.default_small = load_icon_resource( + next.default_small = Some(load_icon_resource( DEFAULT_PROCESS_ICON_RESOURCE, windows_sys::Win32::UI::WindowsAndMessaging::GetSystemMetrics( windows_sys::Win32::UI::WindowsAndMessaging::SM_CXSMICON, @@ -140,8 +125,8 @@ impl TaskIconStore { windows_sys::Win32::UI::WindowsAndMessaging::SM_CYSMICON, ), 0, - ); - next.default_large = load_icon_resource( + )?); + next.default_large = Some(load_icon_resource( DEFAULT_PROCESS_ICON_RESOURCE, windows_sys::Win32::UI::WindowsAndMessaging::GetSystemMetrics( windows_sys::Win32::UI::WindowsAndMessaging::SM_CXICON, @@ -150,10 +135,7 @@ impl TaskIconStore { windows_sys::Win32::UI::WindowsAndMessaging::SM_CYICON, ), 0, - ); - if next.default_small.is_null() || next.default_large.is_null() { - return Err(last_error_or_gen_failure()); - } + )?); next.reset()?; } *self = next; @@ -168,8 +150,12 @@ impl TaskIconStore { self.large } - pub(super) fn allocate(&mut self, small_icon: HICON, large_icon: HICON) -> Result { - if small_icon.is_null() && large_icon.is_null() { + pub(super) fn allocate( + &mut self, + small_icon: Option, + large_icon: Option, + ) -> Result { + if small_icon.is_none() && large_icon.is_none() { return Ok(0); } let requested_slot = self.free_slots.last().copied(); @@ -177,50 +163,43 @@ impl TaskIconStore { let target = match requested_slot { Some(slot) => match i32::try_from(slot) { Ok(slot) => slot, - Err(_) => { - destroy_icon_handle(small_icon); - destroy_icon_handle(large_icon); - return Err(ERROR_INVALID_DATA); - } + Err(_) => return Err(ERROR_INVALID_DATA), }, None => -1, }; - let small_index = - match replace_owned_icon(self.small, target, small_icon, self.default_small) { - Ok(index) => index, - Err(error) => { - destroy_icon_handle(large_icon); - return Err(error); - } - }; - let large_index = - match replace_owned_icon(self.large, target, large_icon, self.default_large) { - Ok(index) => index, - Err(error) => { - rollback_icon_slot( - self.small, - small_index, - self.default_small, - appended, - "task small icon rollback", - ); - return Err(error); - } - }; + let default_small = self.default_small_raw(); + let default_large = self.default_large_raw(); + let small_index = match replace_owned_icon(self.small, target, small_icon, default_small) { + Ok(index) => index, + Err(error) => return Err(error), + }; + let large_index = match replace_owned_icon(self.large, target, large_icon, default_large) { + Ok(index) => index, + Err(error) => { + rollback_icon_slot( + self.small, + small_index, + default_small, + appended, + "task small icon rollback", + ); + return Err(error); + } + }; if small_index != large_index || requested_slot.is_some_and(|slot| slot != small_index) { rollback_icon_slot( self.small, small_index, - self.default_small, + default_small, appended, "task small icon rollback", ); rollback_icon_slot( self.large, large_index, - self.default_large, + default_large, appended, "task large icon rollback", ); @@ -240,10 +219,10 @@ impl TaskIconStore { return Err(ERROR_INVALID_DATA); }; let small_result = - unsafe { ImageList_ReplaceIcon(self.small, slot_i32, self.default_small) }; + unsafe { ImageList_ReplaceIcon(self.small, slot_i32, self.default_small_raw()) }; let small_error = (small_result < 0).then(last_error_or_gen_failure); let large_result = - unsafe { ImageList_ReplaceIcon(self.large, slot_i32, self.default_large) }; + unsafe { ImageList_ReplaceIcon(self.large, slot_i32, self.default_large_raw()) }; let large_error = (large_result < 0).then(last_error_or_gen_failure); if let Some(error) = small_error.or(large_error) { return Err(error); @@ -252,21 +231,43 @@ impl TaskIconStore { Ok(()) } - unsafe fn reset(&mut self) -> Result<(), u32> { - unsafe { - ImageList_Remove(self.small, -1); - ImageList_Remove(self.large, -1); - let small_index = ImageList_ReplaceIcon(self.small, -1, self.default_small); - let large_index = ImageList_ReplaceIcon(self.large, -1, self.default_large); - if small_index != 0 || large_index != 0 { - let error = last_error_or_gen_failure(); + fn reset(&mut self) -> Result<(), u32> { + // ImageList_Remove owns only list storage; the default icons remain borrowed here. + unsafe { SetLastError(0) }; + if unsafe { ImageList_Remove(self.small, -1) } == 0 { + return Err(last_error_or_gen_failure()); + } + unsafe { SetLastError(0) }; + if unsafe { ImageList_Remove(self.large, -1) } == 0 { + return Err(last_error_or_gen_failure()); + } + + unsafe { SetLastError(0) }; + let small_index = unsafe { + ImageList_ReplaceIcon(self.small, -1, self.default_small_raw()) + }; + if small_index != 0 { + let error = last_error_or_gen_failure(); + unsafe { + ImageList_Remove(self.small, -1); + ImageList_Remove(self.large, -1); + } + return Err(error); + } + unsafe { SetLastError(0) }; + let large_index = unsafe { + ImageList_ReplaceIcon(self.large, -1, self.default_large_raw()) + }; + if large_index != 0 { + let error = last_error_or_gen_failure(); + unsafe { ImageList_Remove(self.small, -1); ImageList_Remove(self.large, -1); - return Err(error); } - self.free_slots.clear(); - Ok(()) + return Err(error); } + self.free_slots.clear(); + Ok(()) } pub(super) fn destroy(&mut self) { @@ -280,12 +281,22 @@ impl TaskIconStore { self.large = 0; } } - destroy_icon_handle(self.default_small); - destroy_icon_handle(self.default_large); - self.default_small = null_mut(); - self.default_large = null_mut(); + self.default_small = None; + self.default_large = None; self.free_slots.clear(); } + + fn default_small_raw(&self) -> HICON { + self.default_small + .as_ref() + .map_or(null_mut(), OwnedIcon::as_raw) + } + + fn default_large_raw(&self) -> HICON { + self.default_large + .as_ref() + .map_or(null_mut(), OwnedIcon::as_raw) + } } impl Drop for TaskIconStore { @@ -461,16 +472,9 @@ fn collect_task_icon_work(work: &TaskIconBatchWork) -> TaskIconPoolResult { if window_matches_identity(request.identity) { results.push(TaskIconResult { identity: request.identity, - small_icon: small_icon as isize, - large_icon: large_icon as isize, + small_icon, + large_icon, }); - } else { - if !small_icon.is_null() { - destroy_icon_handle(small_icon); - } - if !large_icon.is_null() { - destroy_icon_handle(large_icon); - } } } Ok(results) @@ -519,7 +523,7 @@ fn io_error_code(error: std::io::Error) -> u32 { .unwrap_or(windows_sys::Win32::Foundation::ERROR_GEN_FAILURE) } -fn fetch_window_icons(hwnd: HWND, is_hung: bool) -> (HICON, HICON) { +fn fetch_window_icons(hwnd: HWND, is_hung: bool) -> (Option, Option) { let (small2, big) = if is_hung { (null_mut(), null_mut()) } else { @@ -562,20 +566,17 @@ fn fetch_window_icons(hwnd: HWND, is_hung: bool) -> (HICON, HICON) { } } - unsafe { - ( - if small_source.is_null() { - null_mut() - } else { - CopyIcon(small_source) - }, - if large_source.is_null() { - null_mut() - } else { - CopyIcon(large_source) - }, - ) + (copy_icon_source(small_source), copy_icon_source(large_source)) +} + +fn copy_icon_source(source: HICON) -> Option { + if source.is_null() { + return None; } + // 安全性: the source is borrowed for this synchronous call. Microsoft documents a successful + // `CopyIcon` result as a new owned icon that the application must release with `DestroyIcon`. + let icon = unsafe { CopyIcon(source) }; + unsafe { OwnedIcon::from_raw(icon) } } // 通过 SendMessageTimeoutW(WM_GETICON) 查询窗口图标。 @@ -614,16 +615,13 @@ fn query_class_icon_source(hwnd: HWND, class_index: i32) -> HICON { fn replace_owned_icon( imagelist: HIMAGELIST, target: i32, - owned_icon: HICON, + owned_icon: Option, default_icon: HICON, ) -> Result { - let source = if owned_icon.is_null() { - default_icon - } else { - owned_icon - }; + let source = owned_icon + .as_ref() + .map_or(default_icon, OwnedIcon::as_raw); let index = unsafe { ImageList_ReplaceIcon(imagelist, target, source) }; - destroy_icon_handle(owned_icon); if index < 0 { Err(last_error_or_gen_failure()) } else { diff --git a/src/pages/applications/mod.rs b/src/pages/applications/mod.rs index 00c9434..20e62fd 100644 --- a/src/pages/applications/mod.rs +++ b/src/pages/applications/mod.rs @@ -345,12 +345,26 @@ impl TaskPageState { } } - pub fn handle_notify(&mut self, lparam: LPARAM) -> isize { + /// Handles a notification forwarded by the task page dialog procedure. + /// + /// # Safety + /// + /// `lparam` must be the live `WM_NOTIFY` payload for this page's ListView. The payload must + /// have the structure implied by its `NMHDR::code` and remain readable (and, for + /// `LVN_GETDISPINFOW`, writable) for the duration of this synchronous call. + pub unsafe fn handle_notify(&mut self, lparam: LPARAM) -> isize { // 任务页同样依赖 ListView 通知来驱动选择同步、双击切换和列表排序。 // 安全性: task dialog proc forwards only WM_NOTIFY LPARAM values from Win32; each cast is // matched to the notification code before accessing the payload. unsafe { - let notify_header = &*(lparam as *const NMHDR); + let Some(notify_header) = (lparam as *const NMHDR).as_ref() else { + return 0; + }; + if notify_header.idFrom != IDC_TASKLIST as usize + || notify_header.hwndFrom != self.list_hwnd() + { + return 0; + } match notify_header.code { code if code == LVN_GETDISPINFOW => { let display_info = &mut *(lparam as *mut NMLVDISPINFOW); @@ -1139,8 +1153,13 @@ impl TaskPageState { } } - fn fill_display_info(&self, item: &mut LVITEMW) { - // 安全性: this function is a safe facade over Win32/FFI work; all callers run it on the owning UI thread and the existing body preserves its original handle/pointer invariants. + /// Fills an `LVN_GETDISPINFOW` callback buffer. + /// + /// # Safety + /// + /// When `LVIF_TEXT` is set, `item.pszText` must be writable for at least + /// `item.cchTextMax` UTF-16 code units and remain exclusively available for this call. + unsafe fn fill_display_info(&self, item: &mut LVITEMW) { unsafe { if (item.mask & LVIF_TEXT) == 0 || item.iItem < 0 @@ -1441,7 +1460,7 @@ mod tests { #[test] fn missing_custom_icons_share_the_default_slot() { let mut store = TaskIconStore::default(); - assert_eq!(store.allocate(null_mut(), null_mut()), Ok(0)); + assert_eq!(store.allocate(None, None), Ok(0)); assert!(store.free_slots.is_empty()); } diff --git a/src/pages/cpu/mod.rs b/src/pages/cpu/mod.rs index fdb8f44..590263e 100644 --- a/src/pages/cpu/mod.rs +++ b/src/pages/cpu/mod.rs @@ -927,7 +927,14 @@ impl CpuPageState { Ok(()) } - pub(crate) fn handle_notify(&mut self, lparam: LPARAM) -> isize { + /// Handles a notification forwarded by the CPU page dialog procedure. + /// + /// # Safety + /// + /// `lparam` must point to a live `WM_NOTIFY` payload for the duration of this synchronous + /// call. A `TTN_GETDISPINFOW` payload from `tooltip_hwnd` must be a writable + /// `NMTTDISPINFOW`. + pub(crate) unsafe fn handle_notify(&mut self, lparam: LPARAM) -> isize { if lparam == 0 || self.tooltip_hwnd.is_null() { return 0; } diff --git a/src/pages/network.rs b/src/pages/network.rs index 32cb0a1..d9bbd2c 100644 --- a/src/pages/network.rs +++ b/src/pages/network.rs @@ -89,12 +89,24 @@ struct OwnedIfTable { } impl OwnedIfTable { - fn new(ptr: *mut MIB_IF_TABLE2) -> Option { + /// Takes ownership of a table returned by a successful `GetIfTable2` call. + /// + /// # Safety + /// + /// `ptr` must be null or uniquely owned storage allocated by `GetIfTable2` that has not + /// already been passed to `FreeMibTable`. Ownership is transferred to the returned value. + unsafe fn from_raw(ptr: *mut MIB_IF_TABLE2) -> Option { (!ptr.is_null()).then_some(Self { ptr }) } - fn as_ptr(&self) -> *mut MIB_IF_TABLE2 { - self.ptr + fn rows(&self) -> &[MIB_IF_ROW2] { + // SAFETY: construction guarantees that `ptr` is the live allocation returned by + // `GetIfTable2`; that allocation contains exactly `NumEntries` initialized rows and + // remains owned by `self` for the lifetime of the returned slice. + unsafe { + let count = (*self.ptr).NumEntries as usize; + slice::from_raw_parts((*self.ptr).Table.as_ptr(), count) + } } } @@ -243,29 +255,27 @@ impl NetworkPageState { Self::default() } - pub unsafe fn initialize( + pub fn initialize( &mut self, hwnd: HWND, main_hwnd: HWND, hwnd_tabs: HWND, ) -> Result<(), u32> { - unsafe { - // 初始化只建立控件和基础布局；当前页由激活入口采样，隐藏页由首帧后的预热消息采样。 - self.hwnd = hwnd; - self.main_hwnd = main_hwnd; - self.hwnd_tabs = hwnd_tabs; - self.start_worker_thread()?; - let list = self.list_hwnd(); - if !list.is_null() { - subclass_list_view(list); - } - self.configure_columns(); - self.size_page(); - Ok(()) + // 初始化只建立控件和基础布局；当前页由激活入口采样，隐藏页由首帧后的预热消息采样。 + self.hwnd = hwnd; + self.main_hwnd = main_hwnd; + self.hwnd_tabs = hwnd_tabs; + self.start_worker_thread()?; + let list = self.list_hwnd(); + if !list.is_null() { + subclass_list_view(list); } + self.configure_columns(); + self.size_page(); + Ok(()) } - pub unsafe fn apply_options(&mut self, options: &Options) { + pub fn apply_options(&mut self, options: &Options) { // 网络页当前只有无标题布局依赖全局选项，因此这里比较轻量。 let previous = self.no_title; self.no_title = options.no_title(); @@ -273,27 +283,21 @@ impl NetworkPageState { return; } - unsafe { - self.size_page(); - } + self.size_page(); } - pub unsafe fn no_title(&self) -> bool { + pub fn no_title(&self) -> bool { self.no_title } - pub unsafe fn timer_event(&mut self) { - unsafe { - // 历史轴只在 worker 成功提交快照时推进，避免慢采样或失败时重绘旧数据。 - self.refresh(); - } + pub fn timer_event(&mut self) { + // 历史轴只在 worker 成功提交快照时推进，避免慢采样或失败时重绘旧数据。 + self.refresh(); } - pub unsafe fn destroy(&mut self) { - unsafe { - self.stop_worker_thread(); - self.destroy_graphs(); - } + pub fn destroy(&mut self) { + self.stop_worker_thread(); + self.destroy_graphs(); } fn start_worker_thread(&mut self) -> Result<(), u32> { @@ -307,7 +311,7 @@ impl NetworkPageState { keep_pending, |()| NetworkWorkerCompletion { sampled_at: Instant::now(), - result: unsafe { NetworkPageState::collect_adapters() }, + result: NetworkPageState::collect_adapters(), }, )?); Ok(()) @@ -317,7 +321,7 @@ impl NetworkPageState { self.worker = None; } - unsafe fn ensure_graph_surface(&mut self, width: i32, height: i32) -> bool { + fn ensure_graph_surface(&mut self, width: i32, height: i32) -> bool { unsafe { if self.cached_graph_dc.is_null() || width > self.cached_graph_width @@ -375,6 +379,11 @@ impl NetworkPageState { } } + /// Draws one network graph into a borrowed device context. + /// + /// # Safety + /// + /// `hdc` must be a live device context that is valid for drawing throughout this call. pub unsafe fn draw_graph(&mut self, hdc: HDC, rect: RECT, pane_index: usize) { unsafe { // 每个图表面板都根据当前适配器的历史数据独立绘制， @@ -512,7 +521,7 @@ impl NetworkPageState { frame.end() } - pub unsafe fn size_page(&mut self) { + pub fn size_page(&mut self) { unsafe { // 网络页需要同时布局“多块图表 + 滚动条 + 底部列表”， // 因此会先算出一页能显示多少图，再决定是否出现滚动条。 @@ -669,11 +678,11 @@ impl NetworkPageState { } } - pub unsafe fn handle_vscroll(&mut self, wparam: WPARAM) -> isize { - unsafe { self.handle_vscroll_steps(wparam, 1) } + pub fn handle_vscroll(&mut self, wparam: WPARAM) -> isize { + self.handle_vscroll_steps(wparam, 1) } - unsafe fn handle_vscroll_steps(&mut self, wparam: WPARAM, steps: i32) -> isize { + fn handle_vscroll_steps(&mut self, wparam: WPARAM, steps: i32) -> isize { unsafe { let scrollbar = GetDlgItem(self.hwnd, IDC_GRAPHSCROLLVERT); if scrollbar.is_null() { @@ -719,29 +728,25 @@ impl NetworkPageState { } } - pub unsafe fn handle_mouse_wheel(&mut self, wparam: WPARAM) -> isize { - unsafe { - // 鼠标滚轮被翻译成垂直滚动命令，保持和滚动条一致的行为。 - let delta = i32::from(hiword(wparam) as i16); - if delta == 0 { - return 0; - } - - let wheel_delta = WHEEL_DELTA as i32; - let step = ((delta.abs() + wheel_delta - 1) / wheel_delta).max(1); - let command = if delta < 0 { SB_LINEDOWN } else { SB_LINEUP }; - self.handle_vscroll_steps(command as usize, step) + pub fn handle_mouse_wheel(&mut self, wparam: WPARAM) -> isize { + // 鼠标滚轮被翻译成垂直滚动命令，保持和滚动条一致的行为。 + let delta = i32::from(hiword(wparam) as i16); + if delta == 0 { + return 0; } + + let wheel_delta = WHEEL_DELTA as i32; + let step = ((delta.abs() + wheel_delta - 1) / wheel_delta).max(1); + let command = if delta < 0 { SB_LINEDOWN } else { SB_LINEUP }; + self.handle_vscroll_steps(command as usize, step) } - unsafe fn refresh(&mut self) { - unsafe { - self.drain_worker_results(); - self.schedule_collection(); - } + fn refresh(&mut self) { + self.drain_worker_results(); + self.schedule_collection(); } - unsafe fn schedule_collection(&mut self) { + fn schedule_collection(&mut self) { let Some(worker) = self.worker.as_mut() else { self.set_refresh_error(windows_sys::Win32::Foundation::ERROR_BROKEN_PIPE); return; @@ -752,7 +757,7 @@ impl NetworkPageState { } } - unsafe fn drain_worker_results(&mut self) { + fn drain_worker_results(&mut self) { let drained = match self.worker.as_mut() { Some(worker) => worker.drain(self.hwnd), None => return, @@ -760,7 +765,7 @@ impl NetworkPageState { for completion in drained.completions { crate::infrastructure::diagnostics::with_operation_id( completion.operation_id, - || unsafe { + || { match completion.value.result { Ok(adapters) => { self.last_refresh_error = None; @@ -783,45 +788,62 @@ impl NetworkPageState { self.last_refresh_error = Some(error); } - pub unsafe fn handle_worker_completion(&mut self) { - unsafe { - self.drain_worker_results(); - } + pub fn handle_worker_completion(&mut self) { + self.drain_worker_results(); } - unsafe fn apply_adapter_snapshot( + fn apply_adapter_snapshot( &mut self, raw_adapters: Vec, sampled_at: Instant, ) { - unsafe { - // UI 提交阶段把完整原始快照转换为列表文本和历史曲线；过程中没有系统查询。 - let raw_adapters = collapse_raw_adapters(raw_adapters); - let needs_initial_layout = self.last_sample_time.is_none(); - let previous_adapter_count = self.adapters.len(); - let previous_adapter_order = self - .adapters - .iter() - .map(|adapter| adapter.key) - .collect::>(); - let elapsed_secs = self - .last_sample_time - .replace(sampled_at) - .map(|previous| sampled_at.duration_since(previous).as_secs_f64()) - .unwrap_or(0.0); - - let mut previous_by_key = HashMap::with_capacity(self.adapters.len()); - for adapter in self.adapters.drain(..) { - previous_by_key.insert(adapter.key, adapter); - } + // UI 提交阶段把完整原始快照转换为列表文本和历史曲线；过程中没有系统查询。 + let raw_adapters = collapse_raw_adapters(raw_adapters); + let needs_initial_layout = self.last_sample_time.is_none(); + let previous_adapter_count = self.adapters.len(); + let previous_adapter_order = self + .adapters + .iter() + .map(|adapter| adapter.key) + .collect::>(); + let elapsed_secs = self + .last_sample_time + .replace(sampled_at) + .map(|previous| sampled_at.duration_since(previous).as_secs_f64()) + .unwrap_or(0.0); + + let mut previous_by_key = HashMap::with_capacity(self.adapters.len()); + for adapter in self.adapters.drain(..) { + previous_by_key.insert(adapter.key, adapter); + } - let mut adapters = Vec::with_capacity(raw_adapters.len()); - let mut adapter_labels_changed = false; - for raw in raw_adapters { - let (mut sent_history, mut received_history, mut total_history, previous_state) = - if let Some(previous_adapter) = previous_by_key.remove(&raw.key) { - adapter_labels_changed |= previous_adapter.name != raw.name; - let NetworkAdapterEntry { + let mut adapters = Vec::with_capacity(raw_adapters.len()); + let mut adapter_labels_changed = false; + for raw_adapter in raw_adapters { + let (mut sent_history, mut received_history, mut total_history, previous_state) = + if let Some(previous_adapter) = previous_by_key.remove(&raw_adapter.key) { + adapter_labels_changed |= previous_adapter.name != raw_adapter.name; + let NetworkAdapterEntry { + name, + state, + link_speed, + utilization, + bytes_sent, + bytes_received, + bytes_total, + current_sent, + current_received, + sent_history, + received_history, + total_history, + .. + } = previous_adapter; + + ( + sent_history, + received_history, + total_history, + Some(PreviousAdapterState { name, state, link_speed, @@ -831,155 +853,147 @@ impl NetworkPageState { bytes_total, current_sent, current_received, - sent_history, - received_history, - total_history, - .. - } = previous_adapter; - - ( - sent_history, - received_history, - total_history, - Some(PreviousAdapterState { - name, - state, - link_speed, - utilization, - bytes_sent, - bytes_received, - bytes_total, - current_sent, - current_received, - }), - ) - } else { - adapter_labels_changed = true; - ( - HistoryBuffer::zeroed(HIST_SIZE), - HistoryBuffer::zeroed(HIST_SIZE), - HistoryBuffer::zeroed(HIST_SIZE), - None, - ) - }; - let counter_delta = adapter_counter_delta( - raw.bytes_sent, - raw.bytes_received, - previous_state - .as_ref() - .map(|state| (state.current_sent, state.current_received)), - ); - // A zero curve point marks an unavailable interval after first sight or counter - // reset; the textual value remains "-" so it is not presented as measured idle. - let total_delta = counter_delta.map_or(0, |delta| delta.2); - let sent_util = counter_delta.map_or(0, |delta| { - utilization_percent_for_history(delta.0, raw.link_speed_bps, elapsed_secs) - }); - let received_util = counter_delta.map_or(0, |delta| { - utilization_percent_for_history(delta.1, raw.link_speed_bps, elapsed_secs) - }); - let total_util = counter_delta.map_or(0, |delta| { - utilization_percent_for_history(delta.2, raw.link_speed_bps, elapsed_secs) - }); - - push_history(&mut sent_history, sent_util); - push_history(&mut received_history, received_util); - push_history(&mut total_history, total_util); - - let bytes_total = raw.bytes_sent.checked_add(raw.bytes_received); - let mut adapter = NetworkAdapterEntry { - interface_index: raw.interface_index, - key: raw.key, - name: raw.name, - state: raw.state, - link_speed: format_link_speed(raw.link_speed_bps), - utilization: counter_delta - .map(|_| utilization_text(total_delta, raw.link_speed_bps, elapsed_secs)) - .unwrap_or_else(|| "-".to_string()), - bytes_sent: format_counter(raw.bytes_sent), - bytes_received: format_counter(raw.bytes_received), - bytes_total: bytes_total - .map(format_counter) - .unwrap_or_else(|| "-".to_string()), - current_sent: raw.bytes_sent, - current_received: raw.bytes_received, - sent_history, - received_history, - total_history, - dirty: true, + }), + ) + } else { + adapter_labels_changed = true; + ( + HistoryBuffer::zeroed(HIST_SIZE), + HistoryBuffer::zeroed(HIST_SIZE), + HistoryBuffer::zeroed(HIST_SIZE), + None, + ) }; - if let Some(previous_state) = previous_state.as_ref() { - adapter.dirty = previous_state.name != adapter.name - || previous_state.state != adapter.state - || previous_state.link_speed != adapter.link_speed - || previous_state.utilization != adapter.utilization - || previous_state.bytes_sent != adapter.bytes_sent - || previous_state.bytes_received != adapter.bytes_received - || previous_state.bytes_total != adapter.bytes_total; - } - adapters.push(adapter); + let counter_delta = adapter_counter_delta( + raw_adapter.bytes_sent, + raw_adapter.bytes_received, + previous_state + .as_ref() + .map(|state| (state.current_sent, state.current_received)), + ); + // A zero curve point marks an unavailable interval after first sight or counter + // reset; the textual value remains "-" so it is not presented as measured idle. + let total_delta = counter_delta.map_or(0, |delta| delta.2); + let sent_util = counter_delta.map_or(0, |delta| { + utilization_percent_for_history( + delta.0, + raw_adapter.link_speed_bps, + elapsed_secs, + ) + }); + let received_util = counter_delta.map_or(0, |delta| { + utilization_percent_for_history( + delta.1, + raw_adapter.link_speed_bps, + elapsed_secs, + ) + }); + let total_util = counter_delta.map_or(0, |delta| { + utilization_percent_for_history( + delta.2, + raw_adapter.link_speed_bps, + elapsed_secs, + ) + }); + + push_history(&mut sent_history, sent_util); + push_history(&mut received_history, received_util); + push_history(&mut total_history, total_util); + + let bytes_total = raw_adapter + .bytes_sent + .checked_add(raw_adapter.bytes_received); + let mut adapter = NetworkAdapterEntry { + interface_index: raw_adapter.interface_index, + key: raw_adapter.key, + name: raw_adapter.name, + state: raw_adapter.state, + link_speed: format_link_speed(raw_adapter.link_speed_bps), + utilization: counter_delta + .map(|_| { + utilization_text(total_delta, raw_adapter.link_speed_bps, elapsed_secs) + }) + .unwrap_or_else(|| "-".to_string()), + bytes_sent: format_counter(raw_adapter.bytes_sent), + bytes_received: format_counter(raw_adapter.bytes_received), + bytes_total: bytes_total + .map(format_counter) + .unwrap_or_else(|| "-".to_string()), + current_sent: raw_adapter.bytes_sent, + current_received: raw_adapter.bytes_received, + sent_history, + received_history, + total_history, + dirty: true, + }; + if let Some(previous_state) = previous_state.as_ref() { + adapter.dirty = previous_state.name != adapter.name + || previous_state.state != adapter.state + || previous_state.link_speed != adapter.link_speed + || previous_state.utilization != adapter.utilization + || previous_state.bytes_sent != adapter.bytes_sent + || previous_state.bytes_received != adapter.bytes_received + || previous_state.bytes_total != adapter.bytes_total; } + adapters.push(adapter); + } - self.adapters = adapters; - self.scroll_offset = (self.scroll_offset + 2) % GRAPH_GRID; - let labels_changed = adapter_labels_changed - || previous_adapter_order - .iter() - .copied() - .ne(self.adapters.iter().map(|adapter| adapter.key)); - self.update_listview(); - if needs_initial_layout || previous_adapter_count != self.adapters.len() { - self.size_page(); - } else if labels_changed { - self.label_graphs(); - } - self.update_graphs(); + self.adapters = adapters; + self.scroll_offset = (self.scroll_offset + 2) % GRAPH_GRID; + let labels_changed = adapter_labels_changed + || previous_adapter_order + .iter() + .copied() + .ne(self.adapters.iter().map(|adapter| adapter.key)); + self.update_listview(); + if needs_initial_layout || previous_adapter_count != self.adapters.len() { + self.size_page(); + } else if labels_changed { + self.label_graphs(); } + self.update_graphs(); } - unsafe fn collect_adapters() -> Result, u32> { - unsafe { - let mut table = null_mut::(); - let status = GetIfTable2(&mut table); - let table = OwnedIfTable::new(table); - if status != 0 { - return Err(status); + fn collect_adapters() -> Result, u32> { + let mut raw_table = null_mut::(); + let status = unsafe { GetIfTable2(&mut raw_table) }; + if status != 0 { + return Err(status); + } + // SAFETY: a successful `GetIfTable2` call transfers a fresh table allocation through + // `raw_table`; this is the sole owner and will release it with `FreeMibTable`. + let table = unsafe { OwnedIfTable::from_raw(raw_table) } + .ok_or(windows_sys::Win32::Foundation::ERROR_INVALID_DATA)?; + + let mut adapters = Vec::with_capacity(table.rows().len()); + for row in table.rows() { + if !include_adapter(row) { + continue; } - let table = table.ok_or(windows_sys::Win32::Foundation::ERROR_INVALID_DATA)?; - let table_ptr = table.as_ptr(); - - let count = (*table_ptr).NumEntries as usize; - let mut adapters = Vec::with_capacity(count); - let rows = slice::from_raw_parts((*table_ptr).Table.as_ptr(), count); - for row in rows { - if !include_adapter(row) { - continue; - } - let mut name = wide_array_to_string(&row.Alias); - if name.is_empty() { - name = wide_array_to_string(&row.Description); - } - let key = AdapterIdentity { - luid: row.InterfaceLuid.Value, - interface_index: row.InterfaceIndex, - }; - - adapters.push(RawAdapterEntry { - interface_index: row.InterfaceIndex, - key, - name, - state: adapter_state_text(row.OperStatus), - link_speed_bps: row.ReceiveLinkSpeed.max(row.TransmitLinkSpeed), - bytes_sent: row.OutOctets, - bytes_received: row.InOctets, - }); + let mut name = wide_array_to_string(&row.Alias); + if name.is_empty() { + name = wide_array_to_string(&row.Description); } - Ok(adapters) + let key = AdapterIdentity { + luid: row.InterfaceLuid.Value, + interface_index: row.InterfaceIndex, + }; + + adapters.push(RawAdapterEntry { + interface_index: row.InterfaceIndex, + key, + name, + state: adapter_state_text(row.OperStatus), + link_speed_bps: row.ReceiveLinkSpeed.max(row.TransmitLinkSpeed), + bytes_sent: row.OutOctets, + bytes_received: row.InOctets, + }); } + Ok(adapters) } - unsafe fn configure_columns(&self) { + fn configure_columns(&self) { unsafe { let list = self.list_hwnd(); if list.is_null() { @@ -1024,7 +1038,7 @@ impl NetworkPageState { } } - unsafe fn update_listview(&self) { + fn update_listview(&self) { unsafe { // 列表只在适配器身份变化时替换整行，普通数值更新尽量走原位写回。 let list = self.list_hwnd(); @@ -1124,7 +1138,7 @@ impl NetworkPageState { } } - unsafe fn ensure_graphs(&mut self, required: usize) { + fn ensure_graphs(&mut self, required: usize) { unsafe { if required <= self.graphs.len() || self.hwnd.is_null() { return; @@ -1187,7 +1201,7 @@ impl NetworkPageState { } } - unsafe fn destroy_graphs(&mut self) { + fn destroy_graphs(&mut self) { unsafe { for graph in self.graphs.drain(..) { if !graph.graph_hwnd.is_null() { @@ -1216,7 +1230,7 @@ impl NetworkPageState { } } - unsafe fn update_graphs(&self) { + fn update_graphs(&self) { unsafe { // 只重绘当前一页真正可见的图表，避免隐藏面板也跟着刷新。 for pane_index in 0..self.graphs_per_page { @@ -1228,7 +1242,7 @@ impl NetworkPageState { } } - unsafe fn label_graphs(&mut self) { + fn label_graphs(&mut self) { unsafe { // 图表标题始终绑定当前可见适配器切片，滚动后要一起更新标题文字。 let first_visible = self.first_visible_adapter(); @@ -1793,7 +1807,7 @@ unsafe fn draw_graph_scale_overlay(hdc: HDC, rect: RECT, scale_max: u8) { } } -unsafe fn layout_spacing() -> (i32, i32) { +fn layout_spacing() -> (i32, i32) { unsafe { let units = GetDialogBaseUnits() as usize; let def_spacing = (DEFSPACING_BASE * i32::from(loword(units))) / DLG_SCALE_X; diff --git a/src/pages/processes/actions.rs b/src/pages/processes/actions.rs index 52c882b..53ab2f5 100644 --- a/src/pages/processes/actions.rs +++ b/src/pages/processes/actions.rs @@ -158,15 +158,17 @@ impl ProcessTreeTerminationOutcome { } impl ProcessPageState { - unsafe fn quick_confirm(&self, title: &str, body: &str) -> bool { - unsafe { - // 用户关闭“确认”选项后，危险操作直接放行，保持与原版 Task Manager 行为一致。 - if !self.confirmations { - return true; - } + fn quick_confirm(&self, title: &str, body: &str) -> bool { + // 用户关闭“确认”选项后，危险操作直接放行，保持与原版 Task Manager 行为一致。 + if !self.confirmations { + return true; + } - let title_wide = to_wide_null(title); - let body_wide = to_wide_null(body); + let title_wide = to_wide_null(title); + let body_wide = to_wide_null(body); + // SAFETY: both UTF-16 buffers are terminated and remain alive for the synchronous call; + // `hwnd_page` is borrowed and no ownership is transferred. + unsafe { MessageBoxW( self.hwnd_page, body_wide.as_ptr(), @@ -177,15 +179,17 @@ impl ProcessPageState { } pub(super) fn show_failure_message(&self, body: &str, error: u32) { + let title = if self.strings.warning.is_empty() { + "Task Manager".to_string() + } else { + self.strings.warning.clone() + }; + let message = format!("{body}\r\n\r\nWin32 error: {error}"); + let title_wide = to_wide_null(&title); + let message_wide = to_wide_null(&message); + // SAFETY: both UTF-16 buffers are terminated and remain live for the synchronous call; + // the page HWND is borrowed and no ownership is transferred. unsafe { - let title = if self.strings.warning.is_empty() { - "Task Manager".to_string() - } else { - self.strings.warning.clone() - }; - let message = format!("{body}\r\n\r\nWin32 error: {error}"); - let title_wide = to_wide_null(&title); - let message_wide = to_wide_null(&message); MessageBoxW( self.hwnd_page, message_wide.as_ptr(), @@ -196,119 +200,121 @@ impl ProcessPageState { } // 结束指定 PID 的进程。先弹确认框，再通过 TerminateProcess 终止。 - pub(super) unsafe fn kill_process(&mut self, identity: ProcIdentity) -> bool { - unsafe { - if !self.quick_confirm(&self.strings.warning, &self.strings.kill) { - return false; - } - - let handle = match open_process_for_identity( - identity, - PROCESS_TERMINATE | PROCESS_QUERY_LIMITED_INFORMATION, - ) { - Ok(handle) => handle, - Err(error) => { - self.show_failure_message(&self.strings.cant_kill, error); - return false; - } - }; - - let result = TerminateProcess(handle.as_raw(), 1); - let error = windows_sys::Win32::Foundation::GetLastError(); + pub(super) fn kill_process(&mut self, identity: ProcIdentity) -> bool { + if !self.quick_confirm(&self.strings.warning, &self.strings.kill) { + return false; + } - if result == 0 { + let handle = match open_process_for_identity( + identity, + PROCESS_TERMINATE | PROCESS_QUERY_LIMITED_INFORMATION, + ) { + Ok(handle) => handle, + Err(error) => { self.show_failure_message(&self.strings.cant_kill, error); - false - } else { - self.paused = false; - self.refresh_processes(); - true + return false; } + }; + + // SAFETY: `open_process_for_identity` returned a live handle with PROCESS_TERMINATE + // access for the exact verified identity. + if unsafe { TerminateProcess(handle.as_raw(), 1) } == 0 { + let error = unsafe { GetLastError() }; + self.show_failure_message(&self.strings.cant_kill, error); + false + } else { + self.paused = false; + self.refresh_processes(); + true } } // 结束进程以及所有子进程。按叶子优先的顺序遍历进程树，逐进程 TerminateProcess。 - pub(super) unsafe fn kill_process_tree(&mut self, identity: ProcIdentity) -> bool { - unsafe { - if !self.quick_confirm(&self.strings.warning, &self.strings.kill_tree) { - return false; - } - - let prepared = match prepare_process_tree_termination(identity) { - Ok(prepared) => prepared, - Err(ProcessTreePrepareError::Root(error)) => { - self.show_failure_message(&self.strings.cant_kill, error); - return false; - } - Err(ProcessTreePrepareError::Tree(error)) => { - self.show_failure_message(&self.strings.kill_tree_fail_body, error); - return false; - } - }; - let outcome = terminate_prepared_process_tree(prepared); + pub(super) fn kill_process_tree(&mut self, identity: ProcIdentity) -> bool { + if !self.quick_confirm(&self.strings.warning, &self.strings.kill_tree) { + return false; + } - if outcome.any_completed { - self.paused = false; - self.refresh_processes(); + let prepared = match prepare_process_tree_termination(identity) { + Ok(prepared) => prepared, + Err(ProcessTreePrepareError::Root(error)) => { + self.show_failure_message(&self.strings.cant_kill, error); + return false; } - - if outcome.root_error != 0 && !outcome.any_success() { - self.show_failure_message(&self.strings.cant_kill, outcome.root_error); + Err(ProcessTreePrepareError::Tree(error)) => { + self.show_failure_message(&self.strings.kill_tree_fail_body, error); return false; } + }; + let outcome = terminate_prepared_process_tree(prepared); + + if outcome.any_completed { + self.paused = false; + self.refresh_processes(); + } - if outcome.any_failure() { - let body_wide = to_wide_null(&self.strings.kill_tree_fail_body); - let title_wide = to_wide_null(&self.strings.kill_tree_fail); + if outcome.root_error != 0 && !outcome.any_success() { + self.show_failure_message(&self.strings.cant_kill, outcome.root_error); + return false; + } + + if outcome.any_failure() { + let body_wide = to_wide_null(&self.strings.kill_tree_fail_body); + let title_wide = to_wide_null(&self.strings.kill_tree_fail); + // SAFETY: the page HWND is borrowed and both terminated buffers outlive this + // synchronous message box call. + unsafe { MessageBoxW( self.hwnd_page, body_wide.as_ptr(), title_wide.as_ptr(), MB_OK | MB_ICONEXCLAMATION, ); - return false; } - - outcome.completed_without_failure() + return false; } + + outcome.completed_without_failure() } // 以 AeDebug 注册表配置的调试器启动并附加到目标进程。命令行传 -p 。 - pub(super) unsafe fn attach_debugger(&mut self, identity: ProcIdentity) -> bool { - unsafe { - let Some(debugger_path) = self.debugger_path.as_ref() else { - let error = match self.debugger_error { - Some(error) => error, - None => ERROR_FILE_NOT_FOUND, - }; - self.show_failure_message(&self.strings.cant_debug, error); - return false; + pub(super) fn attach_debugger(&mut self, identity: ProcIdentity) -> bool { + let Some(debugger_path) = self.debugger_path.as_ref() else { + let error = match self.debugger_error { + Some(error) => error, + None => ERROR_FILE_NOT_FOUND, }; + self.show_failure_message(&self.strings.cant_debug, error); + return false; + }; - if !self.quick_confirm(&self.strings.warning, &self.strings.debug) { - return false; - } + if !self.quick_confirm(&self.strings.warning, &self.strings.debug) { + return false; + } - let target_handle = - match open_process_for_identity(identity, PROCESS_QUERY_LIMITED_INFORMATION) { - Ok(handle) => handle, - Err(error) => { - self.show_failure_message(&self.strings.cant_debug, error); - return false; - } - }; - - let pid = identity.pid; - let command_line = format!("{} -p {pid}", quote_command_line_arg(debugger_path)); - let mut command_line_wide = to_wide_null(&command_line); - let application_name = to_wide_null(debugger_path); - let startup_info = STARTUPINFOW { - cb: size_of::() as u32, - ..zeroed() + let target_handle = + match open_process_for_identity(identity, PROCESS_QUERY_LIMITED_INFORMATION) { + Ok(handle) => handle, + Err(error) => { + self.show_failure_message(&self.strings.cant_debug, error); + return false; + } }; - let mut process_info = zeroed::(); - let created = CreateProcessW( + let pid = identity.pid; + let command_line = format!("{} -p {pid}", quote_command_line_arg(debugger_path)); + let mut command_line_wide = to_wide_null(&command_line); + let application_name = to_wide_null(debugger_path); + let startup_info = STARTUPINFOW { + cb: size_of::() as u32, + ..unsafe { zeroed() } + }; + let mut process_info = unsafe { zeroed::() }; + + // SAFETY: the terminated application name, mutable command line, and initialized + // input/output structs all remain live for this synchronous call. + let created = unsafe { + CreateProcessW( application_name.as_ptr(), command_line_wide.as_mut_ptr(), null_mut(), @@ -319,60 +325,69 @@ impl ProcessPageState { null(), &startup_info, &mut process_info, - ); - let create_error = windows_sys::Win32::Foundation::GetLastError(); - drop(target_handle); + ) + }; + // Capture last-error before dropping `target_handle`, whose destructor may change it. + let create_error = if created == 0 { + unsafe { GetLastError() } + } else { + 0 + }; + drop(target_handle); - if created == 0 { - self.show_failure_message(&self.strings.cant_debug, create_error); - false - } else { - match own_created_process_handles(process_info) { - Ok(_) => true, - Err(error) => { - self.show_failure_message(&self.strings.cant_debug, error); - false - } + if created == 0 { + self.show_failure_message(&self.strings.cant_debug, create_error); + false + } else { + // SAFETY: this branch is reached only after CreateProcessW succeeded, which returned + // two fresh handles whose ownership is transferred here. + match unsafe { own_created_process_handles(process_info) } { + Ok(_) => true, + Err(error) => { + self.show_failure_message(&self.strings.cant_debug, error); + false } } } } // 通过 explorer.exe /select 命令在资源管理器中定位进程的可执行文件。 - pub(super) unsafe fn open_file_location(&mut self, identity: ProcIdentity) -> bool { - unsafe { - let image_path = match query_process_image_path(identity) { - Ok(path) => path, - Err(error) => { - self.show_failure_message(&self.strings.cant_open_file_location, error); - return false; - } - }; - - if !Path::new(&image_path).exists() { - self.show_failure_message(&self.strings.cant_open_file_location, 2); + pub(super) fn open_file_location(&mut self, identity: ProcIdentity) -> bool { + let image_path = match query_process_image_path(identity) { + Ok(path) => path, + Err(error) => { + self.show_failure_message(&self.strings.cant_open_file_location, error); return false; } + }; - let windows_directory = match query_windows_directory() { - Ok(path) => path, - Err(error) => { - self.show_failure_message(&self.strings.cant_open_file_location, error); - return false; - } - }; - let explorer_path = format!("{windows_directory}\\explorer.exe"); - let command_line = format!( - "{explorer_path} /select,{}", - quote_command_line_arg(&image_path) - ); - let mut command_line_wide = to_wide_null(&command_line); - let startup_info = STARTUPINFOW { - cb: size_of::() as u32, - ..zeroed() - }; - let mut process_info = zeroed::(); - let created = CreateProcessW( + if !Path::new(&image_path).exists() { + self.show_failure_message(&self.strings.cant_open_file_location, 2); + return false; + } + + let windows_directory = match query_windows_directory() { + Ok(path) => path, + Err(error) => { + self.show_failure_message(&self.strings.cant_open_file_location, error); + return false; + } + }; + let explorer_path = format!("{windows_directory}\\explorer.exe"); + let command_line = format!( + "{explorer_path} /select,{}", + quote_command_line_arg(&image_path) + ); + let mut command_line_wide = to_wide_null(&command_line); + let startup_info = STARTUPINFOW { + cb: size_of::() as u32, + ..unsafe { zeroed() } + }; + let mut process_info = unsafe { zeroed::() }; + // SAFETY: the mutable command line and initialized input/output structs remain live for + // the synchronous call; successful returned handles are adopted below. + let created = unsafe { + CreateProcessW( null(), command_line_wide.as_mut_ptr(), null_mut(), @@ -383,163 +398,158 @@ impl ProcessPageState { null(), &startup_info, &mut process_info, - ); - if created == 0 { - self.show_failure_message( - &self.strings.cant_open_file_location, - windows_sys::Win32::Foundation::GetLastError(), - ); - return false; - } + ) + }; + if created == 0 { + let error = unsafe { GetLastError() }; + self.show_failure_message(&self.strings.cant_open_file_location, error); + return false; + } - match own_created_process_handles(process_info) { - Ok(_) => true, - Err(error) => { - self.show_failure_message(&self.strings.cant_open_file_location, error); - false - } + // SAFETY: successful CreateProcessW returned fresh process/thread handles and this call + // is their first and only ownership transfer. + match unsafe { own_created_process_handles(process_info) } { + Ok(_) => true, + Err(error) => { + self.show_failure_message(&self.strings.cant_open_file_location, error); + false } } } // 通过 SetPriorityClass 修改进程优先级类。先弹确认框，操作成功后刷新列表。 - pub(super) unsafe fn set_priority( + pub(super) fn set_priority( &mut self, identity: ProcIdentity, priority: ProcPriority, ) -> bool { - unsafe { - let priority_class = match priority { - ProcPriority::Low => IDLE_PRIORITY_CLASS, - ProcPriority::BelowNormal => BELOW_NORMAL_PRIORITY_CLASS, - ProcPriority::Normal => NORMAL_PRIORITY_CLASS, - ProcPriority::AboveNormal => ABOVE_NORMAL_PRIORITY_CLASS, - ProcPriority::High => HIGH_PRIORITY_CLASS, - ProcPriority::Realtime => REALTIME_PRIORITY_CLASS, - }; - - if !self.quick_confirm(&self.strings.warning, &self.strings.prichange) { - return false; - } - - let handle = match open_process_for_identity( - identity, - PROCESS_SET_INFORMATION | PROCESS_QUERY_LIMITED_INFORMATION, - ) { - Ok(handle) => handle, - Err(error) => { - self.show_failure_message(&self.strings.cant_change_priority, error); - return false; - } - }; + let priority_class = match priority { + ProcPriority::Low => IDLE_PRIORITY_CLASS, + ProcPriority::BelowNormal => BELOW_NORMAL_PRIORITY_CLASS, + ProcPriority::Normal => NORMAL_PRIORITY_CLASS, + ProcPriority::AboveNormal => ABOVE_NORMAL_PRIORITY_CLASS, + ProcPriority::High => HIGH_PRIORITY_CLASS, + ProcPriority::Realtime => REALTIME_PRIORITY_CLASS, + }; - let result = SetPriorityClass(handle.as_raw(), priority_class); - let error = windows_sys::Win32::Foundation::GetLastError(); + if !self.quick_confirm(&self.strings.warning, &self.strings.prichange) { + return false; + } - if result == 0 { + let handle = match open_process_for_identity( + identity, + PROCESS_SET_INFORMATION | PROCESS_QUERY_LIMITED_INFORMATION, + ) { + Ok(handle) => handle, + Err(error) => { self.show_failure_message(&self.strings.cant_change_priority, error); - false - } else { - self.paused = false; - self.refresh_processes(); - true + return false; } + }; + + // SAFETY: the identity-validated handle has PROCESS_SET_INFORMATION access. + if unsafe { SetPriorityClass(handle.as_raw(), priority_class) } == 0 { + let error = unsafe { GetLastError() }; + self.show_failure_message(&self.strings.cant_change_priority, error); + false + } else { + self.paused = false; + self.refresh_processes(); + true } } // Single-group processes retain the classic hard-affinity API. Multi-group systems use CPU // Set IDs, whose group-qualified identities do not collapse at the 64-processor boundary. - pub(super) unsafe fn set_affinity(&mut self, identity: ProcIdentity) -> bool { - unsafe { - let handle = match open_process_for_identity( - identity, - PROCESS_QUERY_INFORMATION - | PROCESS_QUERY_LIMITED_INFORMATION - | PROCESS_SET_INFORMATION - | PROCESS_SET_LIMITED_INFORMATION, - ) { - Ok(handle) => handle, - Err(error) => { - self.show_failure_message(&self.strings.cant_set_affinity, error); - return false; - } - }; + pub(super) fn set_affinity(&mut self, identity: ProcIdentity) -> bool { + let handle = match open_process_for_identity( + identity, + PROCESS_QUERY_INFORMATION + | PROCESS_QUERY_LIMITED_INFORMATION + | PROCESS_SET_INFORMATION + | PROCESS_SET_LIMITED_INFORMATION, + ) { + Ok(handle) => handle, + Err(error) => { + self.show_failure_message(&self.strings.cant_set_affinity, error); + return false; + } + }; - let topology = match CpuSetTopology::query(handle.as_raw()) { - Ok(topology) => topology, - Err(error) => { - self.show_failure_message(&self.strings.cant_set_affinity, error.win32_code()); - return false; - } - }; - let original_default_ids = match query_process_default_cpu_sets(handle.as_raw()) { - Ok(ids) => ids, - Err(error) => { - self.show_failure_message(&self.strings.cant_set_affinity, error.win32_code()); - return false; - } - }; - let original_process_groups = match query_process_groups(handle.as_raw()) { - Ok(groups) => groups, - Err(error) => { - self.show_failure_message(&self.strings.cant_set_affinity, error); - return false; - } - }; - let selected_masks = match initial_affinity_masks( - handle.as_raw(), - &topology, - &original_default_ids, - &original_process_groups, - ) { - Ok(masks) => masks, - Err(error) => { - self.show_failure_message(&self.strings.cant_set_affinity, error); - return false; - } - }; - let selected_group_index = selected_masks - .iter() - .position(|mask| *mask != 0) - .unwrap_or(0); - let mut context = AffinityDialogContext { - page: self as *mut ProcessPageState, - topology, - selected_masks, - selected_group_index, - original_default_ids, - }; + let topology = match CpuSetTopology::query(handle.as_raw()) { + Ok(topology) => topology, + Err(error) => { + self.show_failure_message(&self.strings.cant_set_affinity, error.win32_code()); + return false; + } + }; + let original_default_ids = match query_process_default_cpu_sets(handle.as_raw()) { + Ok(ids) => ids, + Err(error) => { + self.show_failure_message(&self.strings.cant_set_affinity, error.win32_code()); + return false; + } + }; + let original_process_groups = match query_process_groups(handle.as_raw()) { + Ok(groups) => groups, + Err(error) => { + self.show_failure_message(&self.strings.cant_set_affinity, error); + return false; + } + }; + let selected_masks = match initial_affinity_masks( + handle.as_raw(), + &topology, + &original_default_ids, + &original_process_groups, + ) { + Ok(masks) => masks, + Err(error) => { + self.show_failure_message(&self.strings.cant_set_affinity, error); + return false; + } + }; + let selected_group_index = selected_masks + .iter() + .position(|mask| *mask != 0) + .unwrap_or(0); + let mut context = AffinityDialogContext { + page: self as *mut ProcessPageState, + topology, + selected_masks, + selected_group_index, + original_default_ids, + }; - match dialog_box( - self.hinstance, - IDD_AFFINITY, - self.hwnd_page, - Some(affinity_dialog_proc), - &mut context as *mut AffinityDialogContext as LPARAM, - ) { - Ok(result) if result == IDOK as isize => { - match apply_affinity_selection(handle.as_raw(), identity.pid, &context) { - Ok(()) => { - self.refresh_processes(); - true - } - Err(error) => { - self.show_failure_message(&self.strings.cant_set_affinity, error); - false - } + match dialog_box( + self.hinstance, + IDD_AFFINITY, + self.hwnd_page, + Some(affinity_dialog_proc), + &mut context as *mut AffinityDialogContext as LPARAM, + ) { + Ok(result) if result == IDOK as isize => { + match apply_affinity_selection(handle.as_raw(), identity.pid, &context) { + Ok(()) => { + self.refresh_processes(); + true + } + Err(error) => { + self.show_failure_message(&self.strings.cant_set_affinity, error); + false } - } - Ok(_) => false, - Err(error) => { - self.show_failure_message(&self.strings.cant_set_affinity, error); - false } } + Ok(_) => false, + Err(error) => { + self.show_failure_message(&self.strings.cant_set_affinity, error); + false + } } } } -unsafe fn initial_affinity_masks( +fn initial_affinity_masks( process: HANDLE, topology: &CpuSetTopology, default_ids: &[u32], @@ -580,7 +590,7 @@ unsafe fn initial_affinity_masks( Ok(masks) } -unsafe fn apply_affinity_selection( +fn apply_affinity_selection( process: HANDLE, process_id: u32, context: &AffinityDialogContext, @@ -594,18 +604,16 @@ unsafe fn apply_affinity_selection( } if context.topology.groups().len() == 1 { - unsafe { - set_process_default_cpu_sets(process, &[])?; - if SetProcessAffinityMask(process, context.selected_masks[0]) == 0 { - let error = nonzero_last_error(); - let _ = set_process_default_cpu_sets(process, &context.original_default_ids); - return Err(error); - } + set_process_default_cpu_sets(process, &[])?; + if unsafe { SetProcessAffinityMask(process, context.selected_masks[0]) } == 0 { + let error = nonzero_last_error(); + let _ = set_process_default_cpu_sets(process, &context.original_default_ids); + return Err(error); } return Ok(()); } - unsafe { set_process_default_cpu_sets(process, &selected_ids)? }; + set_process_default_cpu_sets(process, &selected_ids)?; let selected_groups = context .topology .groups() @@ -619,14 +627,14 @@ unsafe fn apply_affinity_selection( // conflicting CPU Set assignment. Apply a group-qualified hard mask to every existing thread // so an old per-thread affinity cannot keep using a CPU the user just deselected. The process // default CPU Sets above cover threads created after the verified thread snapshot. - if let Err(error) = unsafe { apply_thread_group_affinities(process_id, &selected_groups) } { - let _ = unsafe { set_process_default_cpu_sets(process, &context.original_default_ids) }; + if let Err(error) = apply_thread_group_affinities(process_id, &selected_groups) { + let _ = set_process_default_cpu_sets(process, &context.original_default_ids); return Err(error); } Ok(()) } -unsafe fn set_process_default_cpu_sets(process: HANDLE, ids: &[u32]) -> Result<(), u32> { +fn set_process_default_cpu_sets(process: HANDLE, ids: &[u32]) -> Result<(), u32> { let (pointer, count) = if ids.is_empty() { (null(), 0) } else { @@ -647,7 +655,7 @@ struct ChangedThreadAffinity { previous: GROUP_AFFINITY, } -unsafe fn apply_thread_group_affinities( +fn apply_thread_group_affinities( process_id: u32, selected_groups: &[(u16, usize)], ) -> Result<(), u32> { @@ -659,7 +667,7 @@ unsafe fn apply_thread_group_affinities( let mut seen = HashSet::::new(); let mut assignment_index = 0usize; let result = (|| -> Result<(), u32> { - let thread_ids = unsafe { enumerate_process_threads(process_id)? }; + let thread_ids = enumerate_process_threads(process_id)?; for thread_id in thread_ids { let raw_thread = unsafe { OpenThread( @@ -668,7 +676,9 @@ unsafe fn apply_thread_group_affinities( thread_id, ) }; - let Some(thread) = OwnedHandle::new(raw_thread) else { + // SAFETY: a successful OpenThread call returns a newly opened handle owned by this + // scope and released with CloseHandle; null is mapped to None. + let Some(thread) = (unsafe { OwnedHandle::from_raw(raw_thread) }) else { let error = nonzero_last_error(); if matches!(error, ERROR_INVALID_HANDLE | ERROR_INVALID_PARAMETER) { continue; @@ -704,9 +714,7 @@ unsafe fn apply_thread_group_affinities( if changed.is_empty() { return Err(ERROR_NOT_SUPPORTED); } - unsafe { - verify_racing_thread_affinities(process_id, &seen, selected_groups)?; - } + verify_racing_thread_affinities(process_id, &seen, selected_groups)?; Ok(()) })(); @@ -724,17 +732,19 @@ unsafe fn apply_thread_group_affinities( result } -unsafe fn verify_racing_thread_affinities( +fn verify_racing_thread_affinities( process_id: u32, changed_thread_ids: &HashSet, selected_groups: &[(u16, usize)], ) -> Result<(), u32> { - for thread_id in unsafe { enumerate_process_threads(process_id)? } { + for thread_id in enumerate_process_threads(process_id)? { if changed_thread_ids.contains(&thread_id) { continue; } let raw_thread = unsafe { OpenThread(THREAD_QUERY_LIMITED_INFORMATION, 0, thread_id) }; - let Some(thread) = OwnedHandle::new(raw_thread) else { + // SAFETY: a successful OpenThread call returns a newly opened handle owned by this scope + // and released with CloseHandle; null is mapped to None. + let Some(thread) = (unsafe { OwnedHandle::from_raw(raw_thread) }) else { let error = nonzero_last_error(); if matches!(error, ERROR_INVALID_HANDLE | ERROR_INVALID_PARAMETER) { continue; @@ -778,9 +788,11 @@ pub(super) fn affinity_target_for_thread( .copied() } -unsafe fn enumerate_process_threads(process_id: u32) -> Result, u32> { +fn enumerate_process_threads(process_id: u32) -> Result, u32> { let raw_snapshot = unsafe { CreateToolhelp32Snapshot(TH32CS_SNAPTHREAD, 0) }; - let Some(snapshot) = OwnedHandle::new(raw_snapshot) else { + // SAFETY: CreateToolhelp32Snapshot returns either INVALID_HANDLE_VALUE or a fresh snapshot + // handle owned by this scope and released with CloseHandle. + let Some(snapshot) = (unsafe { OwnedHandle::from_raw(raw_snapshot) }) else { return Err(nonzero_last_error()); }; let mut entry = unsafe { zeroed::() }; @@ -813,7 +825,7 @@ unsafe fn enumerate_process_threads(process_id: u32) -> Result, u32> { Ok(thread_ids) } -unsafe fn query_process_groups(process: HANDLE) -> Result, u32> { +fn query_process_groups(process: HANDLE) -> Result, u32> { let mut required = 0u16; if unsafe { GetProcessGroupAffinity(process, &mut required, null_mut()) } != 0 { return Err(ERROR_INVALID_DATA); @@ -931,7 +943,7 @@ unsafe extern "system" fn affinity_dialog_proc( } } -unsafe fn initialize_affinity_group_selector(hwnd: HWND, context: &mut AffinityDialogContext) { +fn initialize_affinity_group_selector(hwnd: HWND, context: &mut AffinityDialogContext) { let selector = unsafe { GetDlgItem(hwnd, IDC_AFFINITY_GROUP_SELECTOR) }; unsafe { SendMessageW(selector, CB_RESETCONTENT, 0, 0) }; for group in context.topology.groups() { @@ -954,7 +966,7 @@ unsafe fn initialize_affinity_group_selector(hwnd: HWND, context: &mut AffinityD } } -unsafe fn render_affinity_group(hwnd: HWND, context: &AffinityDialogContext) { +fn render_affinity_group(hwnd: HWND, context: &AffinityDialogContext) { let group = &context.topology.groups()[context.selected_group_index]; let selected_mask = context.selected_masks[context.selected_group_index]; let multiple_groups = context.topology.groups().len() > 1; @@ -984,7 +996,7 @@ unsafe fn render_affinity_group(hwnd: HWND, context: &AffinityDialogContext) { } } -unsafe fn save_affinity_group(hwnd: HWND, context: &mut AffinityDialogContext) { +fn save_affinity_group(hwnd: HWND, context: &mut AffinityDialogContext) { let group = &context.topology.groups()[context.selected_group_index]; let mut selected_mask = 0usize; for cpu_index in 0..=MAX_AFFINITY_CPU { @@ -1005,79 +1017,93 @@ pub(super) fn affinity_cpu_mask(cpu_index: i32) -> usize { .and_then(|shift| 1usize.checked_shl(shift)) .unwrap_or(0) } -pub(super) unsafe fn load_debugger_path() -> Result, u32> { - unsafe { - // 进程页的“调试”命令依赖 AeDebug 注册表配置。 - // 这里只提取真正的可执行文件路径，过滤掉旧式 drwtsn32 之类的无效值。 - let mut key: HKEY = null_mut(); - let key_name = to_wide_null("SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\AeDebug"); - let value_name = to_wide_null("Debugger"); - let open_status = - RegOpenKeyExW(HKEY_LOCAL_MACHINE, key_name.as_ptr(), 0, KEY_READ, &mut key); - if open_status != 0 { - return if open_status == ERROR_FILE_NOT_FOUND || open_status == ERROR_PATH_NOT_FOUND { - Ok(None) - } else { - Err(open_status) - }; - } - let mut value_size = 0u32; - let size_status = RegQueryValueExW( +pub(super) fn load_debugger_path() -> Result, u32> { + // 进程页的“调试”命令依赖 AeDebug 注册表配置。 + // 这里只提取真正的可执行文件路径，过滤掉旧式 drwtsn32 之类的无效值。 + let mut key: HKEY = null_mut(); + let key_name = to_wide_null("SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\AeDebug"); + let value_name = to_wide_null("Debugger"); + // SAFETY: both input strings are terminated and `key` is a valid output location. + let open_status = unsafe { + RegOpenKeyExW(HKEY_LOCAL_MACHINE, key_name.as_ptr(), 0, KEY_READ, &mut key) + }; + if open_status != 0 { + return if open_status == ERROR_FILE_NOT_FOUND || open_status == ERROR_PATH_NOT_FOUND { + Ok(None) + } else { + Err(open_status) + }; + } + + let mut value_size = 0u32; + // SAFETY: `key` was opened successfully; this size query uses no data buffer and writes only + // to `value_size`. + let size_status = unsafe { + RegQueryValueExW( key, value_name.as_ptr(), null_mut(), null_mut(), null_mut(), &mut value_size, - ); - if size_status != 0 || value_size < 2 { - let close_status = RegCloseKey(key); - if close_status != 0 { - return Err(close_status); - } - return if size_status == ERROR_FILE_NOT_FOUND { - Ok(None) - } else if size_status != 0 { - Err(size_status) - } else { - Err(ERROR_INVALID_DATA) - }; + ) + }; + if size_status != 0 || value_size < 2 { + let close_status = unsafe { RegCloseKey(key) }; + if close_status != 0 { + return Err(close_status); } + return if size_status == ERROR_FILE_NOT_FOUND { + Ok(None) + } else if size_status != 0 { + Err(size_status) + } else { + Err(ERROR_INVALID_DATA) + }; + } - let mut buffer = vec![0u16; (value_size as usize / size_of::()).max(2)]; - let mut value_type = 0u32; - let status = RegQueryValueExW( + let mut buffer = vec![ + 0u16; + (value_size as usize) + .div_ceil(size_of::()) + .max(2) + ]; + let mut value_type = 0u32; + // SAFETY: `buffer` is writable for the byte count returned by the size query and all output + // pointers reference live local variables. + let status = unsafe { + RegQueryValueExW( key, value_name.as_ptr(), null_mut(), &mut value_type, buffer.as_mut_ptr() as *mut u8, &mut value_size, - ); - let close_status = RegCloseKey(key); - if close_status != 0 { - return Err(close_status); - } - - if status != 0 || value_size < 2 || !(value_type == REG_SZ || value_type == REG_EXPAND_SZ) { - return Err(if status != 0 { - status - } else { - ERROR_INVALID_DATA - }); - } + ) + }; + let close_status = unsafe { RegCloseKey(key) }; + if close_status != 0 { + return Err(close_status); + } - let length = buffer - .iter() - .position(|value| *value == 0) - .unwrap_or(buffer.len()); - let raw = String::from_utf16_lossy(&buffer[..length]); - let Some(executable) = normalize_debugger_command(&raw, value_type)? else { - return Ok(None); - }; - Ok(Path::new(&executable).is_file().then_some(executable)) + if status != 0 || value_size < 2 || !(value_type == REG_SZ || value_type == REG_EXPAND_SZ) { + return Err(if status != 0 { + status + } else { + ERROR_INVALID_DATA + }); } + + let length = buffer + .iter() + .position(|value| *value == 0) + .unwrap_or(buffer.len()); + let raw_command = String::from_utf16_lossy(&buffer[..length]); + let Some(executable) = normalize_debugger_command(&raw_command, value_type)? else { + return Ok(None); + }; + Ok(Path::new(&executable).is_file().then_some(executable)) } // 引用命令行参数。只在包含空格、制表符或引号时加引号，并正确处理反斜杠转义。 @@ -1276,56 +1302,56 @@ pub(super) fn terminate_prepared_process_tree( fn collect_process_tree_termination_order( root_identity: ProcIdentity, ) -> Result, u32> { - unsafe { - let raw_snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0); - let Some(snapshot) = OwnedHandle::new(raw_snapshot) else { - let error = windows_sys::Win32::Foundation::GetLastError(); - return Err(if error == 0 { ERROR_GEN_FAILURE } else { error }); - }; + let raw_snapshot = unsafe { CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0) }; + // SAFETY: a successful CreateToolhelp32Snapshot call returns a fresh snapshot handle owned + // by this scope and released with CloseHandle. + let Some(snapshot) = (unsafe { OwnedHandle::from_raw(raw_snapshot) }) else { + let error = unsafe { GetLastError() }; + return Err(if error == 0 { ERROR_GEN_FAILURE } else { error }); + }; - let mut snapshot_time = zeroed::(); - GetSystemTimeAsFileTime(&mut snapshot_time); - let snapshot_time_100ns = filetime_to_u64(snapshot_time); + let mut snapshot_time = unsafe { zeroed::() }; + unsafe { GetSystemTimeAsFileTime(&mut snapshot_time) }; + let snapshot_time_100ns = filetime_to_u64(snapshot_time); - let mut child_map = HashMap::>::new(); - let mut process_entry = zeroed::(); - process_entry.dwSize = size_of::() as u32; + let mut child_map = HashMap::>::new(); + let mut process_entry = unsafe { zeroed::() }; + process_entry.dwSize = size_of::() as u32; - if Process32FirstW(snapshot.as_raw(), &mut process_entry) == 0 { - let error = windows_sys::Win32::Foundation::GetLastError(); - return Err(if error == 0 { ERROR_GEN_FAILURE } else { error }); - } + if unsafe { Process32FirstW(snapshot.as_raw(), &mut process_entry) } == 0 { + let error = unsafe { GetLastError() }; + return Err(if error == 0 { ERROR_GEN_FAILURE } else { error }); + } - loop { - child_map - .entry(process_entry.th32ParentProcessID) - .or_default() - .push(process_entry.th32ProcessID); - if Process32NextW(snapshot.as_raw(), &mut process_entry) == 0 { - let error = windows_sys::Win32::Foundation::GetLastError(); - if error != ERROR_NO_MORE_FILES { - return Err(if error == 0 { ERROR_GEN_FAILURE } else { error }); - } - break; + loop { + child_map + .entry(process_entry.th32ParentProcessID) + .or_default() + .push(process_entry.th32ProcessID); + if unsafe { Process32NextW(snapshot.as_raw(), &mut process_entry) } == 0 { + let error = unsafe { GetLastError() }; + if error != ERROR_NO_MORE_FILES { + return Err(if error == 0 { ERROR_GEN_FAILURE } else { error }); } + break; } - - // The snapshot is keyed only by PID. Revalidate the root after enumeration so a root - // that exited and had its PID reused cannot lend the replacement's children to this tree. - let current_root = query_process_identity_for_pid(root_identity.pid)?; - validate_snapshot_root_identity(root_identity, current_root)?; - - let mut identities = Vec::new(); - let mut visited = HashSet::new(); - collect_verified_process_tree_children( - root_identity, - snapshot_time_100ns, - &child_map, - &mut visited, - &mut identities, - )?; - Ok(identities) } + + // The snapshot is keyed only by PID. Revalidate the root after enumeration so a root that + // exited and had its PID reused cannot lend the replacement's children to this tree. + let current_root = query_process_identity_for_pid(root_identity.pid)?; + validate_snapshot_root_identity(root_identity, current_root)?; + + let mut identities = Vec::new(); + let mut visited = HashSet::new(); + collect_verified_process_tree_children( + root_identity, + snapshot_time_100ns, + &child_map, + &mut visited, + &mut identities, + )?; + Ok(identities) } const fn filetime_to_u64(filetime: FILETIME) -> u64 { @@ -1344,44 +1370,42 @@ pub(super) fn validate_snapshot_root_identity( } // 后序遍历进程树；每条边都用创建时间验证，避免父 PID 被复用后串入旧子树。 -unsafe fn collect_verified_process_tree_children( +fn collect_verified_process_tree_children( parent: ProcIdentity, snapshot_time_100ns: u64, child_map: &HashMap>, visited: &mut HashSet, order: &mut Vec, ) -> Result<(), u32> { - unsafe { - if !visited.insert(parent.pid) { - return Ok(()); - } + if !visited.insert(parent.pid) { + return Ok(()); + } - if let Some(children) = child_map.get(&parent.pid) { - for &child_pid in children { - if visited.contains(&child_pid) { - continue; - } - let child = match classify_descendant_process_result( - query_process_identity_for_pid(child_pid), - |child| is_valid_process_tree_edge(parent, *child, snapshot_time_100ns), - ) { - DescendantProcessOutcome::Verified(child) => child, - DescendantProcessOutcome::GoneOrReused => continue, - DescendantProcessOutcome::Fatal(error) => return Err(error), - }; - collect_verified_process_tree_children( - child, - snapshot_time_100ns, - child_map, - visited, - order, - )?; + if let Some(children) = child_map.get(&parent.pid) { + for &child_pid in children { + if visited.contains(&child_pid) { + continue; } + let child = match classify_descendant_process_result( + query_process_identity_for_pid(child_pid), + |child| is_valid_process_tree_edge(parent, *child, snapshot_time_100ns), + ) { + DescendantProcessOutcome::Verified(child) => child, + DescendantProcessOutcome::GoneOrReused => continue, + DescendantProcessOutcome::Fatal(error) => return Err(error), + }; + collect_verified_process_tree_children( + child, + snapshot_time_100ns, + child_map, + visited, + order, + )?; } - - order.push(parent); - Ok(()) } + + order.push(parent); + Ok(()) } pub(super) fn is_valid_process_tree_edge( @@ -1395,48 +1419,59 @@ pub(super) fn is_valid_process_tree_edge( && child.creation_time_100ns <= snapshot_time_100ns } -fn own_created_process_handles( +/// Adopts the two handles returned by a successful `CreateProcessW` call. +/// +/// # Safety +/// +/// `process_info.hProcess` and `process_info.hThread` must be fresh, uniquely owned handles from +/// the same successful `CreateProcessW` call and must not have been closed or adopted elsewhere. +unsafe fn own_created_process_handles( process_info: PROCESS_INFORMATION, ) -> Result<(OwnedHandle, OwnedHandle), u32> { - let process = OwnedHandle::new(process_info.hProcess); - let thread = OwnedHandle::new(process_info.hThread); + let process = unsafe { OwnedHandle::from_raw(process_info.hProcess) }; + let thread = unsafe { OwnedHandle::from_raw(process_info.hThread) }; match (process, thread) { (Some(process), Some(thread)) => Ok((process, thread)), _ => Err(ERROR_INVALID_HANDLE), } } -unsafe fn query_process_image_path(identity: ProcIdentity) -> Result { - unsafe { - let handle = open_process_for_identity(identity, PROCESS_QUERY_LIMITED_INFORMATION)?; - - let mut capacity = 32768u32; - let mut buffer = vec![0u16; capacity as usize]; - let success = - QueryFullProcessImageNameW(handle.as_raw(), 0, buffer.as_mut_ptr(), &mut capacity); - let error = windows_sys::Win32::Foundation::GetLastError(); - drop(handle); +fn query_process_image_path(identity: ProcIdentity) -> Result { + let handle = open_process_for_identity(identity, PROCESS_QUERY_LIMITED_INFORMATION)?; - if success == 0 { - return Err(error); - } + let mut capacity = 32768u32; + let mut buffer = vec![0u16; capacity as usize]; + // SAFETY: the identity-validated handle is live and `buffer` is writable for `capacity` + // UTF-16 code units. The API updates `capacity` to the initialized length. + let success = unsafe { + QueryFullProcessImageNameW(handle.as_raw(), 0, buffer.as_mut_ptr(), &mut capacity) + }; + let error = if success == 0 { + unsafe { GetLastError() } + } else { + 0 + }; + drop(handle); - Ok(String::from_utf16_lossy(&buffer[..capacity as usize])) + if success == 0 { + return Err(error); } + + Ok(String::from_utf16_lossy(&buffer[..capacity as usize])) } -unsafe fn query_windows_directory() -> Result { - unsafe { - let mut buffer = vec![0u16; 260]; - loop { - let length = GetWindowsDirectoryW(buffer.as_mut_ptr(), buffer.len() as u32) as usize; - if length == 0 { - return Err(windows_sys::Win32::Foundation::GetLastError()); - } - if length < buffer.len() { - return Ok(String::from_utf16_lossy(&buffer[..length])); - } - buffer.resize(length.saturating_add(1), 0); +fn query_windows_directory() -> Result { + let mut buffer = vec![0u16; 260]; + loop { + // SAFETY: `buffer` is writable for the advertised number of UTF-16 code units. + let length = unsafe { GetWindowsDirectoryW(buffer.as_mut_ptr(), buffer.len() as u32) } + as usize; + if length == 0 { + return Err(unsafe { GetLastError() }); + } + if length < buffer.len() { + return Ok(String::from_utf16_lossy(&buffer[..length])); } + buffer.resize(length.saturating_add(1), 0); } } diff --git a/src/pages/processes/mod.rs b/src/pages/processes/mod.rs index c9d6ed7..6e2f209 100644 --- a/src/pages/processes/mod.rs +++ b/src/pages/processes/mod.rs @@ -141,7 +141,7 @@ impl DirtyRowRange { self.end = self.end.max(index); } - unsafe fn redraw_visible(self, list_hwnd: HWND, item_count: usize) { + fn redraw_visible(self, list_hwnd: HWND, item_count: usize) { unsafe { let Some(start) = self.start else { return; @@ -374,11 +374,11 @@ impl ProcessPageState { Self::default() } - pub unsafe fn no_title(&self) -> bool { + pub fn no_title(&self) -> bool { self.no_title } - pub unsafe fn initialize( + pub fn initialize( &mut self, hinstance: HINSTANCE, hwnd_page: HWND, @@ -420,55 +420,63 @@ impl ProcessPageState { } } - pub unsafe fn apply_options(&mut self, options: &Options, processor_count: usize) { - unsafe { - // 进程页的选项既影响行为，也影响列结构。 - // 当列配置发生变化时，直接重建列和数据比做局部修补更可靠。 - self.no_title = options.no_title(); - self.confirmations = options.confirmations(); - self.processor_count = processor_count.max(1); - - let desired_columns = columns_from_options(options); - if desired_columns != self.active_columns { - self.active_columns = desired_columns; - let visible_columns = DirtyColumns::from_columns(&self.active_columns); - for entry in &mut self.entries { - entry.rebuild_display_columns(&self.active_columns); - entry.dirty_columns = visible_columns; - } - self.setup_columns(options); + pub fn apply_options(&mut self, options: &Options, processor_count: usize) { + // 进程页的选项既影响行为，也影响列结构。 + // 当列配置发生变化时，直接重建列和数据比做局部修补更可靠。 + self.no_title = options.no_title(); + self.confirmations = options.confirmations(); + self.processor_count = processor_count.max(1); + + let desired_columns = columns_from_options(options); + if desired_columns != self.active_columns { + self.active_columns = desired_columns; + let visible_columns = DirtyColumns::from_columns(&self.active_columns); + for entry in &mut self.entries { + entry.rebuild_display_columns(&self.active_columns); + entry.dirty_columns = visible_columns; } + self.setup_columns(options); } } - pub unsafe fn timer_event(&mut self, options: &Options, force: bool) { - unsafe { - // 每一轮刷新都走“采样 -> 合并旧状态 -> 排序/重绘”这条统一链路。 - self.paused = options.update_speed == UpdateSpeed::Paused as i32; - if force || !self.paused { - self.refresh_processes(); - } + pub fn timer_event(&mut self, options: &Options, force: bool) { + // 每一轮刷新都走“采样 -> 合并旧状态 -> 排序/重绘”这条统一链路。 + self.paused = options.update_speed == UpdateSpeed::Paused as i32; + if force || !self.paused { + self.refresh_processes(); } } - pub unsafe fn deactivate(&mut self, options: &mut Options) { - unsafe { - if let Err(error) = self.save_column_layout(options) { - record_win32_error("process column layout persistence", error); - } + pub fn deactivate(&mut self, options: &mut Options) { + if let Err(error) = self.save_column_layout(options) { + record_win32_error("process column layout persistence", error); } } - pub unsafe fn destroy(&mut self) { + pub fn destroy(&mut self) { self.stop_worker_thread(); self.entries.clear(); self.displayed_identities.clear(); } + /// Handles a notification forwarded by the process page dialog procedure. + /// + /// # Safety + /// + /// `lparam` must be the live `WM_NOTIFY` payload for this page's ListView. The payload must + /// have the structure implied by its `NMHDR::code` and remain readable (and, for + /// `LVN_GETDISPINFOW`, writable) for the duration of this synchronous call. pub unsafe fn handle_notify(&mut self, lparam: LPARAM) -> isize { unsafe { // ListView 处于 owner-data 风格，因此文本、排序和选择同步都靠通知消息驱动。 - let notify_header = &*(lparam as *const NMHDR); + let Some(notify_header) = (lparam as *const NMHDR).as_ref() else { + return 0; + }; + if notify_header.idFrom != IDC_PROCLIST as usize + || notify_header.hwndFrom != self.list_hwnd() + { + return 0; + } match notify_header.code { code if code == LVN_GETDISPINFOW => { let display_info = &mut *(lparam as *mut NMLVDISPINFOW); @@ -509,54 +517,51 @@ impl ProcessPageState { } // 将命令 ID 分派到具体的进程操作（结束、调试、优先级、亲和性等）。 - pub unsafe fn handle_command(&mut self, command_id: u16, options: Option<&mut Options>) { - unsafe { - let Some(command) = ProcCommand::from_command_id(command_id, IDC_TERMINATE as u16) - else { - return; - }; + pub fn handle_command(&mut self, command_id: u16, options: Option<&mut Options>) { + let Some(command) = ProcCommand::from_command_id(command_id, IDC_TERMINATE as u16) else { + return; + }; - match command { - ProcCommand::PickColumns => { - if let Some(options) = options { - self.pick_columns(options); - } + match command { + ProcCommand::PickColumns => { + if let Some(options) = options { + self.pick_columns(options); } - ProcCommand::Terminate => { - if let Some(identity) = self.current_selected_identity() { - self.kill_process(identity); - } + } + ProcCommand::Terminate => { + if let Some(identity) = self.current_selected_identity() { + self.kill_process(identity); } - ProcCommand::TerminateTree => { - if let Some(identity) = self.current_selected_identity() { - self.kill_process_tree(identity); - } + } + ProcCommand::TerminateTree => { + if let Some(identity) = self.current_selected_identity() { + self.kill_process_tree(identity); } - ProcCommand::Debug => { - if let Some(identity) = self.current_selected_identity() { - self.attach_debugger(identity); - } + } + ProcCommand::Debug => { + if let Some(identity) = self.current_selected_identity() { + self.attach_debugger(identity); } - ProcCommand::OpenFileLocation => { - if let Some(identity) = self.current_selected_identity() { - self.open_file_location(identity); - } + } + ProcCommand::OpenFileLocation => { + if let Some(identity) = self.current_selected_identity() { + self.open_file_location(identity); } - ProcCommand::Affinity => { - if let Some(identity) = self.current_selected_identity() { - self.set_affinity(identity); - } + } + ProcCommand::Affinity => { + if let Some(identity) = self.current_selected_identity() { + self.set_affinity(identity); } - ProcCommand::SetPriority(priority) => { - if let Some(identity) = self.current_selected_identity() { - self.set_priority(identity, priority); - } + } + ProcCommand::SetPriority(priority) => { + if let Some(identity) = self.current_selected_identity() { + self.set_priority(identity, priority); } } } } - pub unsafe fn show_context_menu(&mut self, x: i32, y: i32) { + pub fn show_context_menu(&mut self, x: i32, y: i32) { unsafe { // 右键菜单会按当前选中进程和系统能力动态裁剪。 self.selected_identity = self.current_selected_identity(); @@ -597,7 +602,7 @@ impl ProcessPageState { } // 构造进程右键菜单，包含结束进程、调试、打开文件位置、优先级和亲和性子菜单。 - unsafe fn build_context_menu(&self, entry: &ProcEntry) -> Result { + fn build_context_menu(&self, entry: &ProcEntry) -> Result { let identity_verified = entry.identity.is_verified(); let mut priority_menu = PopupMenu::new()?; let checked_priority = match entry.priority_class { @@ -692,7 +697,7 @@ impl ProcessPageState { Ok(popup) } - pub unsafe fn size_page(&self) { + pub fn size_page(&self) { unsafe { // 进程页布局以“列表吃掉剩余空间，按钮贴右下角”为核心规则。 let mut parent_rect = zeroed::(); @@ -742,7 +747,7 @@ impl ProcessPageState { } // 从进程页跳转到指定 PID 的行并高亮选中。由任务页的“转到进程”命令触发。 - pub unsafe fn find_process(&mut self, identity: ProcIdentity) -> bool { + pub fn find_process(&mut self, identity: ProcIdentity) -> bool { unsafe { if !identity.is_verified() { return false; @@ -798,7 +803,7 @@ impl ProcessPageState { self.strings.priority_unknown = text(TextKey::Unknown).to_string(); } - unsafe fn update_ui_state(&self) { + fn update_ui_state(&self) { unsafe { // 当前实现里只有“结束进程”按钮依赖选择状态， // 但统一收口在这里，后续扩展其它按钮更容易。 @@ -812,14 +817,12 @@ impl ProcessPageState { } } - unsafe fn refresh_processes(&mut self) { - unsafe { - self.drain_worker_results(); - self.schedule_process_collection(); - } + fn refresh_processes(&mut self) { + self.drain_worker_results(); + self.schedule_process_collection(); } - unsafe fn schedule_process_collection(&mut self) { + fn schedule_process_collection(&mut self) { let Some(worker) = self.worker.as_mut() else { self.set_refresh_error(windows_sys::Win32::Foundation::ERROR_BROKEN_PIPE); return; @@ -832,7 +835,7 @@ impl ProcessPageState { } } - unsafe fn drain_worker_results(&mut self) { + fn drain_worker_results(&mut self) { let drain = match self.worker.as_mut() { Some(worker) => worker.drain(self.hwnd_page), None => return, @@ -840,9 +843,7 @@ impl ProcessPageState { for completion in drain.completions { crate::infrastructure::diagnostics::with_operation_id( completion.operation_id, - || unsafe { - self.apply_worker_completion(completion.value); - }, + || self.apply_worker_completion(completion.value), ); } if let Some(error) = drain.error { @@ -850,66 +851,62 @@ impl ProcessPageState { } } - unsafe fn apply_worker_completion(&mut self, completion: ProcWorkerResult) { - unsafe { - match completion { - Ok(snapshot) => { - self.last_refresh_error = None; - if self.last_row_error != snapshot.row_error - && let Some(error) = snapshot.row_error - { - record_win32_error("process row metadata", error); - } - self.last_row_error = snapshot.row_error; - self.apply_process_snapshot(snapshot.entries); - } - Err(error) => { - self.set_refresh_error(error); + fn apply_worker_completion(&mut self, completion: ProcWorkerResult) { + match completion { + Ok(snapshot) => { + self.last_refresh_error = None; + if self.last_row_error != snapshot.row_error + && let Some(error) = snapshot.row_error + { + record_win32_error("process row metadata", error); } + self.last_row_error = snapshot.row_error; + self.apply_process_snapshot(snapshot.entries); + } + Err(error) => { + self.set_refresh_error(error); } } } - unsafe fn apply_process_snapshot(&mut self, entries: Vec) { - unsafe { - let previous_selection = self.current_selected_identity().or(self.selected_identity); - let current_pass = self.pass_count; - let visible_columns = DirtyColumns::from_columns(&self.active_columns); - let mut sort_dirty = false; - let mut existing_by_identity = HashMap::with_capacity(self.entries.len()); - for (index, entry) in self.entries.iter_mut().enumerate() { - existing_by_identity.insert(entry.identity, index); - } + fn apply_process_snapshot(&mut self, entries: Vec) { + let previous_selection = self.current_selected_identity().or(self.selected_identity); + let current_pass = self.pass_count; + let visible_columns = DirtyColumns::from_columns(&self.active_columns); + let mut sort_dirty = false; + let mut existing_by_identity = HashMap::with_capacity(self.entries.len()); + for (index, entry) in self.entries.iter_mut().enumerate() { + existing_by_identity.insert(entry.identity, index); + } - for snapshot in entries { - if let Some(&index) = existing_by_identity.get(&snapshot.identity) { - let existing = &mut self.entries[index]; - let changed = - update_process_entry(existing, &snapshot, current_pass, visible_columns); - sort_dirty |= changed.contains(self.sort_column); - } else { - self.entries.push(snapshot.with_pass_count( - current_pass, - &self.active_columns, - visible_columns, - )); - sort_dirty = true; - } + for snapshot in entries { + if let Some(&index) = existing_by_identity.get(&snapshot.identity) { + let existing = &mut self.entries[index]; + let changed = + update_process_entry(existing, &snapshot, current_pass, visible_columns); + sort_dirty |= changed.contains(self.sort_column); + } else { + self.entries.push(snapshot.with_pass_count( + current_pass, + &self.active_columns, + visible_columns, + )); + sort_dirty = true; } + } - sort_dirty |= self.remove_stale_entries(current_pass); - if sort_dirty { - self.resort_entries(); - } - let requested_selection = self - .pending_find_identity - .take() - .filter(|identity| self.entries.iter().any(|entry| entry.identity == *identity)); - let selection_scroll_policy = refresh_selection_scroll_policy(requested_selection); - self.selected_identity = requested_selection.or(previous_selection); - self.rebuild_listview(selection_scroll_policy); - self.pass_count = self.pass_count.wrapping_add(1); + sort_dirty |= self.remove_stale_entries(current_pass); + if sort_dirty { + self.resort_entries(); } + let requested_selection = self + .pending_find_identity + .take() + .filter(|identity| self.entries.iter().any(|entry| entry.identity == *identity)); + let selection_scroll_policy = refresh_selection_scroll_policy(requested_selection); + self.selected_identity = requested_selection.or(previous_selection); + self.rebuild_listview(selection_scroll_policy); + self.pass_count = self.pass_count.wrapping_add(1); } fn set_refresh_error(&mut self, error: u32) { @@ -919,10 +916,8 @@ impl ProcessPageState { self.last_refresh_error = Some(error); } - pub unsafe fn handle_worker_completion(&mut self) { - unsafe { - self.drain_worker_results(); - } + pub fn handle_worker_completion(&mut self) { + self.drain_worker_results(); } // 按当前排序列和方向重排 entries；文本列直接比较预先缓存的小写字符串。 @@ -939,7 +934,7 @@ impl ProcessPageState { previous_len != self.entries.len() } - unsafe fn rebuild_listview(&mut self, selection_scroll_policy: SelectionScrollPolicy) { + fn rebuild_listview(&mut self, selection_scroll_policy: SelectionScrollPolicy) { unsafe { // 进程列表使用 LVS_OWNERDATA；刷新只更新虚拟项数量和索引映射， // 不再为每个进程创建、删除或移动 Win32 ListView 项。 @@ -1005,7 +1000,7 @@ impl ProcessPageState { } } - unsafe fn set_list_selection( + fn set_list_selection( &self, list_hwnd: HWND, selected_index: Option, @@ -1040,6 +1035,12 @@ impl ProcessPageState { } } + /// Fills an `LVN_GETDISPINFOW` callback buffer. + /// + /// # Safety + /// + /// When `LVIF_TEXT` is set, `item.pszText` must be writable for at least + /// `item.cchTextMax` UTF-16 code units and remain exclusively available for this call. unsafe fn fill_display_info(&self, item: &mut LVITEMW) { unsafe { if (item.mask & LVIF_TEXT) == 0 @@ -1066,7 +1067,7 @@ impl ProcessPageState { } // 销毁现有列并按照 active_columns 重建所有列头。列宽优先从 options 读取，否则用默认值。 - unsafe fn setup_columns(&self, options: &Options) { + fn setup_columns(&self, options: &Options) { unsafe { let list_hwnd = self.list_hwnd(); while SendMessageW(list_hwnd, LVM_DELETECOLUMN, 0, 0) != 0 {} @@ -1109,7 +1110,7 @@ impl ProcessPageState { } } - unsafe fn save_column_layout(&self, options: &mut Options) -> Result<(), u32> { + fn save_column_layout(&self, options: &mut Options) -> Result<(), u32> { unsafe { let column_count = self.active_columns.len(); if column_count == 0 { @@ -1146,7 +1147,7 @@ impl ProcessPageState { } } - unsafe fn current_selected_identity(&self) -> Option { + fn current_selected_identity(&self) -> Option { unsafe { let list_hwnd = self.list_hwnd(); let index = SendMessageW( @@ -1169,7 +1170,7 @@ impl ProcessPageState { } // 打开“选择列”对话框。通过 ColumnDialogContext 传递页面和选项指针给 dialog proc。 - unsafe fn pick_columns(&mut self, options: &mut Options) { + fn pick_columns(&mut self, options: &mut Options) { let mut context = ColumnDialogContext { page: self as *mut ProcessPageState, options: options as *mut Options, @@ -1276,7 +1277,7 @@ fn write_process_column_layout( } // 将对话框中的列勾选状态持久化到 options。保留已有顺序和列宽，新增列追加到末尾。 -unsafe fn apply_selected_columns(hwnd: HWND, options: &mut Options) { +fn apply_selected_columns(hwnd: HWND, options: &mut Options) { unsafe { let existing_columns = columns_from_options(options); let mut existing_widths = HashMap::with_capacity(NUM_COLUMN); diff --git a/src/pages/processes/sampler.rs b/src/pages/processes/sampler.rs index bc4c530..9617255 100644 --- a/src/pages/processes/sampler.rs +++ b/src/pages/processes/sampler.rs @@ -273,7 +273,9 @@ unsafe fn query_process_account_name( let error = GetLastError(); return Err(if error == 0 { ERROR_GEN_FAILURE } else { error }); } - let token = OwnedHandle::new(raw_token).ok_or(ERROR_INVALID_DATA)?; + // 安全性: successful OpenProcessToken returns one owned token handle released by + // CloseHandle; no other owner is retained. + let token = OwnedHandle::from_raw(raw_token).ok_or(ERROR_INVALID_DATA)?; let mut required_bytes = 0u32; if GetTokenInformation( @@ -367,10 +369,11 @@ unsafe fn collect_process_identity_map( ); if enumeration_result == 0 { let error = windows_sys::Win32::Foundation::GetLastError(); - let _unexpected_buffer = OwnedWtsMemory::new(process_info); return Err(if error == 0 { ERROR_GEN_FAILURE } else { error }); } - let process_info = OwnedWtsMemory::new(process_info).ok_or(ERROR_INVALID_DATA)?; + // 安全性: successful WTSEnumerateProcessesW transfers a uniquely owned array whose + // documented release function is WTSFreeMemory. + let process_info = OwnedWtsMemory::from_raw(process_info).ok_or(ERROR_INVALID_DATA)?; let mut identities = HashMap::with_capacity(count as usize); let mut row_error = None; @@ -471,7 +474,9 @@ unsafe fn collect_process_entries( unsafe { // 采样阶段只构造“当下这一轮”的快照，真正的增量计算依赖外部传入的历史样本。 let raw_snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0); - let Some(snapshot) = OwnedHandle::new(raw_snapshot) else { + // 安全性: successful CreateToolhelp32Snapshot returns one owned snapshot handle released + // by CloseHandle; ownership is transferred immediately. + let Some(snapshot) = OwnedHandle::from_raw(raw_snapshot) else { let error = windows_sys::Win32::Foundation::GetLastError(); return Err(if error == 0 { ERROR_GEN_FAILURE } else { error }); }; @@ -539,14 +544,20 @@ unsafe fn collect_process_entries( let memory_handle = if pid == 0 { None } else { - OwnedHandle::new(OpenProcess( + let raw_handle = OpenProcess( PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, 0, pid, - )) + ); + // 安全性: a successful OpenProcess call returns one owned kernel handle whose + // release function is CloseHandle. + OwnedHandle::from_raw(raw_handle) }; let query_handle = if pid != 0 && memory_handle.is_none() { - OwnedHandle::new(OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, 0, pid)) + let raw_handle = OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, 0, pid); + // 安全性: a successful OpenProcess call returns one owned kernel handle whose + // release function is CloseHandle. + OwnedHandle::from_raw(raw_handle) } else { None }; diff --git a/src/pages/users.rs b/src/pages/users.rs index 5ce5fd8..8b6b21e 100644 --- a/src/pages/users.rs +++ b/src/pages/users.rs @@ -32,7 +32,7 @@ use windows_sys::Win32::UI::Controls::{ LVIF_PARAM, LVIF_STATE, LVIF_TEXT, LVIS_FOCUSED, LVIS_SELECTED, LVITEMW, LVM_DELETECOLUMN, LVM_DELETEITEM, LVM_ENSUREVISIBLE, LVM_GETITEMCOUNT, LVM_GETITEMW, LVM_GETNEXTITEM, LVM_INSERTCOLUMNW, LVM_INSERTITEMW, LVM_SETITEMSTATE, LVM_SETITEMW, LVN_COLUMNCLICK, - LVN_ITEMCHANGED, LVNI_SELECTED, NMLISTVIEW, + LVN_ITEMCHANGED, LVNI_SELECTED, NMHDR, NMLISTVIEW, }; use windows_sys::Win32::UI::Input::KeyboardAndMouse::EnableWindow; use windows_sys::Win32::UI::WindowsAndMessaging::{ @@ -259,18 +259,27 @@ impl UserPageState { EndDeferWindowPos(hdwp); } } - pub fn handle_notify(&mut self, lparam: isize) -> isize { + /// Handles a notification forwarded by the users page dialog procedure. + /// + /// # Safety + /// + /// `lparam` must be the live `WM_NOTIFY` payload for this page's ListView. An + /// `LVN_COLUMNCLICK` payload must point to a readable `NMLISTVIEW` for this synchronous call. + pub unsafe fn handle_notify(&mut self, lparam: LPARAM) -> isize { // 选择变化用于驱动按钮可用性，列点击则触发当前会话列表重新排序。 - // 安全性: `lparam` is provided by WM_NOTIFY and points to an NMLISTVIEW for this handler. + // 安全性: the dialog host established the raw notification contract documented above. unsafe { - let notify = &*(lparam as *const NMLISTVIEW); - if notify.hdr.idFrom as i32 == IDC_USERLIST { - if notify.hdr.code == LVN_ITEMCHANGED { + let Some(header) = (lparam as *const NMHDR).as_ref() else { + return 0; + }; + if header.idFrom == IDC_USERLIST as usize && header.hwndFrom == self.list_hwnd() { + if header.code == LVN_ITEMCHANGED { self.selected_session_identity = self.current_selected_session_identity(); self.update_ui_state(); return 1; } - if notify.hdr.code == LVN_COLUMNCLICK { + if header.code == LVN_COLUMNCLICK { + let notify = &*(lparam as *const NMLISTVIEW); let column = notify.iSubItem.max(0) as usize; if self.sort_column == column { self.sort_ascending = !self.sort_ascending; @@ -1002,14 +1011,13 @@ fn collect_user_sessions() -> UserWorkerResult { ) != 0; let error = GetLastError(); if !succeeded { - if let Some(memory) = OwnedWtsMemory::new(sessions_ptr) { - drop(memory); - } return Err(win32_error_or_gen_failure(error)); } if session_count == 0 { - if let Some(memory) = OwnedWtsMemory::new(sessions_ptr) { + // 安全性: successful WTSEnumerateSessionsW transfers any returned buffer to the + // caller, including a non-null buffer for an empty result. + if let Some(memory) = OwnedWtsMemory::from_raw(sessions_ptr) { drop(memory); } return Ok(UserSessionCollection { @@ -1018,7 +1026,9 @@ fn collect_user_sessions() -> UserWorkerResult { }); } - let Some(sessions_memory) = OwnedWtsMemory::new(sessions_ptr) else { + // 安全性: successful WTSEnumerateSessionsW returns a uniquely owned array whose required + // release function is WTSFreeMemory. + let Some(sessions_memory) = OwnedWtsMemory::from_raw(sessions_ptr) else { return Err(ERROR_INVALID_DATA); }; let raw_sessions = slice::from_raw_parts( @@ -1119,13 +1129,12 @@ fn query_session_info(session_id: u32) -> Result { ) != 0; let error = GetLastError(); if !succeeded { - if let Some(memory) = OwnedWtsMemory::new(buffer) { - drop(memory); - } return Err(win32_error_or_gen_failure(error)); } - let Some(memory) = OwnedWtsMemory::new(buffer) else { + // 安全性: successful WTSQuerySessionInformationW transfers one WTS allocation to the + // caller, and WTSFreeMemory is its documented release function. + let Some(memory) = OwnedWtsMemory::from_raw(buffer) else { return Err(ERROR_INVALID_DATA); }; if bytes < size_of::() as u32 { @@ -1196,18 +1205,18 @@ fn query_session_string(session_id: u32, info_class: i32) -> Result ) != 0; let error = GetLastError(); if !succeeded { - if let Some(buffer) = OwnedWtsMemory::new(buffer) { - drop(buffer); - } return Err(win32_error_or_gen_failure(error)); } if bytes == 0 { - if let Some(buffer) = OwnedWtsMemory::new(buffer) { + // 安全性: the successful WTS query transfers any non-null empty-result allocation. + if let Some(buffer) = OwnedWtsMemory::from_raw(buffer) { drop(buffer); } return Ok(String::new()); } - let Some(buffer) = OwnedWtsMemory::new(buffer) else { + // 安全性: successful WTSQuerySessionInformationW returns a uniquely owned UTF-16 buffer + // whose documented release function is WTSFreeMemory. + let Some(buffer) = OwnedWtsMemory::from_raw(buffer) else { return Err(ERROR_INVALID_DATA); }; if bytes < size_of::() as u32 || !bytes.is_multiple_of(size_of::() as u32) { diff --git a/src/system/cpu_sets.rs b/src/system/cpu_sets.rs index d08ade1..9af38cc 100644 --- a/src/system/cpu_sets.rs +++ b/src/system/cpu_sets.rs @@ -173,49 +173,52 @@ impl CpuSetTopology { } } -pub(crate) unsafe fn query_process_default_cpu_sets( +pub(crate) fn query_process_default_cpu_sets( process: HANDLE, ) -> Result, CpuSetError> { - unsafe { - let mut required = 0u32; - let size_result = GetProcessDefaultCpuSets(process, null_mut(), 0, &mut required); - if size_result != 0 { - return if required == 0 { - Ok(Vec::new()) - } else { - Err(CpuSetError::DefaultSetSize(ERROR_INVALID_DATA)) - }; - } - let error = GetLastError(); - if error != ERROR_INSUFFICIENT_BUFFER || required == 0 { - return Err(CpuSetError::DefaultSetSize(nonzero_error(error))); - } + let mut required = 0u32; + // SAFETY: the null buffer is paired with a zero capacity and `required` is a valid output. + let size_result = unsafe { GetProcessDefaultCpuSets(process, null_mut(), 0, &mut required) }; + if size_result != 0 { + return if required == 0 { + Ok(Vec::new()) + } else { + Err(CpuSetError::DefaultSetSize(ERROR_INVALID_DATA)) + }; + } + let error = unsafe { GetLastError() }; + if error != ERROR_INSUFFICIENT_BUFFER || required == 0 { + return Err(CpuSetError::DefaultSetSize(nonzero_error(error))); + } - for _ in 0..MAX_QUERY_ATTEMPTS { - let mut ids = vec![0u32; required as usize]; - let mut returned = required; - if GetProcessDefaultCpuSets( + for _ in 0..MAX_QUERY_ATTEMPTS { + let mut ids = vec![0u32; required as usize]; + let mut returned = required; + // SAFETY: `ids` is writable for the supplied element count and `returned` is a valid + // output. Invalid or stale process handles are reported by the API as errors. + if unsafe { + GetProcessDefaultCpuSets( process, ids.as_mut_ptr(), u32::try_from(ids.len()).unwrap_or(u32::MAX), &mut returned, - ) != 0 - { - if returned as usize > ids.len() { - return Err(CpuSetError::DefaultSetData(ERROR_INVALID_DATA)); - } - ids.truncate(returned as usize); - return Ok(ids); + ) + } != 0 + { + if returned as usize > ids.len() { + return Err(CpuSetError::DefaultSetData(ERROR_INVALID_DATA)); } + ids.truncate(returned as usize); + return Ok(ids); + } - let error = GetLastError(); - if error != ERROR_INSUFFICIENT_BUFFER || returned <= required { - return Err(CpuSetError::DefaultSetData(nonzero_error(error))); - } - required = returned; + let error = unsafe { GetLastError() }; + if error != ERROR_INSUFFICIENT_BUFFER || returned <= required { + return Err(CpuSetError::DefaultSetData(nonzero_error(error))); } - Err(CpuSetError::DefaultSetData(ERROR_INSUFFICIENT_BUFFER)) + required = returned; } + Err(CpuSetError::DefaultSetData(ERROR_INSUFFICIENT_BUFFER)) } fn query_cpu_set_bytes(process: HANDLE) -> Result, CpuSetError> { diff --git a/src/system/process_identity.rs b/src/system/process_identity.rs index db3a811..0a66fa8 100644 --- a/src/system/process_identity.rs +++ b/src/system/process_identity.rs @@ -77,10 +77,11 @@ pub(crate) fn open_process_for_identity( } fn open_process(pid: u32, access: u32) -> Result { - // SAFETY: OpenProcess takes scalar values only. A successful raw handle is transferred into - // OwnedHandle immediately so every return path closes it exactly once. + // SAFETY: OpenProcess takes scalar values only. let raw_handle = unsafe { OpenProcess(access, 0, pid) }; - OwnedHandle::new(raw_handle).ok_or_else(last_error_or_gen_failure) + // SAFETY: a successful OpenProcess call returns one owned kernel handle whose release function + // is CloseHandle; ownership is transferred immediately and never duplicated. + unsafe { OwnedHandle::from_raw(raw_handle) }.ok_or_else(last_error_or_gen_failure) } fn query_process_creation_time(handle: windows_sys::Win32::Foundation::HANDLE) -> Result { diff --git a/src/ui/assets.rs b/src/ui/assets.rs index d5d1992..be583f4 100644 --- a/src/ui/assets.rs +++ b/src/ui/assets.rs @@ -13,7 +13,9 @@ use std::ptr::{null, null_mut}; -use windows_sys::Win32::Foundation::HINSTANCE; +use windows_sys::Win32::Foundation::{ + ERROR_GEN_FAILURE, ERROR_INVALID_PARAMETER, GetLastError, HINSTANCE, +}; use windows_sys::Win32::Graphics::Gdi::HBITMAP; use windows_sys::Win32::System::LibraryLoader::GetModuleHandleW; use windows_sys::Win32::UI::Input::KeyboardAndMouse::{ @@ -21,9 +23,10 @@ use windows_sys::Win32::UI::Input::KeyboardAndMouse::{ }; use windows_sys::Win32::UI::WindowsAndMessaging::{ ACCEL, CreateAcceleratorTableW, FCONTROL, FNOINVERT, FSHIFT, FVIRTKEY, HACCEL, HICON, - IMAGE_BITMAP, IMAGE_ICON, LoadImageW, + IMAGE_BITMAP, IMAGE_ICON, LR_SHARED, LoadImageW, }; +use crate::infrastructure::native::OwnedIcon; use crate::ui::resource_ids::{ IDB_METER_LIT_GREEN, IDB_METER_LIT_RED, IDB_METER_UNLIT, IDC_ENDTASK, IDC_NEXTTAB, IDC_PREVTAB, IDC_SWITCHTO, IDI_APPLICATION, IDI_DEFAULT_PROCESS, IDM_HIDE, IDM_REFRESH, TRAY_ICON_IDS, @@ -51,17 +54,40 @@ fn current_module() -> HINSTANCE { unsafe { GetModuleHandleW(null::()) as HINSTANCE } } -pub fn load_icon_resource(resource_id: u16, width: i32, height: i32, flags: u32) -> HICON { +pub fn load_icon_resource( + resource_id: u16, + width: i32, + height: i32, + flags: u32, +) -> Result { + // Shared icons remain system-owned and must never enter the `OwnedIcon` destructor path. + if flags & LR_SHARED != 0 { + return Err(ERROR_INVALID_PARAMETER); + } + let module = current_module(); if module.is_null() { - return null_mut(); + return Err(last_error_or_gen_failure()); } // Win32 encodes integer resource IDs as pointer-sized values whose high word is zero. let resource = resource_id as usize as *const u16; // 安全性: `resource` is a valid MAKEINTRESOURCE-style value and `module` is the current // executable image containing the compiled icon table. - unsafe { LoadImageW(module, resource, IMAGE_ICON, width, height, flags) as HICON } + let icon = unsafe { LoadImageW(module, resource, IMAGE_ICON, width, height, flags) as HICON }; + if icon.is_null() { + return Err(last_error_or_gen_failure()); + } + + // 安全性: LR_SHARED was rejected and successful `LoadImageW(IMAGE_ICON)` transfers one icon + // that Microsoft documents must be released with `DestroyIcon`. + unsafe { OwnedIcon::from_raw(icon) }.ok_or(ERROR_GEN_FAILURE) +} + +fn last_error_or_gen_failure() -> u32 { + // 安全性: reading the calling thread's last-error slot has no caller-side preconditions. + let error = unsafe { GetLastError() }; + if error == 0 { ERROR_GEN_FAILURE } else { error } } pub fn load_bitmap_resource(resource_id: u16) -> HBITMAP { diff --git a/src/ui/localization/text_key.rs b/src/ui/localization/text_key.rs index 8a97f3a..908cdea 100644 --- a/src/ui/localization/text_key.rs +++ b/src/ui/localization/text_key.rs @@ -169,6 +169,7 @@ pub enum TextKey { Cascade, BringToFront, HelpTopics, + HelpOpenFailed, DiagnosticLogs, DiagnosticLogsTitle, DiagnosticStatusLabel, From 3bc04a102af954d0917120e9e1eb1d7865371e0c Mon Sep 17 00:00:00 2001 From: JamesLinYJ <110664404+JamesLinYJ@users.noreply.github.com> Date: Sun, 9 Aug 2026 20:34:30 +0800 Subject: [PATCH 2/3] Address Windows CI feedback --- src/app/mod.rs | 42 +++++++++++------------- src/app/single_instance.rs | 16 ++++------ src/infrastructure/native/mod.rs | 5 ++- src/infrastructure/native/ui.rs | 5 ++- src/pages/applications/icons.rs | 19 ++++++----- src/pages/network.rs | 55 ++++++++++---------------------- src/pages/processes/actions.rs | 22 ++++--------- src/pages/processes/mod.rs | 55 +++++++++++++++----------------- src/pages/processes/sampler.rs | 6 +--- src/system/cpu_sets.rs | 4 +-- 10 files changed, 90 insertions(+), 139 deletions(-) diff --git a/src/app/mod.rs b/src/app/mod.rs index 179e164..8529a75 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -62,21 +62,21 @@ use windows_sys::Win32::UI::WindowsAndMessaging::{ DestroyAcceleratorTable, DestroyWindow, DispatchMessageW, DrawMenuBar, EnableMenuItem, GWL_STYLE, GetClassInfoW, GetClientRect, GetCursorPos, GetDlgItem, GetForegroundWindow, GetMenu, GetMenuItemInfoW, GetMessageW, GetShellWindow, GetWindowLongW, GetWindowPlacement, - GetWindowRect, HACCEL, HICON, HMENU, HTCAPTION, HTCLIENT, HWND_NOTOPMOST, HWND_TOP, HWND_TOPMOST, - IDCANCEL, IsDialogMessageW, IsIconic, IsWindowVisible, IsZoomed, - KillTimer, LR_DEFAULTCOLOR, LR_DEFAULTSIZE, MB_ICONSTOP, MB_OK, MENUITEMINFOW, MF_BYCOMMAND, - MF_CHECKED, MF_ENABLED, MF_GRAYED, MF_POPUP, MF_SEPARATOR, MF_SYSMENU, MF_UNCHECKED, MIIM_ID, - MINMAXINFO, MSG, MessageBoxW, OpenIcon, PostMessageW, PostQuitMessage, RegisterClassW, - SIZE_MINIMIZED, SW_HIDE, SW_MINIMIZE, SW_SHOW, SW_SHOWMAXIMIZED, SW_SHOWMINNOACTIVE, - SW_SHOWNOACTIVATE, SW_SHOWNORMAL, SWP_FRAMECHANGED, SWP_NOACTIVATE, SWP_NOMOVE, SWP_NOREDRAW, - SWP_NOSIZE, SWP_NOZORDER, SendMessageW, SetForegroundWindow, SetMenu, SetMenuDefaultItem, - SetTimer, SetWindowLongW, SetWindowPos, SetWindowTextW, ShowWindow, TPM_RETURNCMD, - TrackPopupMenuEx, TranslateAcceleratorW, TranslateMessage, WINDOWPLACEMENT, WM_CLOSE, - WM_COMMAND, WM_CREATE, WM_DESTROY, WM_ENDSESSION, WM_ERASEBKGND, WM_GETMINMAXINFO, - WM_INITDIALOG, WM_INITMENU, WM_LBUTTONDBLCLK, WM_MENUSELECT, WM_MOVE, WM_NCHITTEST, - WM_NCLBUTTONDBLCLK, WM_NCRBUTTONDOWN, WM_NCRBUTTONUP, WM_NOTIFY, WM_RBUTTONDOWN, WM_RBUTTONUP, - WM_SETICON, WM_SETREDRAW, WM_SIZE, WM_TIMER, WNDCLASSW, WS_CAPTION, WS_CHILD, WS_CLIPSIBLINGS, - WS_DLGFRAME, WS_MAXIMIZEBOX, WS_MINIMIZEBOX, WS_POPUP, WS_SYSMENU, WS_TILEDWINDOW, WS_VISIBLE, + GetWindowRect, HACCEL, HICON, HMENU, HTCAPTION, HTCLIENT, HWND_NOTOPMOST, HWND_TOP, + HWND_TOPMOST, IDCANCEL, IsDialogMessageW, IsIconic, IsWindowVisible, IsZoomed, KillTimer, + LR_DEFAULTCOLOR, LR_DEFAULTSIZE, MB_ICONSTOP, MB_OK, MENUITEMINFOW, MF_BYCOMMAND, MF_CHECKED, + MF_ENABLED, MF_GRAYED, MF_POPUP, MF_SEPARATOR, MF_SYSMENU, MF_UNCHECKED, MIIM_ID, MINMAXINFO, + MSG, MessageBoxW, OpenIcon, PostMessageW, PostQuitMessage, RegisterClassW, SIZE_MINIMIZED, + SW_HIDE, SW_MINIMIZE, SW_SHOW, SW_SHOWMAXIMIZED, SW_SHOWMINNOACTIVE, SW_SHOWNOACTIVATE, + SW_SHOWNORMAL, SWP_FRAMECHANGED, SWP_NOACTIVATE, SWP_NOMOVE, SWP_NOREDRAW, SWP_NOSIZE, + SWP_NOZORDER, SendMessageW, SetForegroundWindow, SetMenu, SetMenuDefaultItem, SetTimer, + SetWindowLongW, SetWindowPos, SetWindowTextW, ShowWindow, TPM_RETURNCMD, TrackPopupMenuEx, + TranslateAcceleratorW, TranslateMessage, WINDOWPLACEMENT, WM_CLOSE, WM_COMMAND, WM_CREATE, + WM_DESTROY, WM_ENDSESSION, WM_ERASEBKGND, WM_GETMINMAXINFO, WM_INITDIALOG, WM_INITMENU, + WM_LBUTTONDBLCLK, WM_MENUSELECT, WM_MOVE, WM_NCHITTEST, WM_NCLBUTTONDBLCLK, WM_NCRBUTTONDOWN, + WM_NCRBUTTONUP, WM_NOTIFY, WM_RBUTTONDOWN, WM_RBUTTONUP, WM_SETICON, WM_SETREDRAW, WM_SIZE, + WM_TIMER, WNDCLASSW, WS_CAPTION, WS_CHILD, WS_CLIPSIBLINGS, WS_DLGFRAME, WS_MAXIMIZEBOX, + WS_MINIMIZEBOX, WS_POPUP, WS_SYSMENU, WS_TILEDWINDOW, WS_VISIBLE, }; use self::controllers::{ @@ -1919,12 +1919,7 @@ impl App { let body = to_wide_null(&body); // Safety: the owner HWND is live and both UTF-16 buffers remain valid for the call. unsafe { - MessageBoxW( - hwnd, - body.as_ptr(), - title.as_ptr(), - MB_OK | MB_ICONSTOP, - ); + MessageBoxW(hwnd, body.as_ptr(), title.as_ptr(), MB_OK | MB_ICONSTOP); } } } @@ -2722,9 +2717,8 @@ unsafe extern "system" fn main_window_proc( mod tests { use super::page_registry::PageId; use super::{ - active_page_id, active_page_uses_normal_minimum, clamped_window_size, is_active_page, - ShellExecuteFailure, help_documentation_url, page_uses_normal_minimum, - shell_execute_failure, + ShellExecuteFailure, active_page_id, active_page_uses_normal_minimum, clamped_window_size, + help_documentation_url, is_active_page, page_uses_normal_minimum, shell_execute_failure, }; #[test] diff --git a/src/app/single_instance.rs b/src/app/single_instance.rs index ea426ce..c94fcd5 100644 --- a/src/app/single_instance.rs +++ b/src/app/single_instance.rs @@ -22,8 +22,8 @@ use std::ptr::{null, null_mut}; use sha2::{Digest, Sha256}; use windows_sys::Win32::Foundation::{ - CloseHandle, ERROR_ALREADY_EXISTS, ERROR_GEN_FAILURE, FALSE, GetLastError, HANDLE, HWND, - LocalFree, WAIT_ABANDONED, WAIT_OBJECT_0, WAIT_TIMEOUT, + ERROR_ALREADY_EXISTS, ERROR_GEN_FAILURE, FALSE, GetLastError, HANDLE, HWND, LocalFree, + WAIT_ABANDONED, WAIT_OBJECT_0, WAIT_TIMEOUT, }; use windows_sys::Win32::Security::Authorization::{ ConvertSidToStringSidW, ConvertStringSecurityDescriptorToSecurityDescriptorW, SDDL_REVISION_1, @@ -411,8 +411,7 @@ impl OwnedSid { } } // Safety: the discovered range precedes the terminating NUL. - let output = - String::from_utf16_lossy(unsafe { std::slice::from_raw_parts(value, length) }); + let output = String::from_utf16_lossy(unsafe { std::slice::from_raw_parts(value, length) }); // Safety: ConvertSidToStringSidW transfers a LocalAlloc allocation to the caller. unsafe { LocalFree(value.cast()); @@ -496,7 +495,7 @@ mod tests { use super::*; use std::sync::mpsc; use std::thread; - use windows_sys::Win32::Foundation::ERROR_ACCESS_DENIED; + use windows_sys::Win32::Foundation::{CloseHandle, ERROR_ACCESS_DENIED}; use windows_sys::Win32::System::Threading::ReleaseMutex; fn identity() -> ProcessIdentity { @@ -564,8 +563,7 @@ mod tests { let user = token_information(token.as_raw(), TokenUser).expect("user should be queryable"); let user_sid = unsafe { (*(user.as_ptr().cast::())).User.Sid }; // Safety: the SID is backed by the live TOKEN_USER buffer for this copy. - let user_sid = unsafe { OwnedSid::copy_from_raw(user_sid) } - .expect("user SID should copy"); + let user_sid = unsafe { OwnedSid::copy_from_raw(user_sid) }.expect("user SID should copy"); let integrity = token_information(token.as_raw(), TokenIntegrityLevel) .expect("integrity should be queryable"); let integrity_sid = unsafe { @@ -574,8 +572,8 @@ mod tests { .Sid }; // Safety: the SID is backed by the live TOKEN_MANDATORY_LABEL buffer for this copy. - let integrity_sid = unsafe { OwnedSid::copy_from_raw(integrity_sid) } - .expect("integrity SID should copy"); + let integrity_sid = + unsafe { OwnedSid::copy_from_raw(integrity_sid) }.expect("integrity SID should copy"); integrity_sid .last_subauthority() .expect("integrity SID should be valid"); diff --git a/src/infrastructure/native/mod.rs b/src/infrastructure/native/mod.rs index 8fc4ecf..d4357fc 100644 --- a/src/infrastructure/native/mod.rs +++ b/src/infrastructure/native/mod.rs @@ -26,9 +26,8 @@ pub(crate) use errors::{ pub use handles::{OwnedHandle, OwnedIcon, OwnedWtsMemory}; pub use safety::{enable_debug_privilege, process_is_elevated, process_needs_32_bit_suffix_handle}; pub use ui::{ - append_32_bit_suffix, call_window_proc, copy_text_to_callback_buffer, - copy_text_to_utf16_buffer, finish_list_view_update, format_resource_string, - get_window_userdata, height, hiword, loword, + append_32_bit_suffix, call_window_proc, copy_text_to_callback_buffer, finish_list_view_update, + format_resource_string, get_window_userdata, height, hiword, loword, pause_redraw_for_visible_windows, redraw_window_tree, resume_redraw_for_windows, sanitize_task_manager_menu, set_dialog_msg_result, set_style, set_window_userdata, set_window_userdata_ptr, subclass_list_view, to_wide_null, widestr_ptr_to_string, width, diff --git a/src/infrastructure/native/ui.rs b/src/infrastructure/native/ui.rs index 0bdc2d2..fea0783 100644 --- a/src/infrastructure/native/ui.rs +++ b/src/infrastructure/native/ui.rs @@ -589,7 +589,10 @@ mod tests { fn callback_text_is_nul_terminated_and_leaves_unused_tail_untouched() { let mut buffer = [0xAAAA; 8]; copy_text_to_utf16_buffer(&mut buffer, "Task"); - assert_eq!(&buffer[..5], &[b'T' as u16, b'a' as u16, b's' as u16, b'k' as u16, 0]); + assert_eq!( + &buffer[..5], + &[b'T' as u16, b'a' as u16, b's' as u16, b'k' as u16, 0] + ); assert_eq!(&buffer[5..], &[0xAAAA; 3]); } diff --git a/src/pages/applications/icons.rs b/src/pages/applications/icons.rs index a96bdca..f1f692e 100644 --- a/src/pages/applications/icons.rs +++ b/src/pages/applications/icons.rs @@ -243,9 +243,8 @@ impl TaskIconStore { } unsafe { SetLastError(0) }; - let small_index = unsafe { - ImageList_ReplaceIcon(self.small, -1, self.default_small_raw()) - }; + let small_index = + unsafe { ImageList_ReplaceIcon(self.small, -1, self.default_small_raw()) }; if small_index != 0 { let error = last_error_or_gen_failure(); unsafe { @@ -255,9 +254,8 @@ impl TaskIconStore { return Err(error); } unsafe { SetLastError(0) }; - let large_index = unsafe { - ImageList_ReplaceIcon(self.large, -1, self.default_large_raw()) - }; + let large_index = + unsafe { ImageList_ReplaceIcon(self.large, -1, self.default_large_raw()) }; if large_index != 0 { let error = last_error_or_gen_failure(); unsafe { @@ -566,7 +564,10 @@ fn fetch_window_icons(hwnd: HWND, is_hung: bool) -> (Option, Option Option { @@ -618,9 +619,7 @@ fn replace_owned_icon( owned_icon: Option, default_icon: HICON, ) -> Result { - let source = owned_icon - .as_ref() - .map_or(default_icon, OwnedIcon::as_raw); + let source = owned_icon.as_ref().map_or(default_icon, OwnedIcon::as_raw); let index = unsafe { ImageList_ReplaceIcon(imagelist, target, source) }; if index < 0 { Err(last_error_or_gen_failure()) diff --git a/src/pages/network.rs b/src/pages/network.rs index d9bbd2c..1491d74 100644 --- a/src/pages/network.rs +++ b/src/pages/network.rs @@ -255,12 +255,7 @@ impl NetworkPageState { Self::default() } - pub fn initialize( - &mut self, - hwnd: HWND, - main_hwnd: HWND, - hwnd_tabs: HWND, - ) -> Result<(), u32> { + pub fn initialize(&mut self, hwnd: HWND, main_hwnd: HWND, hwnd_tabs: HWND) -> Result<(), u32> { // 初始化只建立控件和基础布局；当前页由激活入口采样，隐藏页由首帧后的预热消息采样。 self.hwnd = hwnd; self.main_hwnd = main_hwnd; @@ -763,18 +758,15 @@ impl NetworkPageState { None => return, }; for completion in drained.completions { - crate::infrastructure::diagnostics::with_operation_id( - completion.operation_id, - || { - match completion.value.result { - Ok(adapters) => { - self.last_refresh_error = None; - self.apply_adapter_snapshot(adapters, completion.value.sampled_at); - } - Err(error) => self.set_refresh_error(error), + crate::infrastructure::diagnostics::with_operation_id(completion.operation_id, || { + match completion.value.result { + Ok(adapters) => { + self.last_refresh_error = None; + self.apply_adapter_snapshot(adapters, completion.value.sampled_at); } - }, - ); + Err(error) => self.set_refresh_error(error), + } + }); } if let Some(error) = drained.error { self.set_refresh_error(error); @@ -792,11 +784,7 @@ impl NetworkPageState { self.drain_worker_results(); } - fn apply_adapter_snapshot( - &mut self, - raw_adapters: Vec, - sampled_at: Instant, - ) { + fn apply_adapter_snapshot(&mut self, raw_adapters: Vec, sampled_at: Instant) { // UI 提交阶段把完整原始快照转换为列表文本和历史曲线；过程中没有系统查询。 let raw_adapters = collapse_raw_adapters(raw_adapters); let needs_initial_layout = self.last_sample_time.is_none(); @@ -875,25 +863,13 @@ impl NetworkPageState { // reset; the textual value remains "-" so it is not presented as measured idle. let total_delta = counter_delta.map_or(0, |delta| delta.2); let sent_util = counter_delta.map_or(0, |delta| { - utilization_percent_for_history( - delta.0, - raw_adapter.link_speed_bps, - elapsed_secs, - ) + utilization_percent_for_history(delta.0, raw_adapter.link_speed_bps, elapsed_secs) }); let received_util = counter_delta.map_or(0, |delta| { - utilization_percent_for_history( - delta.1, - raw_adapter.link_speed_bps, - elapsed_secs, - ) + utilization_percent_for_history(delta.1, raw_adapter.link_speed_bps, elapsed_secs) }); let total_util = counter_delta.map_or(0, |delta| { - utilization_percent_for_history( - delta.2, - raw_adapter.link_speed_bps, - elapsed_secs, - ) + utilization_percent_for_history(delta.2, raw_adapter.link_speed_bps, elapsed_secs) }); push_history(&mut sent_history, sent_util); @@ -975,8 +951,11 @@ impl NetworkPageState { if name.is_empty() { name = wide_array_to_string(&row.Description); } + // SAFETY: the live row comes from a successfully initialized MIB_IF_TABLE2; Win32 + // initializes InterfaceLuid, and reading its Value view does not outlive the owner. + let luid = unsafe { row.InterfaceLuid.Value }; let key = AdapterIdentity { - luid: row.InterfaceLuid.Value, + luid, interface_index: row.InterfaceIndex, }; diff --git a/src/pages/processes/actions.rs b/src/pages/processes/actions.rs index 53ab2f5..58b859c 100644 --- a/src/pages/processes/actions.rs +++ b/src/pages/processes/actions.rs @@ -418,11 +418,7 @@ impl ProcessPageState { } // 通过 SetPriorityClass 修改进程优先级类。先弹确认框，操作成功后刷新列表。 - pub(super) fn set_priority( - &mut self, - identity: ProcIdentity, - priority: ProcPriority, - ) -> bool { + pub(super) fn set_priority(&mut self, identity: ProcIdentity, priority: ProcPriority) -> bool { let priority_class = match priority { ProcPriority::Low => IDLE_PRIORITY_CLASS, ProcPriority::BelowNormal => BELOW_NORMAL_PRIORITY_CLASS, @@ -1025,9 +1021,8 @@ pub(super) fn load_debugger_path() -> Result, u32> { let key_name = to_wide_null("SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\AeDebug"); let value_name = to_wide_null("Debugger"); // SAFETY: both input strings are terminated and `key` is a valid output location. - let open_status = unsafe { - RegOpenKeyExW(HKEY_LOCAL_MACHINE, key_name.as_ptr(), 0, KEY_READ, &mut key) - }; + let open_status = + unsafe { RegOpenKeyExW(HKEY_LOCAL_MACHINE, key_name.as_ptr(), 0, KEY_READ, &mut key) }; if open_status != 0 { return if open_status == ERROR_FILE_NOT_FOUND || open_status == ERROR_PATH_NOT_FOUND { Ok(None) @@ -1063,12 +1058,7 @@ pub(super) fn load_debugger_path() -> Result, u32> { }; } - let mut buffer = vec![ - 0u16; - (value_size as usize) - .div_ceil(size_of::()) - .max(2) - ]; + let mut buffer = vec![0u16; (value_size as usize).div_ceil(size_of::()).max(2)]; let mut value_type = 0u32; // SAFETY: `buffer` is writable for the byte count returned by the size query and all output // pointers reference live local variables. @@ -1464,8 +1454,8 @@ fn query_windows_directory() -> Result { let mut buffer = vec![0u16; 260]; loop { // SAFETY: `buffer` is writable for the advertised number of UTF-16 code units. - let length = unsafe { GetWindowsDirectoryW(buffer.as_mut_ptr(), buffer.len() as u32) } - as usize; + let length = + unsafe { GetWindowsDirectoryW(buffer.as_mut_ptr(), buffer.len() as u32) } as usize; if length == 0 { return Err(unsafe { GetLastError() }); } diff --git a/src/pages/processes/mod.rs b/src/pages/processes/mod.rs index 6e2f209..536966c 100644 --- a/src/pages/processes/mod.rs +++ b/src/pages/processes/mod.rs @@ -748,30 +748,28 @@ impl ProcessPageState { // 从进程页跳转到指定 PID 的行并高亮选中。由任务页的“转到进程”命令触发。 pub fn find_process(&mut self, identity: ProcIdentity) -> bool { - unsafe { - if !identity.is_verified() { - return false; - } - let Some(index) = self - .entries - .iter() - .position(|entry| entry.identity == identity) - else { - self.pending_find_identity = Some(identity); - self.refresh_processes(); - return true; - }; - - self.selected_identity = Some(self.entries[index].identity); - let list_hwnd = self.list_hwnd(); - self.set_list_selection( - list_hwnd, - Some(index), - SelectionScrollPolicy::RevealSelection, - ); - self.update_ui_state(); - true + if !identity.is_verified() { + return false; } + let Some(index) = self + .entries + .iter() + .position(|entry| entry.identity == identity) + else { + self.pending_find_identity = Some(identity); + self.refresh_processes(); + return true; + }; + + self.selected_identity = Some(self.entries[index].identity); + let list_hwnd = self.list_hwnd(); + self.set_list_selection( + list_hwnd, + Some(index), + SelectionScrollPolicy::RevealSelection, + ); + self.update_ui_state(); + true } fn list_hwnd(&self) -> HWND { @@ -841,10 +839,9 @@ impl ProcessPageState { None => return, }; for completion in drain.completions { - crate::infrastructure::diagnostics::with_operation_id( - completion.operation_id, - || self.apply_worker_completion(completion.value), - ); + crate::infrastructure::diagnostics::with_operation_id(completion.operation_id, || { + self.apply_worker_completion(completion.value) + }); } if let Some(error) = drain.error { self.set_refresh_error(error); @@ -1792,9 +1789,7 @@ mod tests { ..ProcessPageState::default() }; - unsafe { - state.apply_worker_completion(Err(5)); - } + state.apply_worker_completion(Err(5)); assert_eq!(state.entries.len(), 1); assert_eq!(state.entries[0].image_name, "trusted.exe"); diff --git a/src/pages/processes/sampler.rs b/src/pages/processes/sampler.rs index 9617255..7d37aec 100644 --- a/src/pages/processes/sampler.rs +++ b/src/pages/processes/sampler.rs @@ -544,11 +544,7 @@ unsafe fn collect_process_entries( let memory_handle = if pid == 0 { None } else { - let raw_handle = OpenProcess( - PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, - 0, - pid, - ); + let raw_handle = OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, 0, pid); // 安全性: a successful OpenProcess call returns one owned kernel handle whose // release function is CloseHandle. OwnedHandle::from_raw(raw_handle) diff --git a/src/system/cpu_sets.rs b/src/system/cpu_sets.rs index 9af38cc..84df79e 100644 --- a/src/system/cpu_sets.rs +++ b/src/system/cpu_sets.rs @@ -173,9 +173,7 @@ impl CpuSetTopology { } } -pub(crate) fn query_process_default_cpu_sets( - process: HANDLE, -) -> Result, CpuSetError> { +pub(crate) fn query_process_default_cpu_sets(process: HANDLE) -> Result, CpuSetError> { let mut required = 0u32; // SAFETY: the null buffer is paired with a zero capacity and `required` is a valid output. let size_result = unsafe { GetProcessDefaultCpuSets(process, null_mut(), 0, &mut required) }; From 6c7c3105ab2ac58d978e7be875f1a5a3e3a7f3dd Mon Sep 17 00:00:00 2001 From: JamesLinYJ <110664404+JamesLinYJ@users.noreply.github.com> Date: Sun, 9 Aug 2026 20:36:39 +0800 Subject: [PATCH 3/3] Satisfy strict Clippy --- src/infrastructure/diagnostics/crash.rs | 4 +--- src/infrastructure/native/handles.rs | 18 +++++++++--------- src/pages/applications/icons.rs | 5 +---- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/infrastructure/diagnostics/crash.rs b/src/infrastructure/diagnostics/crash.rs index a343581..1fbdff2 100644 --- a/src/infrastructure/diagnostics/crash.rs +++ b/src/infrastructure/diagnostics/crash.rs @@ -235,9 +235,7 @@ fn build_crash_path<'a>( tid: u32, extension: &str, ) -> Option> { - let Some(directory) = CRASH_DIRECTORY.get() else { - return None; - }; + let directory = CRASH_DIRECTORY.get()?; let mut offset = 0usize; if !push_wide(output, &mut offset, directory) { return None; diff --git a/src/infrastructure/native/handles.rs b/src/infrastructure/native/handles.rs index 1ca69f8..90ee61b 100644 --- a/src/infrastructure/native/handles.rs +++ b/src/infrastructure/native/handles.rs @@ -121,6 +121,15 @@ impl Drop for OwnedIcon { } } +impl Drop for OwnedHandle { + fn drop(&mut self) { + if !self.handle.is_null() && self.handle != INVALID_HANDLE_VALUE { + // 安全性: `OwnedHandle` exclusively owns this Win32 HANDLE. + unsafe { CloseHandle(self.handle) }; + } + } +} + #[cfg(test)] mod tests { use super::OwnedIcon; @@ -131,12 +140,3 @@ mod tests { assert_send::(); } } - -impl Drop for OwnedHandle { - fn drop(&mut self) { - if !self.handle.is_null() && self.handle != INVALID_HANDLE_VALUE { - // 安全性: `OwnedHandle` exclusively owns this Win32 HANDLE. - unsafe { CloseHandle(self.handle) }; - } - } -} diff --git a/src/pages/applications/icons.rs b/src/pages/applications/icons.rs index f1f692e..b55debd 100644 --- a/src/pages/applications/icons.rs +++ b/src/pages/applications/icons.rs @@ -170,10 +170,7 @@ impl TaskIconStore { let default_small = self.default_small_raw(); let default_large = self.default_large_raw(); - let small_index = match replace_owned_icon(self.small, target, small_icon, default_small) { - Ok(index) => index, - Err(error) => return Err(error), - }; + let small_index = replace_owned_icon(self.small, target, small_icon, default_small)?; let large_index = match replace_owned_icon(self.large, target, large_icon, default_large) { Ok(index) => index, Err(error) => {