From 8a3a00be95486599d12b85e2ea25e120e2339936 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 28 Nov 2024 02:55:10 +0100 Subject: [PATCH] Code cleanup --- src/unix/apple/disk.rs | 74 ++++++++++++++++++++---------------- src/unix/apple/macos/disk.rs | 2 +- src/unix/freebsd/disk.rs | 40 ++++++++++++------- src/unix/linux/disk.rs | 16 +++----- src/windows/disk.rs | 41 ++++++++++---------- 5 files changed, 96 insertions(+), 77 deletions(-) diff --git a/src/unix/apple/disk.rs b/src/unix/apple/disk.rs index e2f5e28cf..712f9f06b 100644 --- a/src/unix/apple/disk.rs +++ b/src/unix/apple/disk.rs @@ -74,19 +74,23 @@ impl DiskInner { pub(crate) fn refresh_specifics(&mut self, refresh_kind: DiskRefreshKind) -> bool { if refresh_kind.kind() && self.type_ == DiskKind::Unknown(-1) { - let type_ = { - #[cfg(target_os = "macos")] + #[cfg(target_os = "macos")] + { + match self + .bsd_name + .as_ref() + .and_then(|name| crate::sys::inner::disk::get_disk_type(name)) { - self.bsd_name - .as_ref() - .and_then(|name| crate::sys::inner::disk::get_disk_type(name)) - .unwrap_or(DiskKind::Unknown(-1)) + Some(type_) => self.type_ = type_, + None => { + sysinfo_debug!("Failed to retrieve `DiskKind`"); + } } - #[cfg(not(target_os = "macos"))] - DiskKind::SSD - }; - - self.type_ = type_; + } + #[cfg(not(target_os = "macos"))] + { + self.type_ = DiskKind::SSD; + } } if refresh_kind.io_usage() { @@ -117,13 +121,21 @@ impl DiskInner { ]) { match get_disk_properties(&self.volume_url, &requested_properties) { Some(disk_props) => { - self.total_space = get_int_value( + match get_int_value( disk_props.inner(), DictKey::Extern(ffi::kCFURLVolumeTotalCapacityKey), - ) - .unwrap_or_default() - as u64; - self.available_space = get_available_volume_space(&disk_props); + ) { + Some(total_space) => self.total_space = total_space, + None => { + sysinfo_debug!("Failed to get disk total space"); + } + } + match get_available_volume_space(&disk_props) { + Some(available_space) => self.available_space = available_space, + None => { + sysinfo_debug!("Failed to get disk available space"); + } + } } None => { sysinfo_debug!("Failed to get disk properties"); @@ -311,7 +323,7 @@ fn get_disk_properties( }) } -fn get_available_volume_space(disk_props: &RetainedCFDictionary) -> u64 { +fn get_available_volume_space(disk_props: &RetainedCFDictionary) -> Option { // We prefer `AvailableCapacityForImportantUsage` over `AvailableCapacity` because // it takes more of the system's properties into account, like the trash, system-managed caches, // etc. It generally also returns higher values too, because of the above, so it's a more @@ -329,7 +341,6 @@ fn get_available_volume_space(disk_props: &RetainedCFDictionary) -> u64 { ) }) } - .unwrap_or_default() as u64 } pub(super) enum DictKey { @@ -404,15 +415,15 @@ unsafe fn get_bool_value(dict: CFDictionaryRef, key: DictKey) -> Option { get_dict_value(dict, key, |v| Some(v as CFBooleanRef == kCFBooleanTrue)) } -pub(super) unsafe fn get_int_value(dict: CFDictionaryRef, key: DictKey) -> Option { +pub(super) unsafe fn get_int_value(dict: CFDictionaryRef, key: DictKey) -> Option { get_dict_value(dict, key, |v| { let mut val: i64 = 0; if CFNumberGetValue( v.cast(), core_foundation_sys::number::kCFNumberSInt64Type, - &mut val as *mut i64 as *mut c_void, + &mut val as *mut _ as *mut _, ) { - Some(val) + Some(val as _) } else { None } @@ -502,20 +513,17 @@ unsafe fn new_disk( false }; - let total_space = if refresh_kind.details() { - get_int_value( - disk_props.inner(), - DictKey::Extern(ffi::kCFURLVolumeTotalCapacityKey), + let (total_space, available_space) = if refresh_kind.details() { + ( + get_int_value( + disk_props.inner(), + DictKey::Extern(ffi::kCFURLVolumeTotalCapacityKey), + ) + .unwrap_or(0), + get_available_volume_space(disk_props).unwrap_or(0), ) - .unwrap_or_default() as u64 } else { - 0 - }; - - let available_space = if refresh_kind.details() { - get_available_volume_space(disk_props) - } else { - 0 + (0, 0) }; let file_system = { diff --git a/src/unix/apple/macos/disk.rs b/src/unix/apple/macos/disk.rs index 34dba93dc..3522b599a 100644 --- a/src/unix/apple/macos/disk.rs +++ b/src/unix/apple/macos/disk.rs @@ -147,7 +147,7 @@ pub(crate) fn get_disk_io(bsd_name: &[u8]) -> Option<(u64, u64)> { DictKey::Defined(ffi::kIOBlockStorageDriverStatisticsBytesWrittenKey), )?; - Some((read_bytes.try_into().ok()?, written_bytes.try_into().ok()?)) + Some((read_bytes, written_bytes)) } }) } diff --git a/src/unix/freebsd/disk.rs b/src/unix/freebsd/disk.rs index b273ce937..1a808d002 100644 --- a/src/unix/freebsd/disk.rs +++ b/src/unix/freebsd/disk.rs @@ -162,16 +162,30 @@ impl GetValues for DiskInner { } } +/// Returns `(total_space, available_space)`. +unsafe fn get_statvfs( + c_mount_point: &[libc::c_char], + vfs: &mut libc::statvfs, +) -> Option<(u64, u64)> { + if libc::statvfs(c_mount_point.as_ptr() as *const _, vfs as *mut _) < 0 { + sysinfo_debug!("statvfs failed"); + None + } else { + let block_size: u64 = vfs.f_frsize as _; + Some(( + vfs.f_blocks.saturating_mul(block_size), + vfs.f_favail.saturating_mul(block_size), + )) + } +} + fn refresh_disk(disk: &mut DiskInner, refresh_kind: DiskRefreshKind) -> bool { if refresh_kind.details() { unsafe { let mut vfs: libc::statvfs = std::mem::zeroed(); - if libc::statvfs(disk.c_mount_point.as_ptr() as *const _, &mut vfs as *mut _) < 0 { - sysinfo_debug!("statvfs failed"); - } else { - let block_size: u64 = vfs.f_frsize as _; - disk.total_space = vfs.f_blocks.saturating_mul(block_size); - disk.available_space = vfs.f_favail.saturating_mul(block_size); + if let Some(total_space, available_space) = get_statvfs(disk.c_mount_point, &mut vfs) { + disk.total_space = total_space; + disk.available_space = available_space; } } } @@ -360,16 +374,16 @@ pub unsafe fn get_all_list( }; let (is_read_only, total_space, available_space) = if refresh_kind.details() { - if libc::statvfs(fs_info.f_mntonname.as_ptr(), &mut vfs) != 0 { - (false, 0, 0) - } else { - let f_frsize: u64 = vfs.f_frsize as _; - + if let Some((total_space, available_space)) = + get_statvfs(&fs_info.f_mntonname, &mut vfs) + { ( ((vfs.f_flag & libc::ST_RDONLY) != 0), - vfs.f_blocks.saturating_mul(f_frsize), - vfs.f_favail.saturating_mul(f_frsize), + available_space, + total_space, ) + } else { + (false, 0, 0) } } else { (false, 0, 0) diff --git a/src/unix/linux/disk.rs b/src/unix/linux/disk.rs index f07b56a96..83c2c7d8e 100644 --- a/src/unix/linux/disk.rs +++ b/src/unix/linux/disk.rs @@ -100,19 +100,15 @@ impl DiskInner { if self.actual_device_name.is_none() { self.actual_device_name = Some(get_actual_device_name(&self.device_name)); } - if let Some((read_bytes, written_bytes)) = procfs_disk_stats - .get(self.actual_device_name.as_ref().unwrap()) - .map(|stat| { - ( - stat.sectors_read * SECTOR_SIZE, - stat.sectors_written * SECTOR_SIZE, - ) - }) + if let Some(stat) = self + .actual_device_name + .as_ref() + .and_then(|actual_device_name| procfs_disk_stats.get(actual_device_name)) { self.old_read_bytes = self.read_bytes; self.old_written_bytes = self.written_bytes; - self.read_bytes = read_bytes; - self.written_bytes = written_bytes; + self.read_bytes = stat.sectors_read * SECTOR_SIZE; + self.written_bytes = stat.sectors_written * SECTOR_SIZE; } else { sysinfo_debug!("Failed to update disk i/o stats"); } diff --git a/src/windows/disk.rs b/src/windows/disk.rs index f9efc6dfb..6cdbc4d4f 100644 --- a/src/windows/disk.rs +++ b/src/windows/disk.rs @@ -3,7 +3,7 @@ use crate::sys::utils::HandleWrapper; use crate::{Disk, DiskKind, DiskRefreshKind, DiskUsage}; -use std::ffi::{c_void, OsStr, OsString}; +use std::ffi::{OsStr, OsString}; use std::mem::size_of; use std::os::windows::ffi::OsStringExt; use std::path::Path; @@ -169,7 +169,7 @@ impl DiskInner { pub(crate) fn refresh_specifics(&mut self, refreshes: DiskRefreshKind) -> bool { if refreshes.kind() && self.type_ == DiskKind::Unknown(-1) { - self.type_ = get_disk_kind(&self.device_path, None); + self.type_ = unsafe { get_disk_kind(&self.device_path, None) }; } if refreshes.io_usage() { @@ -360,19 +360,20 @@ fn os_string_from_zero_terminated(name: &[u16]) -> OsString { OsString::from_wide(&name[..len]) } -fn get_disk_kind(device_path: &[u16], borrowed_handle: Option<&HandleWrapper>) -> DiskKind { - let binding = ( - borrowed_handle, - if borrowed_handle.is_none() { - unsafe { HandleWrapper::new_from_file(device_path, Default::default()) } - } else { - None - }, - ); - let handle = match binding { - (Some(handle), _) => handle, - (_, Some(ref handle)) => handle, - (None, None) => return DiskKind::Unknown(-1), +unsafe fn get_disk_kind( + device_path: &[u16], + borrowed_handle: Option<&HandleWrapper>, +) -> DiskKind { + let handle_data; + let borrowed_handle = if borrowed_handle.is_none() { + handle_data = HandleWrapper::new_from_file(device_path, Default::default()); + handle_data.as_ref() + } else { + borrowed_handle + }; + let Some(handle) = borrowed_handle else { + sysinfo_debug!("Coudn't get a Handle to retrieve `DiskKind`"); + return DiskKind::Unknown(-1); }; if handle.is_invalid() { @@ -395,17 +396,17 @@ fn get_disk_kind(device_path: &[u16], borrowed_handle: Option<&HandleWrapper>) - DeviceIoControl( handle.0, IOCTL_STORAGE_QUERY_PROPERTY, - Some(&spq_trim as *const STORAGE_PROPERTY_QUERY as *const c_void), - size_of::() as u32, - Some(&mut result as *mut DEVICE_SEEK_PENALTY_DESCRIPTOR as *mut c_void), - size_of::() as u32, + Some(&spq_trim as *const STORAGE_PROPERTY_QUERY as *const _), + size_of::() as _, + Some(&mut result as *mut DEVICE_SEEK_PENALTY_DESCRIPTOR as *mut _), + size_of::() as _, Some(&mut dw_size), None, ) .is_ok() }; - if !device_io_control || dw_size != size_of::() as u32 { + if !device_io_control || dw_size != size_of::() as _ { DiskKind::Unknown(-1) } else { let is_hdd = result.IncursSeekPenalty.as_bool();