From 1892d8ca1ac115dcebda0dcb4a9390b74c7e3361 Mon Sep 17 00:00:00 2001 From: "J. David Lowe" Date: Fri, 22 Nov 2024 12:40:33 -0800 Subject: [PATCH 01/17] Add `DiskRefreshKind` --- src/common/disk.rs | 188 +++++++++++++++++++++++++++++-- src/common/impl_get_set.rs | 173 +++++++++++++++++++++++++++++ src/common/mod.rs | 2 + src/common/system.rs | 173 +---------------------------- src/lib.rs | 2 +- src/unix/apple/disk.rs | 10 +- src/unix/freebsd/disk.rs | 12 +- src/unix/linux/disk.rs | 222 +++++++++++++++++++++++-------------- src/unknown/disk.rs | 8 +- src/windows/disk.rs | 10 +- 10 files changed, 509 insertions(+), 291 deletions(-) create mode 100644 src/common/impl_get_set.rs diff --git a/src/common/disk.rs b/src/common/disk.rs index 1fe004f11..4adaf7273 100644 --- a/src/common/disk.rs +++ b/src/common/disk.rs @@ -4,6 +4,7 @@ use std::ffi::OsStr; use std::fmt; use std::path::Path; +use crate::common::impl_get_set::impl_get_set; use crate::DiskUsage; /// Struct containing a disk information. @@ -133,7 +134,9 @@ impl Disk { self.inner.is_read_only() } - /// Updates the disk' information. + /// Updates the disk' information with everything loaded. + /// + /// Equivalent to [`Disk::refresh_specifics`]`(`[`DiskRefreshKind::everything`]`())`. /// /// ```no_run /// use sysinfo::Disks; @@ -144,7 +147,21 @@ impl Disk { /// } /// ``` pub fn refresh(&mut self) -> bool { - self.inner.refresh() + self.refresh_specifics(DiskRefreshKind::everything()) + } + + /// Updates the disk's information corresponding to the given [`DiskRefreshKind`]. + /// + /// ```no_run + /// use sysinfo::{Disks, DiskRefreshKind}; + /// + /// let mut disks = Disks::new_with_refreshed_list(); + /// for disk in disks.list_mut() { + /// disk.refresh_specifics(DiskRefreshKind::new()); + /// } + /// ``` + pub fn refresh_specifics(&mut self, refreshes: DiskRefreshKind) -> bool { + self.inner.refresh_specifics(refreshes) } /// Returns number of bytes read and written by the disk @@ -244,7 +261,8 @@ impl Disks { } /// Creates a new [`Disks`][crate::Disks] type with the disk list loaded. - /// It is a combination of [`Disks::new`] and [`Disks::refresh_list`]. + /// + /// Equivalent to [`Disks::new_with_refreshed_list_specifics`]`(`[`DiskRefreshKind::everything`]`())`. /// /// ```no_run /// use sysinfo::Disks; @@ -255,8 +273,24 @@ impl Disks { /// } /// ``` pub fn new_with_refreshed_list() -> Self { + Self::new_with_refreshed_list_specifics(DiskRefreshKind::everything()) + } + + /// Creates a new [`Disks`][crate::Disks] type with the disk list loaded + /// and refreshed according to the given [`DiskRefreshKind`]. It is a combination of + /// [`Disks::new`] and [`Disks::refresh_list_specifics`]. + /// + /// ```no_run + /// use sysinfo::{Disks, DiskRefreshKind}; + /// + /// let mut disks = Disks::new_with_refreshed_list_specifics(DiskRefreshKind::new()); + /// for disk in disks.list() { + /// println!("{disk:?}"); + /// } + /// ``` + pub fn new_with_refreshed_list_specifics(refreshes: DiskRefreshKind) -> Self { let mut disks = Self::new(); - disks.refresh_list(); + disks.refresh_list_specifics(refreshes); disks } @@ -291,6 +325,13 @@ impl Disks { /// Refreshes the listed disks' information. /// + /// Equivalent to [`Disks::refresh_specifics`]`(`[`DiskRefreshKind::everything`]`())`. + pub fn refresh(&mut self) { + self.refresh_specifics(DiskRefreshKind::everything()); + } + + /// Refreshes the listed disks' information according to the given [`DiskRefreshKind`]. + /// /// ⚠️ If a disk is added or removed, this method won't take it into account. Use /// [`Disks::refresh_list`] instead. /// @@ -304,30 +345,45 @@ impl Disks { /// // We wait some time...? /// disks.refresh(); /// ``` - pub fn refresh(&mut self) { - self.inner.refresh(); + pub fn refresh_specifics(&mut self, refreshes: DiskRefreshKind) { + self.inner.refresh_specifics(refreshes); } /// The disk list will be emptied then completely recomputed. /// + /// Equivalent to [`Disks::refresh_list_specifics`]`(`[`DiskRefreshKind::everything`]`())`. + /// + /// ```no_run + /// use sysinfo::Disks; + /// + /// let mut disks = Disks::new(); + /// disks.refresh_list(); + /// ``` + pub fn refresh_list(&mut self) { + self.refresh_list_specifics(DiskRefreshKind::everything()); + } + + /// The disk list will be emptied then completely recomputed according to the given + /// [`DiskRefreshKind`]. + /// /// ## Linux /// /// ⚠️ On Linux, the [NFS](https://en.wikipedia.org/wiki/Network_File_System) file /// systems are ignored and the information of a mounted NFS **cannot** be obtained - /// via [`Disks::refresh_list`]. This is due to the fact that I/O function - /// `statvfs` used by [`Disks::refresh_list`] is blocking and + /// via [`Disks::refresh_list_specifics`]. This is due to the fact that I/O function + /// `statvfs` used by [`Disks::refresh_list_specifics`] is blocking and /// [may hang](https://github.com/GuillaumeGomez/sysinfo/pull/876) in some cases, /// requiring to call `systemctl stop` to terminate the NFS service from the remote /// server in some cases. /// /// ```no_run - /// use sysinfo::Disks; + /// use sysinfo::{Disks, DiskRefreshKind}; /// /// let mut disks = Disks::new(); - /// disks.refresh_list(); + /// disks.refresh_list_specifics(DiskRefreshKind::new()); /// ``` - pub fn refresh_list(&mut self) { - self.inner.refresh_list(); + pub fn refresh_list_specifics(&mut self, refreshes: DiskRefreshKind) { + self.inner.refresh_list_specifics(refreshes); } } @@ -376,3 +432,111 @@ impl fmt::Display for DiskKind { }) } } + +/// Used to determine what you want to refresh specifically on the [`Disk`] type. +/// +/// ⚠️ Just like all other refresh types, ruling out a refresh doesn't assure you that +/// the information won't be retrieved if the information is accessible without needing +/// extra computation. +/// +/// ```no_run +/// use sysinfo::{Disks, DiskRefreshKind}; +/// +/// let mut disks = Disks::new_with_refreshed_list_specifics(DiskRefreshKind::new()); +/// +/// for disk in disks.list() { +/// assert_eq!(disk.total_space(), 0); +/// } +/// ``` +#[derive(Clone, Copy, Debug)] +pub struct DiskRefreshKind { + kind: bool, + total_space: bool, + available_space: bool, + is_removable: bool, + is_read_only: bool, + usage: bool, +} + +impl Default for DiskRefreshKind { + fn default() -> Self { + Self { + kind: true, + total_space: false, + available_space: false, + is_removable: false, + is_read_only: false, + usage: false, + } + } +} + +impl DiskRefreshKind { + /// Creates a new `DiskRefreshKind` with every refresh *except kind* set to false. + /// + /// ``` + /// use sysinfo::DiskRefreshKind; + /// + /// let r = DiskRefreshKind::new(); + /// + /// assert_eq!(r.kind(), true); + /// assert_eq!(r.total_space(), false); + /// assert_eq!(r.available_space(), false); + /// assert_eq!(r.is_removable(), false); + /// assert_eq!(r.is_read_only(), false); + /// ``` + pub fn new() -> Self { + Self::default() + } + + /// Creates a new `DiskRefreshKind` with every refresh set to true. + /// + /// ``` + /// use sysinfo::DiskRefreshKind; + /// + /// let r = DiskRefreshKind::everything(); + /// + /// assert_eq!(r.kind(), true); + /// assert_eq!(r.total_space(), true); + /// assert_eq!(r.available_space(), true); + /// assert_eq!(r.is_removable(), true); + /// assert_eq!(r.is_read_only(), true); + /// ``` + pub fn everything() -> Self { + Self { + kind: true, + total_space: true, + available_space: true, + is_removable: true, + is_read_only: true, + usage: true, + } + } + + impl_get_set!(DiskRefreshKind, kind, with_kind, without_kind); + impl_get_set!( + DiskRefreshKind, + total_space, + with_total_space, + without_total_space + ); + impl_get_set!( + DiskRefreshKind, + available_space, + with_available_space, + without_available_space + ); + impl_get_set!( + DiskRefreshKind, + is_removable, + with_is_removable, + without_is_removable + ); + impl_get_set!( + DiskRefreshKind, + is_read_only, + with_is_read_only, + without_is_read_only + ); + impl_get_set!(DiskRefreshKind, usage, with_usage, without_usage); +} diff --git a/src/common/impl_get_set.rs b/src/common/impl_get_set.rs new file mode 100644 index 000000000..2df3616db --- /dev/null +++ b/src/common/impl_get_set.rs @@ -0,0 +1,173 @@ +// Take a look at the license at the top of the repository in the LICENSE file. + +macro_rules! impl_get_set { + ($ty_name:ident, $name:ident, $with:ident, $without:ident $(, $extra_doc:literal)? $(,)?) => { + #[doc = concat!("Returns the value of the \"", stringify!($name), "\" refresh kind.")] + $(#[doc = concat!(" +", $extra_doc, " +")])? + #[doc = concat!(" +``` +use sysinfo::", stringify!($ty_name), "; + +let r = ", stringify!($ty_name), "::new(); + +let r = r.with_", stringify!($name), "(); +assert_eq!(r.", stringify!($name), "(), true); + +let r = r.without_", stringify!($name), "(); +assert_eq!(r.", stringify!($name), "(), false); +```")] + pub fn $name(&self) -> bool { + self.$name + } + + #[doc = concat!("Sets the value of the \"", stringify!($name), "\" refresh kind to `true`. + +``` +use sysinfo::", stringify!($ty_name), "; + +let r = ", stringify!($ty_name), "::new(); + +let r = r.with_", stringify!($name), "(); +assert_eq!(r.", stringify!($name), "(), true); +```")] + #[must_use] + pub fn $with(mut self) -> Self { + self.$name = true; + self + } + + #[doc = concat!("Sets the value of the \"", stringify!($name), "\" refresh kind to `false`. + +``` +use sysinfo::", stringify!($ty_name), "; + +let r = ", stringify!($ty_name), "::everything(); +assert_eq!(r.", stringify!($name), "(), true); + +let r = r.without_", stringify!($name), "(); +assert_eq!(r.", stringify!($name), "(), false); +```")] + #[must_use] + pub fn $without(mut self) -> Self { + self.$name = false; + self + } + }; + + // To handle `UpdateKind`. + ($ty_name:ident, $name:ident, $with:ident, $without:ident, UpdateKind $(, $extra_doc:literal)? $(,)?) => { + #[doc = concat!("Returns the value of the \"", stringify!($name), "\" refresh kind.")] + $(#[doc = concat!(" +", $extra_doc, " +")])? + #[doc = concat!(" +``` +use sysinfo::{", stringify!($ty_name), ", UpdateKind}; + +let r = ", stringify!($ty_name), "::new(); +assert_eq!(r.", stringify!($name), "(), UpdateKind::Never); + +let r = r.with_", stringify!($name), "(UpdateKind::OnlyIfNotSet); +assert_eq!(r.", stringify!($name), "(), UpdateKind::OnlyIfNotSet); + +let r = r.without_", stringify!($name), "(); +assert_eq!(r.", stringify!($name), "(), UpdateKind::Never); +```")] + pub fn $name(&self) -> UpdateKind { + self.$name + } + + #[doc = concat!("Sets the value of the \"", stringify!($name), "\" refresh kind. + +``` +use sysinfo::{", stringify!($ty_name), ", UpdateKind}; + +let r = ", stringify!($ty_name), "::new(); +assert_eq!(r.", stringify!($name), "(), UpdateKind::Never); + +let r = r.with_", stringify!($name), "(UpdateKind::OnlyIfNotSet); +assert_eq!(r.", stringify!($name), "(), UpdateKind::OnlyIfNotSet); +```")] + #[must_use] + pub fn $with(mut self, kind: UpdateKind) -> Self { + self.$name = kind; + self + } + + #[doc = concat!("Sets the value of the \"", stringify!($name), "\" refresh kind to `UpdateKind::Never`. + +``` +use sysinfo::{", stringify!($ty_name), ", UpdateKind}; + +let r = ", stringify!($ty_name), "::everything(); +assert_eq!(r.", stringify!($name), "(), UpdateKind::OnlyIfNotSet); + +let r = r.without_", stringify!($name), "(); +assert_eq!(r.", stringify!($name), "(), UpdateKind::Never); +```")] + #[must_use] + pub fn $without(mut self) -> Self { + self.$name = UpdateKind::Never; + self + } + }; + + // To handle `*RefreshKind`. + ($ty_name:ident, $name:ident, $with:ident, $without:ident, $typ:ty $(,)?) => { + #[doc = concat!("Returns the value of the \"", stringify!($name), "\" refresh kind. + +``` +use sysinfo::{", stringify!($ty_name), ", ", stringify!($typ), "}; + +let r = ", stringify!($ty_name), "::new(); +assert_eq!(r.", stringify!($name), "().is_some(), false); + +let r = r.with_", stringify!($name), "(", stringify!($typ), "::everything()); +assert_eq!(r.", stringify!($name), "().is_some(), true); + +let r = r.without_", stringify!($name), "(); +assert_eq!(r.", stringify!($name), "().is_some(), false); +```")] + pub fn $name(&self) -> Option<$typ> { + self.$name + } + + #[doc = concat!("Sets the value of the \"", stringify!($name), "\" refresh kind to `Some(...)`. + +``` +use sysinfo::{", stringify!($ty_name), ", ", stringify!($typ), "}; + +let r = ", stringify!($ty_name), "::new(); +assert_eq!(r.", stringify!($name), "().is_some(), false); + +let r = r.with_", stringify!($name), "(", stringify!($typ), "::everything()); +assert_eq!(r.", stringify!($name), "().is_some(), true); +```")] + #[must_use] + pub fn $with(mut self, kind: $typ) -> Self { + self.$name = Some(kind); + self + } + + #[doc = concat!("Sets the value of the \"", stringify!($name), "\" refresh kind to `None`. + +``` +use sysinfo::", stringify!($ty_name), "; + +let r = ", stringify!($ty_name), "::everything(); +assert_eq!(r.", stringify!($name), "().is_some(), true); + +let r = r.without_", stringify!($name), "(); +assert_eq!(r.", stringify!($name), "().is_some(), false); +```")] + #[must_use] + pub fn $without(mut self) -> Self { + self.$name = None; + self + } + }; +} + +pub(crate) use impl_get_set; diff --git a/src/common/mod.rs b/src/common/mod.rs index cbbf52131..b59ba48f3 100644 --- a/src/common/mod.rs +++ b/src/common/mod.rs @@ -4,6 +4,8 @@ pub(crate) mod component; #[cfg(feature = "disk")] pub(crate) mod disk; +#[cfg(any(feature = "system", feature = "disk"))] +pub(crate) mod impl_get_set; #[cfg(feature = "network")] pub(crate) mod network; #[cfg(feature = "system")] diff --git a/src/common/system.rs b/src/common/system.rs index 71c71e5b1..73fd41078 100644 --- a/src/common/system.rs +++ b/src/common/system.rs @@ -6,6 +6,7 @@ use std::fmt; use std::path::Path; use std::str::FromStr; +use crate::common::impl_get_set::impl_get_set; use crate::common::DiskUsage; use crate::{CpuInner, Gid, ProcessInner, SystemInner, Uid}; @@ -1686,178 +1687,6 @@ cfg_if! { } } -macro_rules! impl_get_set { - ($ty_name:ident, $name:ident, $with:ident, $without:ident $(, $extra_doc:literal)? $(,)?) => { - #[doc = concat!("Returns the value of the \"", stringify!($name), "\" refresh kind.")] - $(#[doc = concat!(" -", $extra_doc, " -")])? - #[doc = concat!(" -``` -use sysinfo::", stringify!($ty_name), "; - -let r = ", stringify!($ty_name), "::new(); -assert_eq!(r.", stringify!($name), "(), false); - -let r = r.with_", stringify!($name), "(); -assert_eq!(r.", stringify!($name), "(), true); - -let r = r.without_", stringify!($name), "(); -assert_eq!(r.", stringify!($name), "(), false); -```")] - pub fn $name(&self) -> bool { - self.$name - } - - #[doc = concat!("Sets the value of the \"", stringify!($name), "\" refresh kind to `true`. - -``` -use sysinfo::", stringify!($ty_name), "; - -let r = ", stringify!($ty_name), "::new(); -assert_eq!(r.", stringify!($name), "(), false); - -let r = r.with_", stringify!($name), "(); -assert_eq!(r.", stringify!($name), "(), true); -```")] - #[must_use] - pub fn $with(mut self) -> Self { - self.$name = true; - self - } - - #[doc = concat!("Sets the value of the \"", stringify!($name), "\" refresh kind to `false`. - -``` -use sysinfo::", stringify!($ty_name), "; - -let r = ", stringify!($ty_name), "::everything(); -assert_eq!(r.", stringify!($name), "(), true); - -let r = r.without_", stringify!($name), "(); -assert_eq!(r.", stringify!($name), "(), false); -```")] - #[must_use] - pub fn $without(mut self) -> Self { - self.$name = false; - self - } - }; - - // To handle `UpdateKind`. - ($ty_name:ident, $name:ident, $with:ident, $without:ident, UpdateKind $(, $extra_doc:literal)? $(,)?) => { - #[doc = concat!("Returns the value of the \"", stringify!($name), "\" refresh kind.")] - $(#[doc = concat!(" -", $extra_doc, " -")])? - #[doc = concat!(" -``` -use sysinfo::{", stringify!($ty_name), ", UpdateKind}; - -let r = ", stringify!($ty_name), "::new(); -assert_eq!(r.", stringify!($name), "(), UpdateKind::Never); - -let r = r.with_", stringify!($name), "(UpdateKind::OnlyIfNotSet); -assert_eq!(r.", stringify!($name), "(), UpdateKind::OnlyIfNotSet); - -let r = r.without_", stringify!($name), "(); -assert_eq!(r.", stringify!($name), "(), UpdateKind::Never); -```")] - pub fn $name(&self) -> UpdateKind { - self.$name - } - - #[doc = concat!("Sets the value of the \"", stringify!($name), "\" refresh kind. - -``` -use sysinfo::{", stringify!($ty_name), ", UpdateKind}; - -let r = ", stringify!($ty_name), "::new(); -assert_eq!(r.", stringify!($name), "(), UpdateKind::Never); - -let r = r.with_", stringify!($name), "(UpdateKind::OnlyIfNotSet); -assert_eq!(r.", stringify!($name), "(), UpdateKind::OnlyIfNotSet); -```")] - #[must_use] - pub fn $with(mut self, kind: UpdateKind) -> Self { - self.$name = kind; - self - } - - #[doc = concat!("Sets the value of the \"", stringify!($name), "\" refresh kind to `UpdateKind::Never`. - -``` -use sysinfo::{", stringify!($ty_name), ", UpdateKind}; - -let r = ", stringify!($ty_name), "::everything(); -assert_eq!(r.", stringify!($name), "(), UpdateKind::OnlyIfNotSet); - -let r = r.without_", stringify!($name), "(); -assert_eq!(r.", stringify!($name), "(), UpdateKind::Never); -```")] - #[must_use] - pub fn $without(mut self) -> Self { - self.$name = UpdateKind::Never; - self - } - }; - - // To handle `*RefreshKind`. - ($ty_name:ident, $name:ident, $with:ident, $without:ident, $typ:ty $(,)?) => { - #[doc = concat!("Returns the value of the \"", stringify!($name), "\" refresh kind. - -``` -use sysinfo::{", stringify!($ty_name), ", ", stringify!($typ), "}; - -let r = ", stringify!($ty_name), "::new(); -assert_eq!(r.", stringify!($name), "().is_some(), false); - -let r = r.with_", stringify!($name), "(", stringify!($typ), "::everything()); -assert_eq!(r.", stringify!($name), "().is_some(), true); - -let r = r.without_", stringify!($name), "(); -assert_eq!(r.", stringify!($name), "().is_some(), false); -```")] - pub fn $name(&self) -> Option<$typ> { - self.$name - } - - #[doc = concat!("Sets the value of the \"", stringify!($name), "\" refresh kind to `Some(...)`. - -``` -use sysinfo::{", stringify!($ty_name), ", ", stringify!($typ), "}; - -let r = ", stringify!($ty_name), "::new(); -assert_eq!(r.", stringify!($name), "().is_some(), false); - -let r = r.with_", stringify!($name), "(", stringify!($typ), "::everything()); -assert_eq!(r.", stringify!($name), "().is_some(), true); -```")] - #[must_use] - pub fn $with(mut self, kind: $typ) -> Self { - self.$name = Some(kind); - self - } - - #[doc = concat!("Sets the value of the \"", stringify!($name), "\" refresh kind to `None`. - -``` -use sysinfo::", stringify!($ty_name), "; - -let r = ", stringify!($ty_name), "::everything(); -assert_eq!(r.", stringify!($name), "().is_some(), true); - -let r = r.without_", stringify!($name), "(); -assert_eq!(r.", stringify!($name), "().is_some(), false); -```")] - #[must_use] - pub fn $without(mut self) -> Self { - self.$name = None; - self - } - }; -} - /// This enum allows you to specify when you want the related information to be updated. /// /// For example if you only want the [`Process::exe()`] information to be refreshed only if it's not diff --git a/src/lib.rs b/src/lib.rs index 1a1e411c3..c04c3be1a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -71,7 +71,7 @@ cfg_if! { #[cfg(feature = "component")] pub use crate::common::component::{Component, Components}; #[cfg(feature = "disk")] -pub use crate::common::disk::{Disk, DiskKind, Disks}; +pub use crate::common::disk::{Disk, DiskKind, DiskRefreshKind, Disks}; #[cfg(feature = "network")] pub use crate::common::network::{IpNetwork, MacAddr, NetworkData, Networks}; #[cfg(feature = "system")] diff --git a/src/unix/apple/disk.rs b/src/unix/apple/disk.rs index 2ac36ac17..51d3cb183 100644 --- a/src/unix/apple/disk.rs +++ b/src/unix/apple/disk.rs @@ -7,7 +7,7 @@ use crate::{ }, DiskUsage, }; -use crate::{Disk, DiskKind}; +use crate::{Disk, DiskKind, DiskRefreshKind}; use core_foundation_sys::array::CFArrayCreate; use core_foundation_sys::base::kCFAllocatorDefault; @@ -72,7 +72,7 @@ impl DiskInner { self.is_read_only } - pub(crate) fn refresh(&mut self) -> bool { + pub(crate) fn refresh_specifics(&mut self, _refreshes: DiskRefreshKind) -> bool { #[cfg(target_os = "macos")] let Some((read_bytes, written_bytes)) = self .bsd_name @@ -129,7 +129,7 @@ impl crate::DisksInner { } } - pub(crate) fn refresh_list(&mut self) { + pub(crate) fn refresh_list_specifics(&mut self, _refreshes: DiskRefreshKind) { unsafe { // SAFETY: We don't keep any Objective-C objects around because we // don't make any direct Objective-C calls in this code. @@ -139,9 +139,9 @@ impl crate::DisksInner { } } - pub(crate) fn refresh(&mut self) { + pub(crate) fn refresh_specifics(&mut self, refreshes: DiskRefreshKind) { for disk in self.list_mut() { - disk.refresh(); + disk.refresh_specifics(refreshes); } } diff --git a/src/unix/freebsd/disk.rs b/src/unix/freebsd/disk.rs index dff7cafec..c639702c7 100644 --- a/src/unix/freebsd/disk.rs +++ b/src/unix/freebsd/disk.rs @@ -1,5 +1,7 @@ // Take a look at the license at the top of the repository in the LICENSE file. +use crate::{Disk, DiskKind, DiskRefreshKind, DiskUsage}; + use std::collections::HashMap; use std::ffi::{OsStr, OsString}; use std::marker::PhantomData; @@ -69,7 +71,7 @@ impl DiskInner { self.is_read_only } - pub(crate) fn refresh(&mut self) -> bool { + pub(crate) fn refresh_specifics(&mut self, _refreshes: DiskRefreshKind) -> bool { refresh_disk(self) } @@ -90,10 +92,8 @@ impl crate::DisksInner { } } - pub(crate) fn refresh_list(&mut self) { - unsafe { - get_all_list(&mut self.disks, true); - } + pub(crate) fn refresh_list_specifics(&mut self, _refreshes: DiskRefreshKind) { + unsafe { get_all_list(&mut self.disks, true) } } pub(crate) fn list(&self) -> &[Disk] { @@ -104,7 +104,7 @@ impl crate::DisksInner { &mut self.disks } - pub(crate) fn refresh(&mut self) { + pub(crate) fn refresh_specifics(&mut self, refreshes: DiskRefreshKind) { unsafe { get_all_list(&mut self.disks, false); } diff --git a/src/unix/linux/disk.rs b/src/unix/linux/disk.rs index 8d1d41fe9..102abcd8b 100644 --- a/src/unix/linux/disk.rs +++ b/src/unix/linux/disk.rs @@ -1,7 +1,7 @@ // Take a look at the license at the top of the repository in the LICENSE file. use crate::sys::utils::{get_all_utf8_data, to_cpath}; -use crate::{Disk, DiskKind, DiskUsage}; +use crate::{Disk, DiskKind, DiskRefreshKind, DiskUsage}; use libc::statvfs; use std::collections::HashMap; @@ -37,6 +37,8 @@ macro_rules! cast { pub(crate) struct DiskInner { type_: DiskKind, device_name: OsString, + // Potential future surprise: right now, this field is only needed by usage-related code, + // so it is only populated if DiskRefreshKind::usage() is true. actual_device_name: String, file_system: OsString, mount_point: PathBuf, @@ -83,39 +85,44 @@ impl DiskInner { self.is_read_only } - pub(crate) fn refresh(&mut self) -> bool { - self.efficient_refresh(&disk_stats()) + pub(crate) fn refresh_specifics(&mut self, refresh_kind: DiskRefreshKind) -> bool { + self.efficient_refresh(refresh_kind, &disk_stats(&refresh_kind)) } - fn efficient_refresh(&mut self, procfs_disk_stats: &HashMap) -> bool { - let Some((read_bytes, written_bytes)) = - procfs_disk_stats.get(&self.actual_device_name).map(|stat| { - ( - stat.sectors_read * SECTOR_SIZE, - stat.sectors_written * SECTOR_SIZE, - ) - }) - else { - sysinfo_debug!("Failed to update disk i/o stats"); - return false; - }; + fn efficient_refresh( + &mut self, + refresh_kind: DiskRefreshKind, + procfs_disk_stats: &HashMap, + ) -> bool { + if refresh_kind.usage() { + let Some((read_bytes, written_bytes)) = + procfs_disk_stats.get(&self.actual_device_name).map(|stat| { + ( + stat.sectors_read * SECTOR_SIZE, + stat.sectors_written * SECTOR_SIZE, + ) + }) + else { + sysinfo_debug!("Failed to update disk i/o stats"); + return false; + }; - self.old_read_bytes = self.read_bytes; - self.old_written_bytes = self.written_bytes; - self.read_bytes = read_bytes; - self.written_bytes = written_bytes; - - unsafe { - let mut stat: statvfs = mem::zeroed(); - let mount_point_cpath = to_cpath(&self.mount_point); - if retry_eintr!(statvfs(mount_point_cpath.as_ptr() as *const _, &mut stat)) == 0 { - let tmp = cast!(stat.f_bsize).saturating_mul(cast!(stat.f_bavail)); - self.available_space = cast!(tmp); - true - } else { - false - } + self.old_read_bytes = self.read_bytes; + self.old_written_bytes = self.written_bytes; + self.read_bytes = read_bytes; + self.written_bytes = written_bytes; } + + match unsafe { load_statvfs_values(&self.mount_point, refresh_kind) } { + Some((total, available, is_read_only)) => { + self.total_space = total; + self.available_space = available; + self.is_read_only = is_read_only; + } + None => return false, + }; + + true } pub(crate) fn usage(&self) -> DiskUsage { @@ -135,17 +142,19 @@ impl crate::DisksInner { } } - pub(crate) fn refresh_list(&mut self) { + pub(crate) fn refresh_list_specifics(&mut self, refresh_kind: DiskRefreshKind) { get_all_list( &mut self.disks, &get_all_utf8_data("/proc/mounts", 16_385).unwrap_or_default(), + refresh_kind, ) } - pub(crate) fn refresh(&mut self) { - let procfs_disk_stats = disk_stats(); + pub(crate) fn refresh_specifics(&mut self, refresh_kind: DiskRefreshKind) { + let procfs_disk_stats = disk_stats(&refresh_kind); for disk in self.list_mut() { - disk.inner.efficient_refresh(&procfs_disk_stats); + disk.inner + .efficient_refresh(refresh_kind, &procfs_disk_stats); } } @@ -177,39 +186,73 @@ fn get_actual_device_name(device: &OsStr) -> String { .unwrap_or_default() } -fn new_disk( - device_name: &OsStr, +unsafe fn load_statvfs_values( mount_point: &Path, - file_system: &OsStr, - removable_entries: &[PathBuf], - procfs_disk_stats: &HashMap, -) -> Option { - let mount_point_cpath = to_cpath(mount_point); - let type_ = find_type_for_device_name(device_name); - let mut total = 0; - let mut available = 0; - let mut is_read_only = false; - unsafe { + refresh_kind: DiskRefreshKind, +) -> Option<(u64, u64, bool)> { + if refresh_kind.total_space() || refresh_kind.available_space() || refresh_kind.is_read_only() { + let mount_point_cpath = to_cpath(mount_point); let mut stat: statvfs = mem::zeroed(); if retry_eintr!(statvfs(mount_point_cpath.as_ptr() as *const _, &mut stat)) == 0 { let bsize = cast!(stat.f_bsize); let blocks = cast!(stat.f_blocks); let bavail = cast!(stat.f_bavail); - total = bsize.saturating_mul(blocks); - available = bsize.saturating_mul(bavail); - is_read_only = (stat.f_flag & libc::ST_RDONLY) != 0; - } - if total == 0 { - return None; + let total = bsize.saturating_mul(blocks); + let available = bsize.saturating_mul(bavail); + let is_read_only = (stat.f_flag & libc::ST_RDONLY) != 0; + + if total == 0 { + None + } else { + Some((total, available, is_read_only)) + } + } else { + None } - let mount_point = mount_point.to_owned(); - let is_removable = removable_entries + } else { + Some((Default::default(), Default::default(), Default::default())) + } +} + +fn new_disk( + device_name: &OsStr, + mount_point: &Path, + file_system: &OsStr, + removable_entries: &[PathBuf], + procfs_disk_stats: &HashMap, + refresh_kind: DiskRefreshKind, +) -> Option { + let type_ = if refresh_kind.kind() { + find_type_for_device_name(device_name) + } else { + // TODO: discuss the representation of "you opted out of refreshing the DiskKind" + DiskKind::Unknown(-1) + }; + + let (total_space, available_space, is_read_only) = + match unsafe { load_statvfs_values(mount_point, refresh_kind) } { + Some((total_space, available_space, is_read_only)) => { + (total_space, available_space, is_read_only) + } + None => return None, + }; + + let is_removable = if refresh_kind.is_removable() { + removable_entries .iter() - .any(|e| e.as_os_str() == device_name); + .any(|e| e.as_os_str() == device_name) + } else { + Default::default() + }; - let actual_device_name = get_actual_device_name(device_name); + let actual_device_name = if refresh_kind.usage() { + get_actual_device_name(device_name) + } else { + Default::default() + }; - let (read_bytes, written_bytes) = procfs_disk_stats + let (read_bytes, written_bytes) = if refresh_kind.usage() { + procfs_disk_stats .get(&actual_device_name) .map(|stat| { ( @@ -217,26 +260,28 @@ fn new_disk( stat.sectors_written * SECTOR_SIZE, ) }) - .unwrap_or_default(); - - Some(Disk { - inner: DiskInner { - type_, - device_name: device_name.to_owned(), - actual_device_name, - file_system: file_system.to_owned(), - mount_point, - total_space: cast!(total), - available_space: cast!(available), - is_removable, - is_read_only, - old_read_bytes: 0, - old_written_bytes: 0, - read_bytes, - written_bytes, - }, - }) - } + .unwrap_or_default() + } else { + (Default::default(), Default::default()) + }; + + Some(Disk { + inner: DiskInner { + type_, + device_name: device_name.to_owned(), + actual_device_name, + file_system: file_system.to_owned(), + mount_point: mount_point.to_owned(), + total_space, + available_space, + is_removable, + is_read_only, + old_read_bytes: 0, + old_written_bytes: 0, + read_bytes, + written_bytes, + }, + }) } #[allow(clippy::manual_range_contains)] @@ -310,7 +355,7 @@ fn find_type_for_device_name(device_name: &OsStr) -> DiskKind { } } -fn get_all_list(container: &mut Vec, content: &str) { +fn get_all_list(container: &mut Vec, content: &str, refresh_kind: DiskRefreshKind) { container.clear(); // The goal of this array is to list all removable devices (the ones whose name starts with // "usb-"). @@ -331,7 +376,7 @@ fn get_all_list(container: &mut Vec, content: &str) { _ => Vec::new(), }; - let procfs_disk_stats = disk_stats(); + let procfs_disk_stats = disk_stats(&refresh_kind); for disk in content .lines() @@ -385,6 +430,7 @@ fn get_all_list(container: &mut Vec, content: &str) { fs_vfstype.as_ref(), &removable_entries, &procfs_disk_stats, + refresh_kind, ) }) { @@ -444,14 +490,18 @@ impl DiskStat { } } -fn disk_stats() -> HashMap { - let path = "/proc/diskstats"; - match fs::read_to_string(path) { - Ok(content) => disk_stats_inner(&content), - Err(_error) => { - sysinfo_debug!("failed to read {path:?}: {_error:?}"); - HashMap::new() +fn disk_stats(refresh_kind: &DiskRefreshKind) -> HashMap { + if refresh_kind.usage() { + let path = "/proc/diskstats"; + match fs::read_to_string(path) { + Ok(content) => disk_stats_inner(&content), + Err(_error) => { + sysinfo_debug!("failed to read {path:?}: {_error:?}"); + HashMap::new() + } } + } else { + Default::default() } } diff --git a/src/unknown/disk.rs b/src/unknown/disk.rs index 7ab253b47..7a4b97d54 100644 --- a/src/unknown/disk.rs +++ b/src/unknown/disk.rs @@ -1,6 +1,6 @@ // Take a look at the license at the top of the repository in the LICENSE file. -use crate::{Disk, DiskKind, DiskUsage}; +use crate::{Disk, DiskKind, DiskRefreshKind, DiskUsage}; use std::{ffi::OsStr, path::Path}; @@ -39,7 +39,7 @@ impl DiskInner { false } - pub(crate) fn refresh(&mut self) -> bool { + pub(crate) fn refresh_specifics(&mut self, _refreshes: DiskRefreshKind) -> bool { true } @@ -65,11 +65,11 @@ impl DisksInner { self.disks } - pub(crate) fn refresh_list(&mut self) { + pub(crate) fn refresh_list_specifics(&mut self, _refreshes: DiskRefreshKind) { // Does nothing. } - pub(crate) fn refresh(&mut self) { + pub(crate) fn refresh_specifics(&mut self, _refreshes: DiskRefreshKind) { // Does nothing. } diff --git a/src/windows/disk.rs b/src/windows/disk.rs index 5d635c396..a4ea8bf87 100644 --- a/src/windows/disk.rs +++ b/src/windows/disk.rs @@ -1,7 +1,7 @@ // Take a look at the license at the top of the repository in the LICENSE file. use crate::sys::utils::HandleWrapper; -use crate::{Disk, DiskKind, DiskUsage}; +use crate::{Disk, DiskKind, DiskRefreshKind, DiskUsage}; use std::ffi::{c_void, OsStr, OsString}; use std::mem::size_of; @@ -167,7 +167,7 @@ impl DiskInner { self.is_read_only } - pub(crate) fn refresh(&mut self) -> bool { + pub(crate) fn refresh_specifics(&mut self, _refreshes: DiskRefreshKind) -> bool { let Some((read_bytes, written_bytes)) = get_disk_io(&self.device_path, None) else { sysinfo_debug!("Failed to update disk i/o stats"); return false; @@ -220,15 +220,15 @@ impl DisksInner { self.disks } - pub(crate) fn refresh_list(&mut self) { + pub(crate) fn refresh_list_specifics(&mut self, _refreshes: DiskRefreshKind) { unsafe { self.disks = get_list(); } } - pub(crate) fn refresh(&mut self) { + pub(crate) fn refresh_specifics(&mut self, refreshes: DiskRefreshKind) { for disk in self.list_mut() { - disk.refresh(); + disk.refresh_specifics(refreshes); } } From 3319b189dc9ce84c6e05d83371843f3cba9e2ace Mon Sep 17 00:00:00 2001 From: "J. David Lowe" Date: Mon, 25 Nov 2024 14:22:06 -0800 Subject: [PATCH 02/17] requested changes, tests, & osx impl --- Cargo.toml | 1 + src/common/disk.rs | 64 +++------------ src/unix/apple/disk.rs | 171 ++++++++++++++++++++++++++------------- src/unix/freebsd/disk.rs | 6 +- src/unix/linux/disk.rs | 61 +++++++------- tests/disk.rs | 142 ++++++++++++++++++++++++++++---- 6 files changed, 293 insertions(+), 152 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a60eb441c..5efc08745 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -115,6 +115,7 @@ tempfile = "3.9" serde_json = "1.0" # Used in documentation tests. bstr = "1.9.0" tempfile = "3.9" +itertools = "0.13.0" [[example]] name = "simple" diff --git a/src/common/disk.rs b/src/common/disk.rs index 4adaf7273..3bab585ce 100644 --- a/src/common/disk.rs +++ b/src/common/disk.rs @@ -435,10 +435,6 @@ impl fmt::Display for DiskKind { /// Used to determine what you want to refresh specifically on the [`Disk`] type. /// -/// ⚠️ Just like all other refresh types, ruling out a refresh doesn't assure you that -/// the information won't be retrieved if the information is accessible without needing -/// extra computation. -/// /// ```no_run /// use sysinfo::{Disks, DiskRefreshKind}; /// @@ -451,22 +447,16 @@ impl fmt::Display for DiskKind { #[derive(Clone, Copy, Debug)] pub struct DiskRefreshKind { kind: bool, - total_space: bool, - available_space: bool, - is_removable: bool, - is_read_only: bool, - usage: bool, + details: bool, + io_usage: bool, } impl Default for DiskRefreshKind { fn default() -> Self { Self { kind: true, - total_space: false, - available_space: false, - is_removable: false, - is_read_only: false, - usage: false, + details: false, + io_usage: false, } } } @@ -480,10 +470,8 @@ impl DiskRefreshKind { /// let r = DiskRefreshKind::new(); /// /// assert_eq!(r.kind(), true); - /// assert_eq!(r.total_space(), false); - /// assert_eq!(r.available_space(), false); - /// assert_eq!(r.is_removable(), false); - /// assert_eq!(r.is_read_only(), false); + /// assert_eq!(r.details(), false); + /// assert_eq!(r.io_usage(), false); /// ``` pub fn new() -> Self { Self::default() @@ -497,46 +485,18 @@ impl DiskRefreshKind { /// let r = DiskRefreshKind::everything(); /// /// assert_eq!(r.kind(), true); - /// assert_eq!(r.total_space(), true); - /// assert_eq!(r.available_space(), true); - /// assert_eq!(r.is_removable(), true); - /// assert_eq!(r.is_read_only(), true); + /// assert_eq!(r.details(), true); + /// assert_eq!(r.io_usage(), true); /// ``` pub fn everything() -> Self { Self { kind: true, - total_space: true, - available_space: true, - is_removable: true, - is_read_only: true, - usage: true, + details: true, + io_usage: true, } } impl_get_set!(DiskRefreshKind, kind, with_kind, without_kind); - impl_get_set!( - DiskRefreshKind, - total_space, - with_total_space, - without_total_space - ); - impl_get_set!( - DiskRefreshKind, - available_space, - with_available_space, - without_available_space - ); - impl_get_set!( - DiskRefreshKind, - is_removable, - with_is_removable, - without_is_removable - ); - impl_get_set!( - DiskRefreshKind, - is_read_only, - with_is_read_only, - without_is_read_only - ); - impl_get_set!(DiskRefreshKind, usage, with_usage, without_usage); + impl_get_set!(DiskRefreshKind, details, with_details, without_details,); + impl_get_set!(DiskRefreshKind, io_usage, with_io_usage, without_io_usage); } diff --git a/src/unix/apple/disk.rs b/src/unix/apple/disk.rs index 51d3cb183..58763eb4a 100644 --- a/src/unix/apple/disk.rs +++ b/src/unix/apple/disk.rs @@ -72,44 +72,78 @@ impl DiskInner { self.is_read_only } - pub(crate) fn refresh_specifics(&mut self, _refreshes: DiskRefreshKind) -> bool { - #[cfg(target_os = "macos")] - let Some((read_bytes, written_bytes)) = self - .bsd_name - .as_ref() - .and_then(|name| crate::sys::inner::disk::get_disk_io(name)) - else { - sysinfo_debug!("Failed to update disk i/o stats"); - return false; + pub(crate) fn refresh_specifics(&mut self, refresh_kind: DiskRefreshKind) -> bool { + let type_ = if refresh_kind.kind() { + #[cfg(target_os = "macos")] + { + self.bsd_name + .as_ref() + .and_then(|name| crate::sys::inner::disk::get_disk_type(name)) + .unwrap_or(DiskKind::Unknown(-1)) + } + #[cfg(not(target_os = "macos"))] + DiskKind::SSD + } else { + DiskKind::Unknown(-1) + }; + + self.type_ = type_; + + let (read_bytes, written_bytes) = if refresh_kind.io_usage() { + #[cfg(target_os = "macos")] + { + self.bsd_name + .as_ref() + .and_then(|name| crate::sys::inner::disk::get_disk_io(name)) + .unwrap_or_else(|| { + sysinfo_debug!("Failed to update disk i/o stats"); + (0, 0) + }) + } + #[cfg(not(target_os = "macos"))] + (0, 0) + } else { + Default::default() }; - #[cfg(not(target_os = "macos"))] - let (read_bytes, written_bytes) = (0, 0); self.old_read_bytes = self.read_bytes; self.old_written_bytes = self.written_bytes; self.read_bytes = read_bytes; self.written_bytes = written_bytes; - unsafe { - if let Some(requested_properties) = build_requested_properties(&[ - ffi::kCFURLVolumeAvailableCapacityKey, - ffi::kCFURLVolumeAvailableCapacityForImportantUsageKey, - ]) { - match get_disk_properties(&self.volume_url, &requested_properties) { - Some(disk_props) => { - self.available_space = get_available_volume_space(&disk_props); - true - } - None => { - sysinfo_debug!("Failed to get disk properties"); - false + let (total_space, available_space) = if refresh_kind.details() { + unsafe { + if let Some(requested_properties) = build_requested_properties(&[ + ffi::kCFURLVolumeTotalCapacityKey, + ffi::kCFURLVolumeAvailableCapacityKey, + ffi::kCFURLVolumeAvailableCapacityForImportantUsageKey, + ]) { + match get_disk_properties(&self.volume_url, &requested_properties) { + Some(disk_props) => ( + get_int_value( + disk_props.inner(), + DictKey::Extern(ffi::kCFURLVolumeTotalCapacityKey), + ) + .unwrap_or_default() as u64, + get_available_volume_space(&disk_props), + ), + None => { + sysinfo_debug!("Failed to get disk properties"); + Default::default() + } } + } else { + sysinfo_debug!("failed to create volume key list, skipping refresh"); + Default::default() } - } else { - sysinfo_debug!("failed to create volume key list, skipping refresh"); - false } - } + } else { + Default::default() + }; + + self.total_space = total_space; + self.available_space = available_space; + true } pub(crate) fn usage(&self) -> DiskUsage { @@ -129,19 +163,19 @@ impl crate::DisksInner { } } - pub(crate) fn refresh_list_specifics(&mut self, _refreshes: DiskRefreshKind) { + pub(crate) fn refresh_list_specifics(&mut self, refresh_kind: DiskRefreshKind) { unsafe { // SAFETY: We don't keep any Objective-C objects around because we // don't make any direct Objective-C calls in this code. with_autorelease(|| { - get_list(&mut self.disks); + get_list(&mut self.disks, refresh_kind); }) } } - pub(crate) fn refresh_specifics(&mut self, refreshes: DiskRefreshKind) { + pub(crate) fn refresh_specifics(&mut self, refresh_kind: DiskRefreshKind) { for disk in self.list_mut() { - disk.refresh_specifics(refreshes); + disk.refresh_specifics(refresh_kind); } } @@ -154,7 +188,7 @@ impl crate::DisksInner { } } -unsafe fn get_list(container: &mut Vec) { +unsafe fn get_list(container: &mut Vec, refresh_kind: DiskRefreshKind) { container.clear(); let raw_disks = { @@ -252,7 +286,7 @@ unsafe fn get_list(container: &mut Vec) { CStr::from_ptr(c_disk.f_mntonname.as_ptr()).to_bytes(), )); - if let Some(disk) = new_disk(mount_point, volume_url, c_disk, &prop_dict) { + if let Some(disk) = new_disk(mount_point, volume_url, c_disk, &prop_dict, refresh_kind) { container.push(disk); } } @@ -398,6 +432,7 @@ unsafe fn new_disk( volume_url: RetainedCFURL, c_disk: libc::statfs, disk_props: &RetainedCFDictionary, + refresh_kind: DiskRefreshKind, ) -> Option { let bsd_name = get_bsd_name(&c_disk); @@ -406,21 +441,33 @@ unsafe fn new_disk( // so we just assume the disk type is an SSD and set disk i/o stats to 0 until Rust has a way to conditionally link to // IOKit in more recent deployment versions. - #[cfg(target_os = "macos")] - let type_ = bsd_name - .as_ref() - .and_then(|name| crate::sys::inner::disk::get_disk_type(name)) - .unwrap_or(DiskKind::Unknown(-1)); - #[cfg(not(target_os = "macos"))] - let type_ = DiskKind::SSD; + let type_ = if refresh_kind.kind() { + #[cfg(target_os = "macos")] + { + bsd_name + .as_ref() + .and_then(|name| crate::sys::inner::disk::get_disk_type(name)) + .unwrap_or(DiskKind::Unknown(-1)) + } + #[cfg(not(target_os = "macos"))] + DiskKind::SSD + } else { + DiskKind::Unknown(-1) + }; - #[cfg(target_os = "macos")] - let (read_bytes, written_bytes) = bsd_name - .as_ref() - .and_then(|name| crate::sys::inner::disk::get_disk_io(name)) - .unwrap_or_default(); - #[cfg(not(target_os = "macos"))] - let (read_bytes, written_bytes) = (0, 0); + let (read_bytes, written_bytes) = if refresh_kind.io_usage() { + #[cfg(target_os = "macos")] + { + bsd_name + .as_ref() + .and_then(|name| crate::sys::inner::disk::get_disk_io(name)) + .unwrap_or_default() + } + #[cfg(not(target_os = "macos"))] + (0, 0) + } else { + (0, 0) + }; // Note: Since we requested these properties from the system, we don't expect // these property retrievals to fail. @@ -431,7 +478,7 @@ unsafe fn new_disk( ) .map(OsString::from)?; - let is_removable = { + let is_removable = if refresh_kind.details() { let ejectable = get_bool_value( disk_props.inner(), DictKey::Extern(ffi::kCFURLVolumeIsEjectableKey), @@ -459,14 +506,24 @@ unsafe fn new_disk( !internal } + } else { + Default::default() }; - let total_space = get_int_value( - disk_props.inner(), - DictKey::Extern(ffi::kCFURLVolumeTotalCapacityKey), - )? as u64; + let total_space = if refresh_kind.details() { + get_int_value( + disk_props.inner(), + DictKey::Extern(ffi::kCFURLVolumeTotalCapacityKey), + )? as u64 + } else { + Default::default() + }; - let available_space = get_available_volume_space(disk_props); + let available_space = if refresh_kind.details() { + get_available_volume_space(disk_props) + } else { + Default::default() + }; let file_system = { let len = c_disk @@ -482,7 +539,11 @@ unsafe fn new_disk( ) }; - let is_read_only = (c_disk.f_flags & libc::MNT_RDONLY as u32) != 0; + let is_read_only = if refresh_kind.details() { + (c_disk.f_flags & libc::MNT_RDONLY as u32) != 0 + } else { + Default::default() + }; Some(Disk { inner: DiskInner { diff --git a/src/unix/freebsd/disk.rs b/src/unix/freebsd/disk.rs index c639702c7..8ed3a41bc 100644 --- a/src/unix/freebsd/disk.rs +++ b/src/unix/freebsd/disk.rs @@ -1,7 +1,5 @@ // Take a look at the license at the top of the repository in the LICENSE file. -use crate::{Disk, DiskKind, DiskRefreshKind, DiskUsage}; - use std::collections::HashMap; use std::ffi::{OsStr, OsString}; use std::marker::PhantomData; @@ -18,7 +16,7 @@ use super::ffi::{ DEVSTAT_WRITE, }; use super::utils::{c_buf_to_utf8_str, get_sys_value_str_by_name}; -use crate::{Disk, DiskKind, DiskUsage}; +use crate::{Disk, DiskKind, DiskRefreshKind, DiskUsage}; #[derive(Debug)] pub(crate) struct DiskInner { @@ -104,7 +102,7 @@ impl crate::DisksInner { &mut self.disks } - pub(crate) fn refresh_specifics(&mut self, refreshes: DiskRefreshKind) { + pub(crate) fn refresh_specifics(&mut self, _refreshes: DiskRefreshKind) { unsafe { get_all_list(&mut self.disks, false); } diff --git a/src/unix/linux/disk.rs b/src/unix/linux/disk.rs index 102abcd8b..f6de95ce0 100644 --- a/src/unix/linux/disk.rs +++ b/src/unix/linux/disk.rs @@ -94,34 +94,42 @@ impl DiskInner { refresh_kind: DiskRefreshKind, procfs_disk_stats: &HashMap, ) -> bool { - if refresh_kind.usage() { - let Some((read_bytes, written_bytes)) = + let (read_bytes, written_bytes) = if refresh_kind.io_usage() { + if let Some((read_bytes, written_bytes)) = procfs_disk_stats.get(&self.actual_device_name).map(|stat| { ( stat.sectors_read * SECTOR_SIZE, stat.sectors_written * SECTOR_SIZE, ) }) - else { + { + (read_bytes, written_bytes) + } else { sysinfo_debug!("Failed to update disk i/o stats"); - return false; - }; + Default::default() + } + } else { + Default::default() + }; - 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.old_read_bytes = self.read_bytes; + self.old_written_bytes = self.written_bytes; + self.read_bytes = read_bytes; + self.written_bytes = written_bytes; - match unsafe { load_statvfs_values(&self.mount_point, refresh_kind) } { - Some((total, available, is_read_only)) => { - self.total_space = total; - self.available_space = available; - self.is_read_only = is_read_only; + let (total_space, available_space, is_read_only) = if refresh_kind.details() { + match unsafe { load_statvfs_values(&self.mount_point, refresh_kind) } { + Some((total, available, is_read_only)) => (total, available, is_read_only), + None => Default::default(), } - None => return false, + } else { + Default::default() }; + self.total_space = total_space; + self.available_space = available_space; + self.is_read_only = is_read_only; + true } @@ -190,7 +198,7 @@ unsafe fn load_statvfs_values( mount_point: &Path, refresh_kind: DiskRefreshKind, ) -> Option<(u64, u64, bool)> { - if refresh_kind.total_space() || refresh_kind.available_space() || refresh_kind.is_read_only() { + if refresh_kind.details() { let mount_point_cpath = to_cpath(mount_point); let mut stat: statvfs = mem::zeroed(); if retry_eintr!(statvfs(mount_point_cpath.as_ptr() as *const _, &mut stat)) == 0 { @@ -221,11 +229,10 @@ fn new_disk( removable_entries: &[PathBuf], procfs_disk_stats: &HashMap, refresh_kind: DiskRefreshKind, -) -> Option { +) -> Disk { let type_ = if refresh_kind.kind() { find_type_for_device_name(device_name) } else { - // TODO: discuss the representation of "you opted out of refreshing the DiskKind" DiskKind::Unknown(-1) }; @@ -234,10 +241,10 @@ fn new_disk( Some((total_space, available_space, is_read_only)) => { (total_space, available_space, is_read_only) } - None => return None, + None => (Default::default(), Default::default(), Default::default()), }; - let is_removable = if refresh_kind.is_removable() { + let is_removable = if refresh_kind.details() { removable_entries .iter() .any(|e| e.as_os_str() == device_name) @@ -245,13 +252,13 @@ fn new_disk( Default::default() }; - let actual_device_name = if refresh_kind.usage() { + let actual_device_name = if refresh_kind.io_usage() { get_actual_device_name(device_name) } else { Default::default() }; - let (read_bytes, written_bytes) = if refresh_kind.usage() { + let (read_bytes, written_bytes) = if refresh_kind.io_usage() { procfs_disk_stats .get(&actual_device_name) .map(|stat| { @@ -265,7 +272,7 @@ fn new_disk( (Default::default(), Default::default()) }; - Some(Disk { + Disk { inner: DiskInner { type_, device_name: device_name.to_owned(), @@ -281,7 +288,7 @@ fn new_disk( read_bytes, written_bytes, }, - }) + } } #[allow(clippy::manual_range_contains)] @@ -423,7 +430,7 @@ fn get_all_list(container: &mut Vec, content: &str, refresh_kind: DiskRefr (fs_file.starts_with("/run") && !fs_file.starts_with("/run/media")) || fs_spec.starts_with("sunrpc")) }) - .filter_map(|(fs_spec, fs_file, fs_vfstype)| { + .map(|(fs_spec, fs_file, fs_vfstype)| { new_disk( fs_spec.as_ref(), Path::new(&fs_file), @@ -491,7 +498,7 @@ impl DiskStat { } fn disk_stats(refresh_kind: &DiskRefreshKind) -> HashMap { - if refresh_kind.usage() { + if refresh_kind.io_usage() { let path = "/proc/diskstats"; match fs::read_to_string(path) { Ok(content) => disk_stats_inner(&content), diff --git a/tests/disk.rs b/tests/disk.rs index 4210fcac7..2c1cb1407 100644 --- a/tests/disk.rs +++ b/tests/disk.rs @@ -1,34 +1,148 @@ // Take a look at the license at the top of the repository in the LICENSE file. +#[cfg(all(feature = "system", feature = "disk"))] +fn should_skip() -> bool { + if !sysinfo::IS_SUPPORTED_SYSTEM { + return true; + } + + let s = sysinfo::System::new_all(); + if s.physical_core_count().unwrap_or_default() == 0 { + return true; + } + false +} + #[test] #[cfg(all(feature = "system", feature = "disk"))] fn test_disks() { - if sysinfo::IS_SUPPORTED_SYSTEM { - let s = sysinfo::System::new_all(); - // If we don't have any physical core present, it's very likely that we're inside a VM... - if s.physical_core_count().unwrap_or_default() > 0 { - let mut disks = sysinfo::Disks::new(); - assert!(disks.list().is_empty()); - disks.refresh_list(); - assert!(!disks.list().is_empty()); + if should_skip() { + return; + } + + let mut disks = sysinfo::Disks::new(); + assert!(disks.list().is_empty()); + disks.refresh_list(); + assert!(!disks.list().is_empty()); +} + +#[test] +#[cfg(all(feature = "system", feature = "disk"))] +fn test_disk_refresh_kind() { + use itertools::Itertools; + + use sysinfo::{DiskKind, DiskRefreshKind, Disks}; + + if should_skip() { + return; + } + + for fs in [ + DiskRefreshKind::with_kind, + DiskRefreshKind::without_kind, + DiskRefreshKind::with_details, + DiskRefreshKind::without_details, + DiskRefreshKind::with_io_usage, + DiskRefreshKind::without_io_usage, + ] + .iter() + .powerset() + { + let mut refreshes = DiskRefreshKind::new(); + for f in fs { + refreshes = f(refreshes); } + + let assertions = |disks: &Disks| { + for disk in disks.list().iter() { + if refreshes.kind() { + assert_ne!( + disk.kind(), + DiskKind::Unknown(-1), + "disk.kind should be refreshed" + ); + } else { + assert_eq!( + disk.kind(), + DiskKind::Unknown(-1), + "disk.kind should not be refreshed" + ); + } + + if refreshes.details() { + assert_ne!( + disk.available_space(), + Default::default(), + "disk.available_space should be refreshed" + ); + assert_ne!( + disk.total_space(), + Default::default(), + "disk.total_space should be refreshed" + ); + // We can't assert anything about booleans, since false is indistinguishable from + // not-refreshed + } else { + assert_eq!( + disk.available_space(), + Default::default(), + "disk.available_space should not be refreshed" + ); + assert_eq!( + disk.total_space(), + Default::default(), + "disk.total_space should not be refreshed" + ); + assert_eq!( + disk.is_read_only(), + Default::default(), + "disk.is_read_only should not be refreshed" + ); + assert_eq!( + disk.is_removable(), + Default::default(), + "disk.is_removable should not be refreshed" + ); + } + + if refreshes.io_usage() { + assert_ne!( + disk.usage(), + Default::default(), + "disk.usage should be refreshed" + ); + } else { + assert_eq!( + disk.usage(), + Default::default(), + "disk.usage should not be refreshed" + ); + } + } + }; + + // load and refresh with the desired details should work + let disks = Disks::new_with_refreshed_list_specifics(refreshes); + assertions(&disks); + + // load with minimal `DiskRefreshKind`, then refresh for added detail should also work! + let mut disks = Disks::new_with_refreshed_list_specifics(DiskRefreshKind::new()); + disks.refresh_specifics(refreshes); + assertions(&disks); } } #[test] -#[cfg(feature = "disk")] +#[cfg(all(feature = "system", feature = "disk"))] fn test_disks_usage() { use std::fs::{remove_file, File}; use std::io::Write; use std::path::{Path, PathBuf}; use std::thread::sleep; - use sysinfo::{CpuRefreshKind, Disks, RefreshKind, System}; - - let s = System::new_with_specifics(RefreshKind::new().with_cpu(CpuRefreshKind::new())); + use sysinfo::Disks; - // Skip the tests on unsupported platforms and on systems with no physical cores (likely a VM) - if !sysinfo::IS_SUPPORTED_SYSTEM || s.physical_core_count().unwrap_or_default() == 0 { + if should_skip() { return; } From 0bb8a91445d03175c4ff87d70dd4712930c1b90e Mon Sep 17 00:00:00 2001 From: "J. David Lowe" Date: Mon, 25 Nov 2024 14:56:45 -0800 Subject: [PATCH 03/17] freebsd impl --- src/unix/freebsd/disk.rs | 97 +++++++++++++++++++++++++++------------- tests/disk.rs | 1 + 2 files changed, 67 insertions(+), 31 deletions(-) diff --git a/src/unix/freebsd/disk.rs b/src/unix/freebsd/disk.rs index 8ed3a41bc..137e9df54 100644 --- a/src/unix/freebsd/disk.rs +++ b/src/unix/freebsd/disk.rs @@ -69,8 +69,8 @@ impl DiskInner { self.is_read_only } - pub(crate) fn refresh_specifics(&mut self, _refreshes: DiskRefreshKind) -> bool { - refresh_disk(self) + pub(crate) fn refresh_specifics(&mut self, refresh_kind: DiskRefreshKind) -> bool { + refresh_disk(self, refresh_kind) } pub(crate) fn usage(&self) -> DiskUsage { @@ -90,8 +90,8 @@ impl crate::DisksInner { } } - pub(crate) fn refresh_list_specifics(&mut self, _refreshes: DiskRefreshKind) { - unsafe { get_all_list(&mut self.disks, true) } + pub(crate) fn refresh_list_specifics(&mut self, refresh_kind: DiskRefreshKind) { + unsafe { get_all_list(&mut self.disks, true, refresh_kind) } } pub(crate) fn list(&self) -> &[Disk] { @@ -102,9 +102,9 @@ impl crate::DisksInner { &mut self.disks } - pub(crate) fn refresh_specifics(&mut self, _refreshes: DiskRefreshKind) { + pub(crate) fn refresh_specifics(&mut self, refresh_kind: DiskRefreshKind) { unsafe { - get_all_list(&mut self.disks, false); + get_all_list(&mut self.disks, false, refresh_kind); } } } @@ -162,19 +162,38 @@ impl GetValues for DiskInner { } } -fn refresh_disk(disk: &mut DiskInner) -> bool { - 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 { - return false; +fn refresh_disk(disk: &mut DiskInner, refresh_kind: DiskRefreshKind) -> bool { + let (total_space, available_space) = 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 { + return false; + } + let block_size: u64 = vfs.f_frsize as _; + + ( + vfs.f_blocks.saturating_mul(block_size), + vfs.f_favail.saturating_mul(block_size), + ) } - let block_size: u64 = vfs.f_frsize as _; + } else { + Default::default() + }; + + disk.total_space = total_space; + disk.available_space = available_space; - disk.total_space = vfs.f_blocks.saturating_mul(block_size); - disk.available_space = vfs.f_favail.saturating_mul(block_size); - refresh_disk_io(&mut [disk]); - true + if refresh_kind.io_usage() { + unsafe { + refresh_disk_io(&mut [disk]); + } + } else { + disk.update_old(); + *disk.get_read() = Default::default(); + *disk.get_written() = Default::default(); } + + true } unsafe fn initialize_geom() -> Result<(), ()> { @@ -289,7 +308,11 @@ fn get_disks_mapping() -> HashMap { disk_mapping } -pub unsafe fn get_all_list(container: &mut Vec, add_new_disks: bool) { +pub unsafe fn get_all_list( + container: &mut Vec, + add_new_disks: bool, + refresh_kind: DiskRefreshKind, +) { let mut fs_infos: *mut libc::statfs = null_mut(); let count = libc::getmntinfo(&mut fs_infos, libc::MNT_WAIT); @@ -347,15 +370,21 @@ pub unsafe fn get_all_list(container: &mut Vec, add_new_disks: bool) { OsString::from(mount_point) }; - if libc::statvfs(fs_info.f_mntonname.as_ptr(), &mut vfs) != 0 { - continue; - } - - let f_frsize: u64 = vfs.f_frsize as _; - - let is_read_only = (vfs.f_flag & libc::ST_RDONLY) != 0; - let total_space = vfs.f_blocks.saturating_mul(f_frsize); - let available_space = vfs.f_favail.saturating_mul(f_frsize); + let (is_read_only, total_space, available_space) = if refresh_kind.details() { + if libc::statvfs(fs_info.f_mntonname.as_ptr(), &mut vfs) != 0 { + Default::default() + } else { + let f_frsize: u64 = vfs.f_frsize as _; + + ( + ((vfs.f_flag & libc::ST_RDONLY) != 0), + vfs.f_blocks.saturating_mul(f_frsize), + vfs.f_favail.saturating_mul(f_frsize), + ) + } + } else { + Default::default() + }; if let Some(disk) = container.iter_mut().find(|d| d.inner.name == name) { disk.inner.updated = true; @@ -365,8 +394,12 @@ pub unsafe fn get_all_list(container: &mut Vec, add_new_disks: bool) { let dev_mount_point = c_buf_to_utf8_str(&fs_info.f_mntfromname).unwrap_or(""); // USB keys and CDs are removable. - let is_removable = [b"USB", b"usb"].iter().any(|b| *b == &fs_type[..]) - || fs_type.starts_with(b"/dev/cd"); + let is_removable = if refresh_kind.details() { + [b"USB", b"usb"].iter().any(|b| *b == &fs_type[..]) + || fs_type.starts_with(b"/dev/cd") + } else { + Default::default() + }; container.push(Disk { inner: DiskInner { @@ -374,8 +407,8 @@ pub unsafe fn get_all_list(container: &mut Vec, add_new_disks: bool) { c_mount_point: fs_info.f_mntonname.to_vec(), mount_point: PathBuf::from(mount_point), dev_id: disk_mapping.get(dev_mount_point).map(ToString::to_string), - total_space: vfs.f_blocks.saturating_mul(f_frsize), - available_space: vfs.f_favail.saturating_mul(f_frsize), + total_space, + available_space, file_system: OsString::from_vec(fs_type), is_removable, is_read_only, @@ -402,7 +435,9 @@ pub unsafe fn get_all_list(container: &mut Vec, add_new_disks: bool) { c.inner.updated = false; } } - refresh_disk_io(container.as_mut_slice()); + if refresh_kind.io_usage() { + refresh_disk_io(container.as_mut_slice()); + } } // struct DevInfoWrapper { diff --git a/tests/disk.rs b/tests/disk.rs index 2c1cb1407..3daf4d0bf 100644 --- a/tests/disk.rs +++ b/tests/disk.rs @@ -56,6 +56,7 @@ fn test_disk_refresh_kind() { let assertions = |disks: &Disks| { for disk in disks.list().iter() { if refreshes.kind() { + #[cfg(not(target_os = "freebsd"))] assert_ne!( disk.kind(), DiskKind::Unknown(-1), From 81af1e393da0ce9337d4d35da4f5890c8fc42c2c Mon Sep 17 00:00:00 2001 From: "J. David Lowe" Date: Tue, 26 Nov 2024 10:28:42 -0800 Subject: [PATCH 04/17] don't special-case the default of DiskRefreshKind.kind --- src/common/disk.rs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/common/disk.rs b/src/common/disk.rs index 3bab585ce..9d3f09783 100644 --- a/src/common/disk.rs +++ b/src/common/disk.rs @@ -438,38 +438,28 @@ impl fmt::Display for DiskKind { /// ```no_run /// use sysinfo::{Disks, DiskRefreshKind}; /// -/// let mut disks = Disks::new_with_refreshed_list_specifics(DiskRefreshKind::new()); +/// let mut disks = Disks::new_with_refreshed_list_specifics(DiskRefreshKind::everything()); /// /// for disk in disks.list() { /// assert_eq!(disk.total_space(), 0); /// } /// ``` -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, Default)] pub struct DiskRefreshKind { kind: bool, details: bool, io_usage: bool, } -impl Default for DiskRefreshKind { - fn default() -> Self { - Self { - kind: true, - details: false, - io_usage: false, - } - } -} - impl DiskRefreshKind { - /// Creates a new `DiskRefreshKind` with every refresh *except kind* set to false. + /// Creates a new `DiskRefreshKind` with every refresh set to false. /// /// ``` /// use sysinfo::DiskRefreshKind; /// /// let r = DiskRefreshKind::new(); /// - /// assert_eq!(r.kind(), true); + /// assert_eq!(r.kind(), false); /// assert_eq!(r.details(), false); /// assert_eq!(r.io_usage(), false); /// ``` From aa03905d6bbfc835270bb60177af2d7fb41a7d4d Mon Sep 17 00:00:00 2001 From: "J. David Lowe" Date: Tue, 26 Nov 2024 11:19:39 -0800 Subject: [PATCH 05/17] clean up the doc links a bit --- src/common/disk.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/common/disk.rs b/src/common/disk.rs index 9d3f09783..90d2f307a 100644 --- a/src/common/disk.rs +++ b/src/common/disk.rs @@ -136,7 +136,7 @@ impl Disk { /// Updates the disk' information with everything loaded. /// - /// Equivalent to [`Disk::refresh_specifics`]`(`[`DiskRefreshKind::everything`]`())`. + /// Equivalent to [Disk::refresh_specifics]\([DiskRefreshKind::everything]\()). /// /// ```no_run /// use sysinfo::Disks; @@ -262,7 +262,7 @@ impl Disks { /// Creates a new [`Disks`][crate::Disks] type with the disk list loaded. /// - /// Equivalent to [`Disks::new_with_refreshed_list_specifics`]`(`[`DiskRefreshKind::everything`]`())`. + /// Equivalent to [Disks::new_with_refreshed_list_specifics]\([DiskRefreshKind::everything]\()). /// /// ```no_run /// use sysinfo::Disks; @@ -325,7 +325,7 @@ impl Disks { /// Refreshes the listed disks' information. /// - /// Equivalent to [`Disks::refresh_specifics`]`(`[`DiskRefreshKind::everything`]`())`. + /// Equivalent to [Disks::refresh_specifics]\([DiskRefreshKind::everything]\()). pub fn refresh(&mut self) { self.refresh_specifics(DiskRefreshKind::everything()); } @@ -351,7 +351,7 @@ impl Disks { /// The disk list will be emptied then completely recomputed. /// - /// Equivalent to [`Disks::refresh_list_specifics`]`(`[`DiskRefreshKind::everything`]`())`. + /// Equivalent to [Disks::refresh_list_specifics]\([DiskRefreshKind::everything]\()). /// /// ```no_run /// use sysinfo::Disks; From 80caec5042f889e816fdc6b613c15be7afe643b4 Mon Sep 17 00:00:00 2001 From: "J. David Lowe" Date: Tue, 26 Nov 2024 11:30:04 -0800 Subject: [PATCH 06/17] prefer explicit constants over Default::default() --- src/unix/apple/disk.rs | 16 ++++++++-------- src/unix/freebsd/disk.rs | 12 ++++++------ src/unix/linux/disk.rs | 18 +++++++++--------- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/unix/apple/disk.rs b/src/unix/apple/disk.rs index 58763eb4a..a5b9c859a 100644 --- a/src/unix/apple/disk.rs +++ b/src/unix/apple/disk.rs @@ -103,7 +103,7 @@ impl DiskInner { #[cfg(not(target_os = "macos"))] (0, 0) } else { - Default::default() + (0, 0) }; self.old_read_bytes = self.read_bytes; @@ -129,16 +129,16 @@ impl DiskInner { ), None => { sysinfo_debug!("Failed to get disk properties"); - Default::default() + (0, 0) } } } else { sysinfo_debug!("failed to create volume key list, skipping refresh"); - Default::default() + (0, 0) } } } else { - Default::default() + (0, 0) }; self.total_space = total_space; @@ -507,7 +507,7 @@ unsafe fn new_disk( !internal } } else { - Default::default() + false }; let total_space = if refresh_kind.details() { @@ -516,13 +516,13 @@ unsafe fn new_disk( DictKey::Extern(ffi::kCFURLVolumeTotalCapacityKey), )? as u64 } else { - Default::default() + 0 }; let available_space = if refresh_kind.details() { get_available_volume_space(disk_props) } else { - Default::default() + 0 }; let file_system = { @@ -542,7 +542,7 @@ unsafe fn new_disk( let is_read_only = if refresh_kind.details() { (c_disk.f_flags & libc::MNT_RDONLY as u32) != 0 } else { - Default::default() + false }; Some(Disk { diff --git a/src/unix/freebsd/disk.rs b/src/unix/freebsd/disk.rs index 137e9df54..4214a84be 100644 --- a/src/unix/freebsd/disk.rs +++ b/src/unix/freebsd/disk.rs @@ -177,7 +177,7 @@ fn refresh_disk(disk: &mut DiskInner, refresh_kind: DiskRefreshKind) -> bool { ) } } else { - Default::default() + (0, 0) }; disk.total_space = total_space; @@ -189,8 +189,8 @@ fn refresh_disk(disk: &mut DiskInner, refresh_kind: DiskRefreshKind) -> bool { } } else { disk.update_old(); - *disk.get_read() = Default::default(); - *disk.get_written() = Default::default(); + *disk.get_read() = 0; + *disk.get_written() = 0; } true @@ -372,7 +372,7 @@ 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 { - Default::default() + (false, 0, 0) } else { let f_frsize: u64 = vfs.f_frsize as _; @@ -383,7 +383,7 @@ pub unsafe fn get_all_list( ) } } else { - Default::default() + (false, 0, 0) }; if let Some(disk) = container.iter_mut().find(|d| d.inner.name == name) { @@ -398,7 +398,7 @@ pub unsafe fn get_all_list( [b"USB", b"usb"].iter().any(|b| *b == &fs_type[..]) || fs_type.starts_with(b"/dev/cd") } else { - Default::default() + false }; container.push(Disk { diff --git a/src/unix/linux/disk.rs b/src/unix/linux/disk.rs index f6de95ce0..7ff6cba78 100644 --- a/src/unix/linux/disk.rs +++ b/src/unix/linux/disk.rs @@ -106,10 +106,10 @@ impl DiskInner { (read_bytes, written_bytes) } else { sysinfo_debug!("Failed to update disk i/o stats"); - Default::default() + (0, 0) } } else { - Default::default() + (0, 0) }; self.old_read_bytes = self.read_bytes; @@ -120,10 +120,10 @@ impl DiskInner { let (total_space, available_space, is_read_only) = if refresh_kind.details() { match unsafe { load_statvfs_values(&self.mount_point, refresh_kind) } { Some((total, available, is_read_only)) => (total, available, is_read_only), - None => Default::default(), + None => (0, 0, false), } } else { - Default::default() + (0, 0, false) }; self.total_space = total_space; @@ -218,7 +218,7 @@ unsafe fn load_statvfs_values( None } } else { - Some((Default::default(), Default::default(), Default::default())) + Some((0, 0, false)) } } @@ -241,7 +241,7 @@ fn new_disk( Some((total_space, available_space, is_read_only)) => { (total_space, available_space, is_read_only) } - None => (Default::default(), Default::default(), Default::default()), + None => (0, 0, false), }; let is_removable = if refresh_kind.details() { @@ -249,13 +249,13 @@ fn new_disk( .iter() .any(|e| e.as_os_str() == device_name) } else { - Default::default() + false }; let actual_device_name = if refresh_kind.io_usage() { get_actual_device_name(device_name) } else { - Default::default() + String::new() }; let (read_bytes, written_bytes) = if refresh_kind.io_usage() { @@ -269,7 +269,7 @@ fn new_disk( }) .unwrap_or_default() } else { - (Default::default(), Default::default()) + (0, 0) }; Disk { From 2e6d12da26ff07e1280b3d34a56931f6b7e8a114 Mon Sep 17 00:00:00 2001 From: "J. David Lowe" Date: Tue, 26 Nov 2024 11:44:03 -0800 Subject: [PATCH 07/17] don't discard potentially stale values during weird refreshes --- src/unix/apple/disk.rs | 85 ++++++++++++++++++++-------------------- src/unix/freebsd/disk.rs | 18 +++------ src/unix/linux/disk.rs | 42 +++++++++----------- 3 files changed, 67 insertions(+), 78 deletions(-) diff --git a/src/unix/apple/disk.rs b/src/unix/apple/disk.rs index a5b9c859a..50cb65b97 100644 --- a/src/unix/apple/disk.rs +++ b/src/unix/apple/disk.rs @@ -73,46 +73,46 @@ impl DiskInner { } pub(crate) fn refresh_specifics(&mut self, refresh_kind: DiskRefreshKind) -> bool { - let type_ = if refresh_kind.kind() { - #[cfg(target_os = "macos")] - { - self.bsd_name - .as_ref() - .and_then(|name| crate::sys::inner::disk::get_disk_type(name)) - .unwrap_or(DiskKind::Unknown(-1)) - } - #[cfg(not(target_os = "macos"))] - DiskKind::SSD - } else { - DiskKind::Unknown(-1) - }; + if refresh_kind.kind() { + let type_ = { + #[cfg(target_os = "macos")] + { + self.bsd_name + .as_ref() + .and_then(|name| crate::sys::inner::disk::get_disk_type(name)) + .unwrap_or(DiskKind::Unknown(-1)) + } + #[cfg(not(target_os = "macos"))] + DiskKind::SSD + }; - self.type_ = type_; - - let (read_bytes, written_bytes) = if refresh_kind.io_usage() { - #[cfg(target_os = "macos")] - { - self.bsd_name - .as_ref() - .and_then(|name| crate::sys::inner::disk::get_disk_io(name)) - .unwrap_or_else(|| { - sysinfo_debug!("Failed to update disk i/o stats"); - (0, 0) - }) - } - #[cfg(not(target_os = "macos"))] - (0, 0) - } else { - (0, 0) - }; + self.type_ = type_; + } - self.old_read_bytes = self.read_bytes; - self.old_written_bytes = self.written_bytes; - self.read_bytes = read_bytes; - self.written_bytes = written_bytes; + if refresh_kind.io_usage() { + let (read_bytes, written_bytes) = { + #[cfg(target_os = "macos")] + { + self.bsd_name + .as_ref() + .and_then(|name| crate::sys::inner::disk::get_disk_io(name)) + .unwrap_or_else(|| { + sysinfo_debug!("Failed to update disk i/o stats"); + (0, 0) + }) + } + #[cfg(not(target_os = "macos"))] + (0, 0) + }; + + self.old_read_bytes = self.read_bytes; + self.old_written_bytes = self.written_bytes; + self.read_bytes = read_bytes; + self.written_bytes = written_bytes; + } - let (total_space, available_space) = if refresh_kind.details() { - unsafe { + if refresh_kind.details() { + let (total_space, available_space) = unsafe { if let Some(requested_properties) = build_requested_properties(&[ ffi::kCFURLVolumeTotalCapacityKey, ffi::kCFURLVolumeAvailableCapacityKey, @@ -136,13 +136,12 @@ impl DiskInner { sysinfo_debug!("failed to create volume key list, skipping refresh"); (0, 0) } - } - } else { - (0, 0) - }; + }; + + self.total_space = total_space; + self.available_space = available_space; + } - self.total_space = total_space; - self.available_space = available_space; true } diff --git a/src/unix/freebsd/disk.rs b/src/unix/freebsd/disk.rs index 4214a84be..64c268538 100644 --- a/src/unix/freebsd/disk.rs +++ b/src/unix/freebsd/disk.rs @@ -163,8 +163,8 @@ impl GetValues for DiskInner { } fn refresh_disk(disk: &mut DiskInner, refresh_kind: DiskRefreshKind) -> bool { - let (total_space, available_space) = if refresh_kind.details() { - unsafe { + if refresh_kind.details() { + let (total_space, available_space) = 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 { return false; @@ -175,22 +175,16 @@ fn refresh_disk(disk: &mut DiskInner, refresh_kind: DiskRefreshKind) -> bool { vfs.f_blocks.saturating_mul(block_size), vfs.f_favail.saturating_mul(block_size), ) - } - } else { - (0, 0) - }; + }; - disk.total_space = total_space; - disk.available_space = available_space; + disk.total_space = total_space; + disk.available_space = available_space; + } if refresh_kind.io_usage() { unsafe { refresh_disk_io(&mut [disk]); } - } else { - disk.update_old(); - *disk.get_read() = 0; - *disk.get_written() = 0; } true diff --git a/src/unix/linux/disk.rs b/src/unix/linux/disk.rs index 7ff6cba78..e481b5385 100644 --- a/src/unix/linux/disk.rs +++ b/src/unix/linux/disk.rs @@ -94,41 +94,37 @@ impl DiskInner { refresh_kind: DiskRefreshKind, procfs_disk_stats: &HashMap, ) -> bool { - let (read_bytes, written_bytes) = if refresh_kind.io_usage() { - if let Some((read_bytes, written_bytes)) = + if refresh_kind.io_usage() { + let (read_bytes, written_bytes) = if let Some((read_bytes, written_bytes)) = procfs_disk_stats.get(&self.actual_device_name).map(|stat| { ( stat.sectors_read * SECTOR_SIZE, stat.sectors_written * SECTOR_SIZE, ) - }) - { + }) { (read_bytes, written_bytes) } else { sysinfo_debug!("Failed to update disk i/o stats"); (0, 0) - } - } else { - (0, 0) - }; + }; - 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.old_read_bytes = self.read_bytes; + self.old_written_bytes = self.written_bytes; + self.read_bytes = read_bytes; + self.written_bytes = written_bytes; + } - let (total_space, available_space, is_read_only) = if refresh_kind.details() { - match unsafe { load_statvfs_values(&self.mount_point, refresh_kind) } { - Some((total, available, is_read_only)) => (total, available, is_read_only), - None => (0, 0, false), - } - } else { - (0, 0, false) - }; + if refresh_kind.details() { + let (total_space, available_space, is_read_only) = + match unsafe { load_statvfs_values(&self.mount_point, refresh_kind) } { + Some((total, available, is_read_only)) => (total, available, is_read_only), + None => (0, 0, false), + }; - self.total_space = total_space; - self.available_space = available_space; - self.is_read_only = is_read_only; + self.total_space = total_space; + self.available_space = available_space; + self.is_read_only = is_read_only; + } true } From 23e808e784377f2894f643c90a4d4c1097e2e8dd Mon Sep 17 00:00:00 2001 From: "J. David Lowe" Date: Tue, 26 Nov 2024 12:13:08 -0800 Subject: [PATCH 08/17] type annotation for windows --- tests/disk.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/disk.rs b/tests/disk.rs index 3daf4d0bf..d209fdf3c 100644 --- a/tests/disk.rs +++ b/tests/disk.rs @@ -96,12 +96,12 @@ fn test_disk_refresh_kind() { ); assert_eq!( disk.is_read_only(), - Default::default(), + ::default(), "disk.is_read_only should not be refreshed" ); assert_eq!( disk.is_removable(), - Default::default(), + ::default(), "disk.is_removable should not be refreshed" ); } From 6320a1486b03178753b8a0a7e745a7545ed49d30 Mon Sep 17 00:00:00 2001 From: "J. David Lowe" Date: Tue, 26 Nov 2024 12:52:15 -0800 Subject: [PATCH 09/17] windows impl & consistency tweaks --- src/unix/apple/disk.rs | 2 +- src/unix/linux/disk.rs | 4 + src/windows/disk.rs | 168 ++++++++++++++++++++++++++--------------- tests/disk.rs | 26 +++---- 4 files changed, 126 insertions(+), 74 deletions(-) diff --git a/src/unix/apple/disk.rs b/src/unix/apple/disk.rs index 50cb65b97..54e6ff327 100644 --- a/src/unix/apple/disk.rs +++ b/src/unix/apple/disk.rs @@ -73,7 +73,7 @@ impl DiskInner { } pub(crate) fn refresh_specifics(&mut self, refresh_kind: DiskRefreshKind) -> bool { - if refresh_kind.kind() { + if refresh_kind.kind() && self.type_ == DiskKind::Unknown(-1) { let type_ = { #[cfg(target_os = "macos")] { diff --git a/src/unix/linux/disk.rs b/src/unix/linux/disk.rs index e481b5385..8318a8c4d 100644 --- a/src/unix/linux/disk.rs +++ b/src/unix/linux/disk.rs @@ -94,6 +94,10 @@ impl DiskInner { refresh_kind: DiskRefreshKind, procfs_disk_stats: &HashMap, ) -> bool { + if refresh_kind.kind() && self.type_ == DiskKind::Unknown(-1) { + self.type_ = find_type_for_device_name(&self.device_name); + } + if refresh_kind.io_usage() { let (read_bytes, written_bytes) = if let Some((read_bytes, written_bytes)) = procfs_disk_stats.get(&self.actual_device_name).map(|stat| { diff --git a/src/windows/disk.rs b/src/windows/disk.rs index a4ea8bf87..f5f4af1c8 100644 --- a/src/windows/disk.rs +++ b/src/windows/disk.rs @@ -167,26 +167,35 @@ impl DiskInner { self.is_read_only } - pub(crate) fn refresh_specifics(&mut self, _refreshes: DiskRefreshKind) -> bool { - let Some((read_bytes, written_bytes)) = get_disk_io(&self.device_path, None) else { - sysinfo_debug!("Failed to update disk i/o stats"); - return false; - }; + 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.old_read_bytes = self.read_bytes; - self.old_written_bytes = self.written_bytes; - self.read_bytes = read_bytes; - self.written_bytes = written_bytes; + if refreshes.io_usage() { + let Some((read_bytes, written_bytes)) = get_disk_io(&self.device_path, None) else { + sysinfo_debug!("Failed to update disk i/o stats"); + return false; + }; - if self.total_space != 0 { - unsafe { + self.old_read_bytes = self.read_bytes; + self.old_written_bytes = self.written_bytes; + self.read_bytes = read_bytes; + self.written_bytes = written_bytes; + } + + if refreshes.details() && self.total_space != 0 { + let available_space = unsafe { let mut tmp = 0; let lpdirectoryname = PCWSTR::from_raw(self.mount_point.as_ptr()); if GetDiskFreeSpaceExW(lpdirectoryname, None, None, Some(&mut tmp)).is_ok() { - self.available_space = tmp; - return true; + tmp + } else { + 0 } - } + }; + + self.available_space = available_space; } false } @@ -220,9 +229,9 @@ impl DisksInner { self.disks } - pub(crate) fn refresh_list_specifics(&mut self, _refreshes: DiskRefreshKind) { + pub(crate) fn refresh_list_specifics(&mut self, refreshes: DiskRefreshKind) { unsafe { - self.disks = get_list(); + self.disks = get_list(refreshes); } } @@ -259,7 +268,7 @@ unsafe fn get_drive_size(mount_point: &[u16]) -> Option<(u64, u64)> { } } -pub(crate) unsafe fn get_list() -> Vec { +pub(crate) unsafe fn get_list(refreshes: DiskRefreshKind) -> Vec { #[cfg(feature = "multithread")] use rayon::iter::ParallelIterator; @@ -268,7 +277,11 @@ pub(crate) unsafe fn get_list() -> Vec { let raw_volume_name = PCWSTR::from_raw(volume_name.as_ptr()); let drive_type = GetDriveTypeW(raw_volume_name); - let is_removable = drive_type == DRIVE_REMOVABLE; + let is_removable = if refreshes.details() { + drive_type == DRIVE_REMOVABLE + } else { + false + }; if drive_type != DRIVE_FIXED && drive_type != DRIVE_REMOVABLE { return Vec::new(); @@ -292,7 +305,11 @@ pub(crate) unsafe fn get_list() -> Vec { ); return Vec::new(); } - let is_read_only = (flags & FILE_READ_ONLY_VOLUME) != 0; + let is_read_only = if refreshes.details() { + (flags & FILE_READ_ONLY_VOLUME) != 0 + } else { + false + }; let mount_paths = get_volume_path_names_for_volume_name(&volume_name[..]); if mount_paths.is_empty() { @@ -305,51 +322,25 @@ pub(crate) unsafe fn get_list() -> Vec { .copied() .chain([0]) .collect::>(); - let Some(handle) = HandleWrapper::new_from_file(&device_path[..], Default::default()) - else { - return Vec::new(); - }; - let Some((total_space, available_space)) = get_drive_size(&mount_paths[0][..]) else { - return Vec::new(); - }; - if total_space == 0 { - sysinfo_debug!("total_space == 0"); - return Vec::new(); - } - let spq_trim = STORAGE_PROPERTY_QUERY { - PropertyId: StorageDeviceSeekPenaltyProperty, - QueryType: PropertyStandardQuery, - AdditionalParameters: [0], + let handle = HandleWrapper::new_from_file(&device_path[..], Default::default()); + + let (total_space, available_space) = if refreshes.details() { + get_drive_size(&mount_paths[0][..]).unwrap_or_default() + } else { + (0, 0) }; - let mut result: DEVICE_SEEK_PENALTY_DESCRIPTOR = std::mem::zeroed(); - - let mut dw_size = 0; - let device_io_control = 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(&mut dw_size), - None, - ) - .is_ok(); - let type_ = if !device_io_control - || dw_size != size_of::() as u32 - { - DiskKind::Unknown(-1) + + let type_ = if refreshes.kind() { + get_disk_kind(&device_path, &handle) } else { - let is_hdd = result.IncursSeekPenalty.as_bool(); - if is_hdd { - DiskKind::HDD - } else { - DiskKind::SSD - } + DiskKind::Unknown(-1) }; - let (read_bytes, written_bytes) = - get_disk_io(&device_path, Some(handle)).unwrap_or_default(); + let (read_bytes, written_bytes) = if refreshes.io_usage() { + get_disk_io(&device_path, handle).unwrap_or_default() + } else { + (0, 0) + }; let name = os_string_from_zero_terminated(&name); let file_system = os_string_from_zero_terminated(&file_system); @@ -383,6 +374,63 @@ fn os_string_from_zero_terminated(name: &[u16]) -> OsString { OsString::from_wide(&name[..len]) } +fn get_disk_kind(device_path: &[u16], borrowed_handle: &Option) -> 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(ref handle), _) => handle, + (_, Some(ref handle)) => handle, + (None, None) => return DiskKind::Unknown(-1), + }; + + if handle.is_invalid() { + sysinfo_debug!( + "Expected handle to '{:?}' to be valid", + String::from_utf16_lossy(device_path) + ); + return DiskKind::Unknown(-1); + } + + let spq_trim = STORAGE_PROPERTY_QUERY { + PropertyId: StorageDeviceSeekPenaltyProperty, + QueryType: PropertyStandardQuery, + AdditionalParameters: [0], + }; + let mut result: DEVICE_SEEK_PENALTY_DESCRIPTOR = unsafe { std::mem::zeroed() }; + + let mut dw_size = 0; + let device_io_control = unsafe { + 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(&mut dw_size), + None, + ) + .is_ok() + }; + + if !device_io_control || dw_size != size_of::() as u32 { + DiskKind::Unknown(-1) + } else { + let is_hdd = result.IncursSeekPenalty.as_bool(); + if is_hdd { + DiskKind::HDD + } else { + DiskKind::SSD + } + } +} + /// Returns a tuple consisting of the total number of bytes read and written by the volume with the specified device path fn get_disk_io(device_path: &[u16], handle: Option) -> Option<(u64, u64)> { let handle = diff --git a/tests/disk.rs b/tests/disk.rs index d209fdf3c..92e167d1d 100644 --- a/tests/disk.rs +++ b/tests/disk.rs @@ -53,20 +53,20 @@ fn test_disk_refresh_kind() { refreshes = f(refreshes); } - let assertions = |disks: &Disks| { + let assertions = |name: &'static str, disks: &Disks| { for disk in disks.list().iter() { if refreshes.kind() { #[cfg(not(target_os = "freebsd"))] assert_ne!( disk.kind(), DiskKind::Unknown(-1), - "disk.kind should be refreshed" + "{name}: disk.kind should be refreshed" ); } else { assert_eq!( disk.kind(), DiskKind::Unknown(-1), - "disk.kind should not be refreshed" + "{name}: disk.kind should not be refreshed" ); } @@ -74,12 +74,12 @@ fn test_disk_refresh_kind() { assert_ne!( disk.available_space(), Default::default(), - "disk.available_space should be refreshed" + "{name}: disk.available_space should be refreshed" ); assert_ne!( disk.total_space(), Default::default(), - "disk.total_space should be refreshed" + "{name}: disk.total_space should be refreshed" ); // We can't assert anything about booleans, since false is indistinguishable from // not-refreshed @@ -87,22 +87,22 @@ fn test_disk_refresh_kind() { assert_eq!( disk.available_space(), Default::default(), - "disk.available_space should not be refreshed" + "{name}: disk.available_space should not be refreshed" ); assert_eq!( disk.total_space(), Default::default(), - "disk.total_space should not be refreshed" + "{name}: disk.total_space should not be refreshed" ); assert_eq!( disk.is_read_only(), ::default(), - "disk.is_read_only should not be refreshed" + "{name}: disk.is_read_only should not be refreshed" ); assert_eq!( disk.is_removable(), ::default(), - "disk.is_removable should not be refreshed" + "{name}: disk.is_removable should not be refreshed" ); } @@ -110,13 +110,13 @@ fn test_disk_refresh_kind() { assert_ne!( disk.usage(), Default::default(), - "disk.usage should be refreshed" + "{name}: disk.usage should be refreshed" ); } else { assert_eq!( disk.usage(), Default::default(), - "disk.usage should not be refreshed" + "{name}: disk.usage should not be refreshed" ); } } @@ -124,12 +124,12 @@ fn test_disk_refresh_kind() { // load and refresh with the desired details should work let disks = Disks::new_with_refreshed_list_specifics(refreshes); - assertions(&disks); + assertions("full", &disks); // load with minimal `DiskRefreshKind`, then refresh for added detail should also work! let mut disks = Disks::new_with_refreshed_list_specifics(DiskRefreshKind::new()); disks.refresh_specifics(refreshes); - assertions(&disks); + assertions("incremental", &disks); } } From 446f155ee205e6ffb38ee826b593ec4994c84cec Mon Sep 17 00:00:00 2001 From: "J. David Lowe" Date: Tue, 26 Nov 2024 14:12:42 -0800 Subject: [PATCH 10/17] ?? --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 5efc08745..2a9fc5935 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -103,7 +103,7 @@ ntapi = { version = "0.4", optional = true } windows = { version = ">=0.54, <=0.57", optional = true } [target.'cfg(not(any(target_os = "unknown", target_arch = "wasm32")))'.dependencies] -libc = "^0.2.165" +libc = "^0.2.164" [target.'cfg(any(target_os = "macos", target_os = "ios"))'.dependencies] core-foundation-sys = "0.8.7" From 821bd0c98977728040441cae4e35698c165da26e Mon Sep 17 00:00:00 2001 From: "J. David Lowe" Date: Tue, 26 Nov 2024 15:08:39 -0800 Subject: [PATCH 11/17] more review feedback --- src/unix/linux/disk.rs | 37 ++++++++++++++++++------------------- src/windows/disk.rs | 12 ++++++------ tests/disk.rs | 7 +++---- 3 files changed, 27 insertions(+), 29 deletions(-) diff --git a/src/unix/linux/disk.rs b/src/unix/linux/disk.rs index 8318a8c4d..df143af3f 100644 --- a/src/unix/linux/disk.rs +++ b/src/unix/linux/disk.rs @@ -37,9 +37,7 @@ macro_rules! cast { pub(crate) struct DiskInner { type_: DiskKind, device_name: OsString, - // Potential future surprise: right now, this field is only needed by usage-related code, - // so it is only populated if DiskRefreshKind::usage() is true. - actual_device_name: String, + actual_device_name: Option, file_system: OsString, mount_point: PathBuf, total_space: u64, @@ -99,13 +97,18 @@ impl DiskInner { } if refresh_kind.io_usage() { + if self.actual_device_name.is_none() { + self.actual_device_name = Some(get_actual_device_name(&self.device_name)); + } let (read_bytes, written_bytes) = if let Some((read_bytes, written_bytes)) = - procfs_disk_stats.get(&self.actual_device_name).map(|stat| { - ( - stat.sectors_read * SECTOR_SIZE, - stat.sectors_written * SECTOR_SIZE, - ) - }) { + procfs_disk_stats + .get(self.actual_device_name.as_ref().unwrap()) + .map(|stat| { + ( + stat.sectors_read * SECTOR_SIZE, + stat.sectors_written * SECTOR_SIZE, + ) + }) { (read_bytes, written_bytes) } else { sysinfo_debug!("Failed to update disk i/o stats"); @@ -252,14 +255,9 @@ fn new_disk( false }; - let actual_device_name = if refresh_kind.io_usage() { - get_actual_device_name(device_name) - } else { - String::new() - }; - - let (read_bytes, written_bytes) = if refresh_kind.io_usage() { - procfs_disk_stats + let (actual_device_name, read_bytes, written_bytes) = if refresh_kind.io_usage() { + let actual_device_name = get_actual_device_name(device_name); + let (read_bytes, written_bytes) = procfs_disk_stats .get(&actual_device_name) .map(|stat| { ( @@ -267,9 +265,10 @@ fn new_disk( stat.sectors_written * SECTOR_SIZE, ) }) - .unwrap_or_default() + .unwrap_or_default(); + (Some(actual_device_name), read_bytes, written_bytes) } else { - (0, 0) + (None, 0, 0) }; Disk { diff --git a/src/windows/disk.rs b/src/windows/disk.rs index f5f4af1c8..22afcbb93 100644 --- a/src/windows/disk.rs +++ b/src/windows/disk.rs @@ -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_ = get_disk_kind(&self.device_path, None); } if refreshes.io_usage() { @@ -331,7 +331,7 @@ pub(crate) unsafe fn get_list(refreshes: DiskRefreshKind) -> Vec { }; let type_ = if refreshes.kind() { - get_disk_kind(&device_path, &handle) + get_disk_kind(&device_path, handle.as_ref()) } else { DiskKind::Unknown(-1) }; @@ -374,7 +374,7 @@ fn os_string_from_zero_terminated(name: &[u16]) -> OsString { OsString::from_wide(&name[..len]) } -fn get_disk_kind(device_path: &[u16], borrowed_handle: &Option) -> DiskKind { +fn get_disk_kind(device_path: &[u16], borrowed_handle: Option<&HandleWrapper>) -> DiskKind { let binding = ( borrowed_handle, if borrowed_handle.is_none() { @@ -384,14 +384,14 @@ fn get_disk_kind(device_path: &[u16], borrowed_handle: &Option) - }, ); let handle = match binding { - (Some(ref handle), _) => handle, + (Some(handle), _) => handle, (_, Some(ref handle)) => handle, (None, None) => return DiskKind::Unknown(-1), }; if handle.is_invalid() { sysinfo_debug!( - "Expected handle to '{:?}' to be valid", + "Expected handle to {:?} to be valid", String::from_utf16_lossy(device_path) ); return DiskKind::Unknown(-1); @@ -438,7 +438,7 @@ fn get_disk_io(device_path: &[u16], handle: Option) -> Option<(u6 if handle.is_invalid() { sysinfo_debug!( - "Expected handle to '{:?}' to be valid", + "Expected handle to {:?} to be valid", String::from_utf16_lossy(device_path) ); return None; diff --git a/tests/disk.rs b/tests/disk.rs index 92e167d1d..e0fde2e91 100644 --- a/tests/disk.rs +++ b/tests/disk.rs @@ -7,10 +7,9 @@ fn should_skip() -> bool { } let s = sysinfo::System::new_all(); - if s.physical_core_count().unwrap_or_default() == 0 { - return true; - } - false + + // If we don't have any physical core present, it's very likely that we're inside a VM... + s.physical_core_count().unwrap_or_default() == 0 } #[test] From 51b701727b5ebcbade0d81cc30abe0c9bcc8687f Mon Sep 17 00:00:00 2001 From: "J. David Lowe" Date: Tue, 26 Nov 2024 15:50:53 -0800 Subject: [PATCH 12/17] relax the refreshed test conditions --- src/unix/linux/disk.rs | 7 ++- tests/disk.rs | 140 ++++++++++++++++++++++------------------- 2 files changed, 80 insertions(+), 67 deletions(-) diff --git a/src/unix/linux/disk.rs b/src/unix/linux/disk.rs index df143af3f..8f1a14f25 100644 --- a/src/unix/linux/disk.rs +++ b/src/unix/linux/disk.rs @@ -239,13 +239,16 @@ fn new_disk( DiskKind::Unknown(-1) }; - let (total_space, available_space, is_read_only) = + let (total_space, available_space, is_read_only) = if refresh_kind.details() { match unsafe { load_statvfs_values(mount_point, refresh_kind) } { Some((total_space, available_space, is_read_only)) => { (total_space, available_space, is_read_only) } None => (0, 0, false), - }; + } + } else { + (0, 0, false) + }; let is_removable = if refresh_kind.details() { removable_entries diff --git a/tests/disk.rs b/tests/disk.rs index e0fde2e91..cf9910f3a 100644 --- a/tests/disk.rs +++ b/tests/disk.rs @@ -53,71 +53,81 @@ fn test_disk_refresh_kind() { } let assertions = |name: &'static str, disks: &Disks| { - for disk in disks.list().iter() { - if refreshes.kind() { - #[cfg(not(target_os = "freebsd"))] - assert_ne!( - disk.kind(), - DiskKind::Unknown(-1), - "{name}: disk.kind should be refreshed" - ); - } else { - assert_eq!( - disk.kind(), - DiskKind::Unknown(-1), - "{name}: disk.kind should not be refreshed" - ); - } - - if refreshes.details() { - assert_ne!( - disk.available_space(), - Default::default(), - "{name}: disk.available_space should be refreshed" - ); - assert_ne!( - disk.total_space(), - Default::default(), - "{name}: disk.total_space should be refreshed" - ); - // We can't assert anything about booleans, since false is indistinguishable from - // not-refreshed - } else { - assert_eq!( - disk.available_space(), - Default::default(), - "{name}: disk.available_space should not be refreshed" - ); - assert_eq!( - disk.total_space(), - Default::default(), - "{name}: disk.total_space should not be refreshed" - ); - assert_eq!( - disk.is_read_only(), - ::default(), - "{name}: disk.is_read_only should not be refreshed" - ); - assert_eq!( - disk.is_removable(), - ::default(), - "{name}: disk.is_removable should not be refreshed" - ); - } - - if refreshes.io_usage() { - assert_ne!( - disk.usage(), - Default::default(), - "{name}: disk.usage should be refreshed" - ); - } else { - assert_eq!( - disk.usage(), - Default::default(), - "{name}: disk.usage should not be refreshed" - ); - } + if refreshes.kind() { + // This would ideally assert that *all* are refreshed, but we settle for a weaker + // assertion because failures can't be distinguished from "not refreshed" values. + #[cfg(not(target_os = "freebsd"))] + assert!( + disks + .iter() + .any(|disk| disk.kind() != DiskKind::Unknown(-1)), + "{name}: disk.kind should be refreshed" + ); + } else { + assert!( + disks + .iter() + .all(|disk| disk.kind() == DiskKind::Unknown(-1)), + "{name}: disk.kind should not be refreshed" + ); + } + + if refreshes.details() { + // These would ideally assert that *all* are refreshed, but we settle for a weaker + // assertion because failures can't be distinguished from "not refreshed" values. + assert!( + disks + .iter() + .any(|disk| disk.available_space() != Default::default()), + "{name}: disk.available_space should be refreshed" + ); + assert!( + disks + .iter() + .any(|disk| disk.total_space() != Default::default()), + "{name}: disk.total_space should be refreshed" + ); + // We can't assert anything about booleans, since false is indistinguishable from + // not-refreshed + } else { + assert!( + disks + .iter() + .all(|disk| disk.available_space() == Default::default()), + "{name}: disk.available_space should not be refreshed" + ); + assert!( + disks + .iter() + .all(|disk| disk.total_space() == Default::default()), + "{name}: disk.total_space should not be refreshed" + ); + assert!( + disks + .iter() + .all(|disk| disk.is_read_only() == ::default()), + "{name}: disk.is_read_only should not be refreshed" + ); + assert!( + disks + .iter() + .all(|disk| disk.is_removable() == ::default()), + "{name}: disk.is_removable should not be refreshed" + ); + } + + if refreshes.io_usage() { + // This would ideally assert that *all* are refreshed, but we settle for a weaker + // assertion because failures can't be distinguished from "not refreshed" values. + assert!( + disks.iter().any(|disk| disk.usage() != Default::default()), + "{name}: disk.usage should be refreshed" + ); + } else { + assert!( + disks.iter().all(|disk| disk.usage() == Default::default()), + "{name}: disk.usage should not be refreshed" + ); } }; From 29381b7af7f0d479e5c113fb6ce22edfd7f09893 Mon Sep 17 00:00:00 2001 From: "J. David Lowe" Date: Tue, 26 Nov 2024 16:44:40 -0800 Subject: [PATCH 13/17] what's up with windows? --- src/windows/disk.rs | 15 ++++++--------- tests/disk.rs | 2 +- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/windows/disk.rs b/src/windows/disk.rs index 22afcbb93..9588cac15 100644 --- a/src/windows/disk.rs +++ b/src/windows/disk.rs @@ -184,17 +184,14 @@ impl DiskInner { self.written_bytes = written_bytes; } - if refreshes.details() && self.total_space != 0 { - let available_space = unsafe { - let mut tmp = 0; - let lpdirectoryname = PCWSTR::from_raw(self.mount_point.as_ptr()); - if GetDiskFreeSpaceExW(lpdirectoryname, None, None, Some(&mut tmp)).is_ok() { - tmp - } else { - 0 - } + if refreshes.details() { + let (total_space, available_space) = if refreshes.details() { + unsafe { get_drive_size(&self.mount_point).unwrap_or_default() } + } else { + (0, 0) }; + self.total_space = total_space; self.available_space = available_space; } false diff --git a/tests/disk.rs b/tests/disk.rs index cf9910f3a..2fbab2ba3 100644 --- a/tests/disk.rs +++ b/tests/disk.rs @@ -56,7 +56,7 @@ fn test_disk_refresh_kind() { if refreshes.kind() { // This would ideally assert that *all* are refreshed, but we settle for a weaker // assertion because failures can't be distinguished from "not refreshed" values. - #[cfg(not(target_os = "freebsd"))] + #[cfg(not(any(target_os = "freebsd", target_os = "windows")))] assert!( disks .iter() From fef139398c71da5d0981dac4030388400762f715 Mon Sep 17 00:00:00 2001 From: "J. David Lowe" Date: Wed, 27 Nov 2024 10:50:41 -0800 Subject: [PATCH 14/17] early return instead of indenting everything --- src/unix/linux/disk.rs | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/unix/linux/disk.rs b/src/unix/linux/disk.rs index 8f1a14f25..be5878200 100644 --- a/src/unix/linux/disk.rs +++ b/src/unix/linux/disk.rs @@ -201,27 +201,27 @@ unsafe fn load_statvfs_values( mount_point: &Path, refresh_kind: DiskRefreshKind, ) -> Option<(u64, u64, bool)> { - if refresh_kind.details() { - let mount_point_cpath = to_cpath(mount_point); - let mut stat: statvfs = mem::zeroed(); - if retry_eintr!(statvfs(mount_point_cpath.as_ptr() as *const _, &mut stat)) == 0 { - let bsize = cast!(stat.f_bsize); - let blocks = cast!(stat.f_blocks); - let bavail = cast!(stat.f_bavail); - let total = bsize.saturating_mul(blocks); - let available = bsize.saturating_mul(bavail); - let is_read_only = (stat.f_flag & libc::ST_RDONLY) != 0; - - if total == 0 { - None - } else { - Some((total, available, is_read_only)) - } - } else { + if !refresh_kind.details() { + return Some((0, 0, false)); + } + + let mount_point_cpath = to_cpath(mount_point); + let mut stat: statvfs = mem::zeroed(); + if retry_eintr!(statvfs(mount_point_cpath.as_ptr() as *const _, &mut stat)) == 0 { + let bsize = cast!(stat.f_bsize); + let blocks = cast!(stat.f_blocks); + let bavail = cast!(stat.f_bavail); + let total = bsize.saturating_mul(blocks); + let available = bsize.saturating_mul(bavail); + let is_read_only = (stat.f_flag & libc::ST_RDONLY) != 0; + + if total == 0 { None + } else { + Some((total, available, is_read_only)) } } else { - Some((0, 0, false)) + None } } From 78f0362ba2a20c9cbd4b04a750bb47f7db297321 Mon Sep 17 00:00:00 2001 From: "J. David Lowe" Date: Wed, 27 Nov 2024 11:18:06 -0800 Subject: [PATCH 15/17] review feedback --- src/unix/linux/disk.rs | 41 ++++++++++++----------------------------- src/windows/disk.rs | 41 +++++++++++++++-------------------------- 2 files changed, 27 insertions(+), 55 deletions(-) diff --git a/src/unix/linux/disk.rs b/src/unix/linux/disk.rs index be5878200..9e2013069 100644 --- a/src/unix/linux/disk.rs +++ b/src/unix/linux/disk.rs @@ -122,15 +122,13 @@ impl DiskInner { } if refresh_kind.details() { - let (total_space, available_space, is_read_only) = - match unsafe { load_statvfs_values(&self.mount_point, refresh_kind) } { - Some((total, available, is_read_only)) => (total, available, is_read_only), - None => (0, 0, false), - }; - - self.total_space = total_space; - self.available_space = available_space; - self.is_read_only = is_read_only; + if let Some((total_space, available_space, is_read_only)) = + unsafe { load_statvfs_values(&self.mount_point) } + { + self.total_space = total_space; + self.available_space = available_space; + self.is_read_only = is_read_only; + } } true @@ -197,14 +195,7 @@ fn get_actual_device_name(device: &OsStr) -> String { .unwrap_or_default() } -unsafe fn load_statvfs_values( - mount_point: &Path, - refresh_kind: DiskRefreshKind, -) -> Option<(u64, u64, bool)> { - if !refresh_kind.details() { - return Some((0, 0, false)); - } - +unsafe fn load_statvfs_values(mount_point: &Path) -> Option<(u64, u64, bool)> { let mount_point_cpath = to_cpath(mount_point); let mut stat: statvfs = mem::zeroed(); if retry_eintr!(statvfs(mount_point_cpath.as_ptr() as *const _, &mut stat)) == 0 { @@ -240,23 +231,15 @@ fn new_disk( }; let (total_space, available_space, is_read_only) = if refresh_kind.details() { - match unsafe { load_statvfs_values(mount_point, refresh_kind) } { - Some((total_space, available_space, is_read_only)) => { - (total_space, available_space, is_read_only) - } - None => (0, 0, false), - } + unsafe { load_statvfs_values(mount_point).unwrap_or((0, 0, false)) } } else { (0, 0, false) }; - let is_removable = if refresh_kind.details() { - removable_entries + let is_removable = refresh_kind.details() + && removable_entries .iter() - .any(|e| e.as_os_str() == device_name) - } else { - false - }; + .any(|e| e.as_os_str() == device_name); let (actual_device_name, read_bytes, written_bytes) = if refresh_kind.io_usage() { let actual_device_name = get_actual_device_name(device_name); diff --git a/src/windows/disk.rs b/src/windows/disk.rs index 9588cac15..f9efc6dfb 100644 --- a/src/windows/disk.rs +++ b/src/windows/disk.rs @@ -173,26 +173,23 @@ impl DiskInner { } if refreshes.io_usage() { - let Some((read_bytes, written_bytes)) = get_disk_io(&self.device_path, None) else { + if let Some((read_bytes, written_bytes)) = get_disk_io(&self.device_path, None) { + self.old_read_bytes = self.read_bytes; + self.old_written_bytes = self.written_bytes; + self.read_bytes = read_bytes; + self.written_bytes = written_bytes; + } else { sysinfo_debug!("Failed to update disk i/o stats"); - return false; - }; - - self.old_read_bytes = self.read_bytes; - self.old_written_bytes = self.written_bytes; - self.read_bytes = read_bytes; - self.written_bytes = written_bytes; + } } if refreshes.details() { - let (total_space, available_space) = if refreshes.details() { - unsafe { get_drive_size(&self.mount_point).unwrap_or_default() } - } else { - (0, 0) - }; - - self.total_space = total_space; - self.available_space = available_space; + if let Some((total_space, available_space)) = + unsafe { get_drive_size(&self.mount_point) } + { + self.total_space = total_space; + self.available_space = available_space; + } } false } @@ -274,11 +271,7 @@ pub(crate) unsafe fn get_list(refreshes: DiskRefreshKind) -> Vec { let raw_volume_name = PCWSTR::from_raw(volume_name.as_ptr()); let drive_type = GetDriveTypeW(raw_volume_name); - let is_removable = if refreshes.details() { - drive_type == DRIVE_REMOVABLE - } else { - false - }; + let is_removable = refreshes.details() && drive_type == DRIVE_REMOVABLE; if drive_type != DRIVE_FIXED && drive_type != DRIVE_REMOVABLE { return Vec::new(); @@ -302,11 +295,7 @@ pub(crate) unsafe fn get_list(refreshes: DiskRefreshKind) -> Vec { ); return Vec::new(); } - let is_read_only = if refreshes.details() { - (flags & FILE_READ_ONLY_VOLUME) != 0 - } else { - false - }; + let is_read_only = refreshes.details() && (flags & FILE_READ_ONLY_VOLUME) != 0; let mount_paths = get_volume_path_names_for_volume_name(&volume_name[..]); if mount_paths.is_empty() { From 507813665a61cbd80860300644de870d92b35af8 Mon Sep 17 00:00:00 2001 From: "J. David Lowe" Date: Wed, 27 Nov 2024 12:31:38 -0800 Subject: [PATCH 16/17] more feedback --- src/unix/apple/disk.rs | 6 +----- src/unix/freebsd/disk.rs | 16 +++++++++------- src/unix/linux/disk.rs | 31 ++++++++++++++----------------- 3 files changed, 24 insertions(+), 29 deletions(-) diff --git a/src/unix/apple/disk.rs b/src/unix/apple/disk.rs index 54e6ff327..6967c5b5a 100644 --- a/src/unix/apple/disk.rs +++ b/src/unix/apple/disk.rs @@ -538,11 +538,7 @@ unsafe fn new_disk( ) }; - let is_read_only = if refresh_kind.details() { - (c_disk.f_flags & libc::MNT_RDONLY as u32) != 0 - } else { - false - }; + let is_read_only = refresh_kind.details() && (c_disk.f_flags & libc::MNT_RDONLY as u32) != 0; Some(Disk { inner: DiskInner { diff --git a/src/unix/freebsd/disk.rs b/src/unix/freebsd/disk.rs index 64c268538..edd79d815 100644 --- a/src/unix/freebsd/disk.rs +++ b/src/unix/freebsd/disk.rs @@ -167,14 +167,16 @@ fn refresh_disk(disk: &mut DiskInner, refresh_kind: DiskRefreshKind) -> bool { let (total_space, available_space) = 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 { - return false; - } - let block_size: u64 = vfs.f_frsize as _; + sysinfo_debug!("statvfs failed"); + (0, 0) + } else { + let block_size: u64 = vfs.f_frsize as _; - ( - vfs.f_blocks.saturating_mul(block_size), - vfs.f_favail.saturating_mul(block_size), - ) + ( + vfs.f_blocks.saturating_mul(block_size), + vfs.f_favail.saturating_mul(block_size), + ) + } }; disk.total_space = total_space; diff --git a/src/unix/linux/disk.rs b/src/unix/linux/disk.rs index 9e2013069..ece4d6943 100644 --- a/src/unix/linux/disk.rs +++ b/src/unix/linux/disk.rs @@ -100,25 +100,22 @@ impl DiskInner { if self.actual_device_name.is_none() { self.actual_device_name = Some(get_actual_device_name(&self.device_name)); } - let (read_bytes, written_bytes) = 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, - ) - }) { - (read_bytes, written_bytes) + 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, + ) + }) + { + self.old_read_bytes = self.read_bytes; + self.old_written_bytes = self.written_bytes; + self.read_bytes = read_bytes; + self.written_bytes = written_bytes; } else { sysinfo_debug!("Failed to update disk i/o stats"); - (0, 0) - }; - - self.old_read_bytes = self.read_bytes; - self.old_written_bytes = self.written_bytes; - self.read_bytes = read_bytes; - self.written_bytes = written_bytes; + } } if refresh_kind.details() { From a5cc1180219292148e7f27364c117f45b621f69b Mon Sep 17 00:00:00 2001 From: "J. David Lowe" Date: Wed, 27 Nov 2024 15:57:34 -0800 Subject: [PATCH 17/17] more review feedback --- src/unix/apple/disk.rs | 56 ++++++++++++++++++---------------------- src/unix/freebsd/disk.rs | 15 +++-------- src/unix/linux/disk.rs | 9 +++---- 3 files changed, 33 insertions(+), 47 deletions(-) diff --git a/src/unix/apple/disk.rs b/src/unix/apple/disk.rs index 6967c5b5a..e2f5e28cf 100644 --- a/src/unix/apple/disk.rs +++ b/src/unix/apple/disk.rs @@ -90,56 +90,49 @@ impl DiskInner { } if refresh_kind.io_usage() { - let (read_bytes, written_bytes) = { - #[cfg(target_os = "macos")] - { - self.bsd_name - .as_ref() - .and_then(|name| crate::sys::inner::disk::get_disk_io(name)) - .unwrap_or_else(|| { - sysinfo_debug!("Failed to update disk i/o stats"); - (0, 0) - }) + #[cfg(target_os = "macos")] + match self + .bsd_name + .as_ref() + .and_then(|name| crate::sys::inner::disk::get_disk_io(name)) + { + Some((read_bytes, written_bytes)) => { + self.old_read_bytes = self.read_bytes; + self.old_written_bytes = self.written_bytes; + self.read_bytes = read_bytes; + self.written_bytes = written_bytes; } - #[cfg(not(target_os = "macos"))] - (0, 0) - }; - - self.old_read_bytes = self.read_bytes; - self.old_written_bytes = self.written_bytes; - self.read_bytes = read_bytes; - self.written_bytes = written_bytes; + None => { + sysinfo_debug!("Failed to update disk i/o stats"); + } + } } if refresh_kind.details() { - let (total_space, available_space) = unsafe { + unsafe { if let Some(requested_properties) = build_requested_properties(&[ ffi::kCFURLVolumeTotalCapacityKey, ffi::kCFURLVolumeAvailableCapacityKey, ffi::kCFURLVolumeAvailableCapacityForImportantUsageKey, ]) { match get_disk_properties(&self.volume_url, &requested_properties) { - Some(disk_props) => ( - get_int_value( + Some(disk_props) => { + self.total_space = get_int_value( disk_props.inner(), DictKey::Extern(ffi::kCFURLVolumeTotalCapacityKey), ) - .unwrap_or_default() as u64, - get_available_volume_space(&disk_props), - ), + .unwrap_or_default() + as u64; + self.available_space = get_available_volume_space(&disk_props); + } None => { sysinfo_debug!("Failed to get disk properties"); - (0, 0) } } } else { sysinfo_debug!("failed to create volume key list, skipping refresh"); - (0, 0) } - }; - - self.total_space = total_space; - self.available_space = available_space; + } } true @@ -513,7 +506,8 @@ unsafe fn new_disk( get_int_value( disk_props.inner(), DictKey::Extern(ffi::kCFURLVolumeTotalCapacityKey), - )? as u64 + ) + .unwrap_or_default() as u64 } else { 0 }; diff --git a/src/unix/freebsd/disk.rs b/src/unix/freebsd/disk.rs index edd79d815..b273ce937 100644 --- a/src/unix/freebsd/disk.rs +++ b/src/unix/freebsd/disk.rs @@ -164,23 +164,16 @@ impl GetValues for DiskInner { fn refresh_disk(disk: &mut DiskInner, refresh_kind: DiskRefreshKind) -> bool { if refresh_kind.details() { - let (total_space, available_space) = unsafe { + 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"); - (0, 0) } else { let block_size: u64 = vfs.f_frsize as _; - - ( - vfs.f_blocks.saturating_mul(block_size), - vfs.f_favail.saturating_mul(block_size), - ) + disk.total_space = vfs.f_blocks.saturating_mul(block_size); + disk.available_space = vfs.f_favail.saturating_mul(block_size); } - }; - - disk.total_space = total_space; - disk.available_space = available_space; + } } if refresh_kind.io_usage() { diff --git a/src/unix/linux/disk.rs b/src/unix/linux/disk.rs index ece4d6943..f07b56a96 100644 --- a/src/unix/linux/disk.rs +++ b/src/unix/linux/disk.rs @@ -200,14 +200,13 @@ unsafe fn load_statvfs_values(mount_point: &Path) -> Option<(u64, u64, bool)> { let blocks = cast!(stat.f_blocks); let bavail = cast!(stat.f_bavail); let total = bsize.saturating_mul(blocks); + if total == 0 { + return None; + } let available = bsize.saturating_mul(bavail); let is_read_only = (stat.f_flag & libc::ST_RDONLY) != 0; - if total == 0 { - None - } else { - Some((total, available, is_read_only)) - } + Some((total, available, is_read_only)) } else { None }