From ea841efce3d2aaa80d9f9e6e60b90643c0839da6 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 30 Nov 2024 23:29:59 +0100 Subject: [PATCH] Remove `Disks::refresh_list` --- benches/basic.rs | 11 +------ examples/simple.rs | 2 +- src/c_interface.rs | 16 +--------- src/common/disk.rs | 67 ++++++---------------------------------- src/sysinfo.h | 2 -- src/unix/apple/disk.rs | 40 +++++++++--------------- src/unix/freebsd/disk.rs | 13 ++------ src/unix/linux/disk.rs | 44 +++++++++----------------- src/unknown/disk.rs | 6 +--- src/windows/disk.rs | 8 +---- tests/disk.rs | 6 ++-- 11 files changed, 51 insertions(+), 164 deletions(-) diff --git a/benches/basic.rs b/benches/basic.rs index e552ba17a..5148d2911 100644 --- a/benches/basic.rs +++ b/benches/basic.rs @@ -77,16 +77,7 @@ fn bench_refresh_disks(b: &mut test::Bencher) { let mut disks = sysinfo::Disks::new_with_refreshed_list(); b.iter(move || { - disks.refresh(); - }); -} - -#[bench] -fn bench_refresh_disks_list(b: &mut test::Bencher) { - let mut disks = sysinfo::Disks::new_with_refreshed_list(); - - b.iter(move || { - disks.refresh_list(true); + disks.refresh(true); }); } diff --git a/examples/simple.rs b/examples/simple.rs index f848f0c05..ee2250481 100644 --- a/examples/simple.rs +++ b/examples/simple.rs @@ -158,7 +158,7 @@ fn interpret_input( "help" => print_help(), "refresh_disks" => { writeln!(&mut io::stdout(), "Refreshing disk list..."); - disks.refresh_list(true); + disks.refresh(true); writeln!(&mut io::stdout(), "Done."); } "refresh_users" => { diff --git a/src/c_interface.rs b/src/c_interface.rs index ae771606f..eb4d13130 100644 --- a/src/c_interface.rs +++ b/src/c_interface.rs @@ -142,21 +142,7 @@ pub extern "C" fn sysinfo_disks_refresh(disks: CDisks) { let mut disks: Box = Box::from_raw(disks as *mut Disks); { let disks: &mut Disks = disks.borrow_mut(); - disks.refresh(); - } - let _ = Box::into_raw(disks); - } -} - -/// Equivalent of [`Disks::refresh_list()`][crate::Disks#method.refresh_list]. -#[no_mangle] -pub extern "C" fn sysinfo_disks_refresh_list(disks: CDisks) { - assert!(!disks.is_null()); - unsafe { - let mut disks: Box = Box::from_raw(disks as *mut Disks); - { - let disks: &mut Disks = disks.borrow_mut(); - disks.refresh_list(true); + disks.refresh(true); } let _ = Box::into_raw(disks); } diff --git a/src/common/disk.rs b/src/common/disk.rs index 04986f86b..83996da85 100644 --- a/src/common/disk.rs +++ b/src/common/disk.rs @@ -249,7 +249,7 @@ impl Disks { /// use sysinfo::Disks; /// /// let mut disks = Disks::new(); - /// disks.refresh_list(false); + /// disks.refresh(false); /// for disk in disks.list() { /// println!("{disk:?}"); /// } @@ -277,8 +277,7 @@ impl Disks { } /// 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`]. + /// and refreshed according to the given [`DiskRefreshKind`]. /// /// ```no_run /// use sysinfo::{Disks, DiskRefreshKind}; @@ -290,7 +289,7 @@ impl Disks { /// ``` pub fn new_with_refreshed_list_specifics(refreshes: DiskRefreshKind) -> Self { let mut disks = Self::new(); - disks.refresh_list_specifics(false, refreshes); + disks.refresh_specifics(false, refreshes); disks } @@ -326,69 +325,23 @@ impl Disks { /// Refreshes the listed disks' information. /// /// Equivalent to [Disks::refresh_specifics]\([DiskRefreshKind::everything]\()). - pub fn refresh(&mut self) { - self.refresh_specifics(DiskRefreshKind::everything()); + pub fn refresh(&mut self, remove_not_listed_disks: bool) { + self.inner + .refresh_specifics(remove_not_listed_disks, 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. - /// - /// ⚠️ If you didn't call [`Disks::refresh_list`] beforehand, this method will do nothing as - /// the disk list will be empty. + /// Refreshes the disks' information according to the given [`DiskRefreshKind`]. /// /// ```no_run /// use sysinfo::Disks; /// /// let mut disks = Disks::new_with_refreshed_list(); /// // We wait some time...? - /// disks.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(true); - /// ``` - pub fn refresh_list(&mut self, remove_not_listed_disks: bool) { - self.refresh_list_specifics(remove_not_listed_disks, 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_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, DiskRefreshKind}; - /// - /// let mut disks = Disks::new(); - /// disks.refresh_list_specifics(true, DiskRefreshKind::nothing()); + /// disks.refresh(true); /// ``` - pub fn refresh_list_specifics( - &mut self, - remove_not_listed_disks: bool, - refreshes: DiskRefreshKind, - ) { + pub fn refresh_specifics(&mut self, remove_not_listed_disks: bool, refreshes: DiskRefreshKind) { self.inner - .refresh_list_specifics(remove_not_listed_disks, refreshes); + .refresh_specifics(remove_not_listed_disks, refreshes); } } diff --git a/src/sysinfo.h b/src/sysinfo.h index 20a9832e3..01f6ce368 100644 --- a/src/sysinfo.h +++ b/src/sysinfo.h @@ -33,7 +33,6 @@ void sysinfo_refresh_process(CSystem system, PID pid); CDisks sysinfo_disks_init(void); void sysinfo_disks_destroy(CDisks disks); void sysinfo_disks_refresh(CDisks disks); -void sysinfo_disks_refresh_list(CDisks disks); size_t sysinfo_total_memory(CSystem system); size_t sysinfo_free_memory(CSystem system); @@ -57,7 +56,6 @@ size_t sysinfo_process_virtual_memory(CProcess process); RString sysinfo_process_executable_path(CProcess process); RString sysinfo_process_root_directory(CProcess process); RString sysinfo_process_current_directory(CProcess process); -void sysinfo_networks_refresh_list(CNetworks networks); void sysinfo_networks_refresh(CNetworks networks); size_t sysinfo_networks_received(CNetworks networks); size_t sysinfo_networks_transmitted(CNetworks networks); diff --git a/src/unix/apple/disk.rs b/src/unix/apple/disk.rs index 812f3637a..190140e19 100644 --- a/src/unix/apple/disk.rs +++ b/src/unix/apple/disk.rs @@ -178,7 +178,7 @@ impl crate::DisksInner { } } - pub(crate) fn refresh_list_specifics( + pub(crate) fn refresh_specifics( &mut self, remove_not_listed_disks: bool, refresh_kind: DiskRefreshKind, @@ -187,14 +187,22 @@ impl crate::DisksInner { // 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, remove_not_listed_disks, refresh_kind); + get_list(&mut self.disks, refresh_kind); }) } - } - pub(crate) fn refresh_specifics(&mut self, refresh_kind: DiskRefreshKind) { - for disk in self.list_mut() { - disk.refresh_specifics(refresh_kind); + if remove_not_listed_disks { + self.disks.retain_mut(|disk| { + if !disk.inner.updated { + return false; + } + disk.inner.updated = false; + true + }); + } else { + for c in self.disks.iter_mut() { + c.inner.updated = false; + } } } @@ -207,11 +215,7 @@ impl crate::DisksInner { } } -unsafe fn get_list( - container: &mut Vec, - remove_not_listed_disks: bool, - refresh_kind: DiskRefreshKind, -) { +unsafe fn get_list(container: &mut Vec, refresh_kind: DiskRefreshKind) { let raw_disks = { let count = libc::getfsstat(ptr::null_mut(), 0, libc::MNT_NOWAIT); if count < 1 { @@ -329,20 +333,6 @@ unsafe fn get_list( container.push(disk); } } - - if remove_not_listed_disks { - container.retain_mut(|disk| { - if !disk.inner.updated { - return false; - } - disk.inner.updated = false; - true - }); - } else { - for c in container.iter_mut() { - c.inner.updated = false; - } - } } type RetainedCFArray = CFReleaser; diff --git a/src/unix/freebsd/disk.rs b/src/unix/freebsd/disk.rs index 080434498..4aff58ee2 100644 --- a/src/unix/freebsd/disk.rs +++ b/src/unix/freebsd/disk.rs @@ -91,12 +91,12 @@ impl crate::DisksInner { } } - pub(crate) fn refresh_list_specifics( + pub(crate) fn refresh_specifics( &mut self, remove_not_listed_disks: bool, refresh_kind: DiskRefreshKind, ) { - unsafe { get_all_list(&mut self.disks, true, remove_not_listed_disks, refresh_kind) } + unsafe { get_all_list(&mut self.disks, remove_not_listed_disks, refresh_kind) } } pub(crate) fn list(&self) -> &[Disk] { @@ -106,12 +106,6 @@ impl crate::DisksInner { pub(crate) fn list_mut(&mut self) -> &mut [Disk] { &mut self.disks } - - pub(crate) fn refresh_specifics(&mut self, refresh_kind: DiskRefreshKind) { - unsafe { - get_all_list(&mut self.disks, false, false, refresh_kind); - } - } } trait GetValues { @@ -322,7 +316,6 @@ fn get_disks_mapping() -> HashMap { pub unsafe fn get_all_list( container: &mut Vec, - add_new_disks: bool, remove_not_listed_disks: bool, refresh_kind: DiskRefreshKind, ) { @@ -386,7 +379,7 @@ pub unsafe fn get_all_list( // I/O usage is updated for all disks at once at the end. refresh_disk(&mut disk.inner, refresh_kind.without_io_usage()); disk.inner.updated = true; - } else if add_new_disks { + } else { let dev_mount_point = c_buf_to_utf8_str(&fs_info.f_mntfromname).unwrap_or(""); // USB keys and CDs are removable. diff --git a/src/unix/linux/disk.rs b/src/unix/linux/disk.rs index 2dfb227d8..9d4927279 100644 --- a/src/unix/linux/disk.rs +++ b/src/unix/linux/disk.rs @@ -148,24 +148,29 @@ impl crate::DisksInner { } } - pub(crate) fn refresh_list_specifics( + pub(crate) fn refresh_specifics( &mut self, remove_not_listed_disks: bool, refresh_kind: DiskRefreshKind, ) { get_all_list( &mut self.disks, - remove_not_listed_disks, &get_all_utf8_data("/proc/mounts", 16_385).unwrap_or_default(), refresh_kind, - ) - } + ); - 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(refresh_kind, &procfs_disk_stats, false); + if remove_not_listed_disks { + self.disks.retain_mut(|disk| { + if !disk.inner.updated { + return false; + } + disk.inner.updated = false; + true + }); + } else { + for c in self.disks.iter_mut() { + c.inner.updated = false; + } } } @@ -329,12 +334,7 @@ fn find_type_for_device_name(device_name: &OsStr) -> DiskKind { } } -fn get_all_list( - container: &mut Vec, - remove_not_listed_disks: bool, - content: &str, - refresh_kind: DiskRefreshKind, -) { +fn get_all_list(container: &mut Vec, content: &str, refresh_kind: DiskRefreshKind) { // The goal of this array is to list all removable devices (the ones whose name starts with // "usb-"). let removable_entries = match fs::read_dir("/dev/disk/by-id/") { @@ -421,20 +421,6 @@ fn get_all_list( refresh_kind, )); } - - if remove_not_listed_disks { - container.retain_mut(|disk| { - if !disk.inner.updated { - return false; - } - disk.inner.updated = false; - true - }); - } else { - for c in container.iter_mut() { - c.inner.updated = false; - } - } } /// Disk IO stat information from `/proc/diskstats` file. diff --git a/src/unknown/disk.rs b/src/unknown/disk.rs index 99bc6a634..a65e2589a 100644 --- a/src/unknown/disk.rs +++ b/src/unknown/disk.rs @@ -65,7 +65,7 @@ impl DisksInner { self.disks } - pub(crate) fn refresh_list_specifics( + pub(crate) fn refresh_specifics( &mut self, _remove_not_listed_disks: bool, _refreshes: DiskRefreshKind, @@ -73,10 +73,6 @@ impl DisksInner { // Does nothing. } - pub(crate) fn refresh_specifics(&mut self, _refreshes: DiskRefreshKind) { - // Does nothing. - } - pub(crate) fn list(&self) -> &[Disk] { &self.disks } diff --git a/src/windows/disk.rs b/src/windows/disk.rs index 46e6606c9..7fe11ef49 100644 --- a/src/windows/disk.rs +++ b/src/windows/disk.rs @@ -232,7 +232,7 @@ impl DisksInner { self.disks } - pub(crate) fn refresh_list_specifics( + pub(crate) fn refresh_specifics( &mut self, remove_not_listed_disks: bool, refreshes: DiskRefreshKind, @@ -242,12 +242,6 @@ impl DisksInner { } } - pub(crate) fn refresh_specifics(&mut self, refreshes: DiskRefreshKind) { - for disk in self.list_mut() { - disk.refresh_specifics(refreshes); - } - } - pub(crate) fn list(&self) -> &[Disk] { &self.disks } diff --git a/tests/disk.rs b/tests/disk.rs index 94af64c2a..73ddc16d1 100644 --- a/tests/disk.rs +++ b/tests/disk.rs @@ -21,7 +21,7 @@ fn test_disks() { let mut disks = sysinfo::Disks::new(); assert!(disks.list().is_empty()); - disks.refresh_list(false); + disks.refresh(false); assert!(!disks.list().is_empty()); } @@ -125,7 +125,7 @@ fn test_disk_refresh_kind() { // load with minimal `DiskRefreshKind`, then refresh for added detail should also work! let mut disks = Disks::new_with_refreshed_list_specifics(DiskRefreshKind::nothing()); - disks.refresh_specifics(refreshes); + disks.refresh_specifics(false, refreshes); assertions("incremental", &disks); } } @@ -168,7 +168,7 @@ fn test_disks_usage() { // Wait a bit just in case sleep(std::time::Duration::from_millis(500)); - disks.refresh(); + disks.refresh(false); // Depending on the OS and how disks are configured, the disk usage may be the exact same // across multiple disks. To account for this, collect the disk usages and dedup.