From d996953aaf49da6150f925e2306d177dbc90f15f Mon Sep 17 00:00:00 2001 From: Sergei Patiakin Date: Fri, 14 Mar 2025 21:50:11 +0100 Subject: [PATCH 1/3] df: fix ordering of mount-point filtering and best-path determination --- src/uu/df/src/df.rs | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index a73cc21ef24..a385fa560bd 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -12,8 +12,8 @@ use blocks::HumanReadable; use clap::builder::ValueParser; use table::HeaderMode; use uucore::display::Quotable; -use uucore::error::{UError, UResult, USimpleError}; -use uucore::fsext::{MountInfo, read_fs_list}; +use uucore::error::{get_exit_code, UError, UResult, USimpleError}; +use uucore::fsext::{read_fs_list, MountInfo}; use uucore::parse_size::ParseSizeError; use uucore::{format_usage, help_about, help_section, help_usage, show}; @@ -243,7 +243,10 @@ fn is_included(mi: &MountInfo, opt: &Options) -> bool { } // Don't show pseudo filesystems unless `--all` has been given. - if mi.dummy && !opt.show_all_fs { + // The "lofs" filesystem is a loopback + // filesystem present on Solaris and FreeBSD systems. It + // is similar to a symbolic link. + if (mi.dummy || mi.fs_type == "lofs") && !opt.show_all_fs { return false; } @@ -379,29 +382,19 @@ where P: AsRef, { // The list of all mounted filesystems. - // - // Filesystems marked as `dummy` or of type "lofs" are not - // considered. The "lofs" filesystem is a loopback - // filesystem present on Solaris and FreeBSD systems. It - // is similar to a symbolic link. - let mounts: Vec = filter_mount_list(read_fs_list()?, opt) - .into_iter() - .filter(|mi| mi.fs_type != "lofs" && !mi.dummy) - .collect(); + let mounts: Vec = read_fs_list()?; let mut result = vec![]; - // this happens if the file system type doesn't exist - if mounts.is_empty() { - show!(USimpleError::new(1, "no file systems processed")); - return Ok(result); - } - // Convert each path into a `Filesystem`, which contains // both the mount information and usage information. for path in paths { match Filesystem::from_path(&mounts, path) { - Ok(fs) => result.push(fs), + Ok(fs) => { + if is_included(&fs.mount_info, opt) { + result.push(fs); + } + }, Err(FsError::InvalidPath) => { show!(USimpleError::new( 1, @@ -423,6 +416,11 @@ where } } } + if get_exit_code() == 0 && result.is_empty() { + show!(USimpleError::new(1, "no file systems processed")); + return Ok(result); + } + Ok(result) } From 7519276aa0dfa62c18a43c8514d120978c57900c Mon Sep 17 00:00:00 2001 From: Sergei Patiakin Date: Fri, 14 Mar 2025 22:11:48 +0100 Subject: [PATCH 2/3] df: remove function filter_mount_list --- src/uu/df/src/df.rs | 52 +++++++++++---------------------------------- 1 file changed, 12 insertions(+), 40 deletions(-) diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index a385fa560bd..6209d1c1641 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -309,28 +309,6 @@ fn is_best(previous: &[MountInfo], mi: &MountInfo) -> bool { true } -/// Keep only the specified subset of [`MountInfo`] instances. -/// -/// The `opt` argument specifies a variety of ways of excluding -/// [`MountInfo`] instances; see [`Options`] for more information. -/// -/// Finally, if there are duplicate entries, the one with the shorter -/// path is kept. -fn filter_mount_list(vmi: Vec, opt: &Options) -> Vec { - let mut result = vec![]; - for mi in vmi { - // TODO The running time of the `is_best()` function is linear - // in the length of `result`. That makes the running time of - // this loop quadratic in the length of `vmi`. This could be - // improved by a more efficient implementation of `is_best()`, - // but `vmi` is probably not very long in practice. - if is_included(&mi, opt) && is_best(&result, &mi) { - result.push(mi); - } - } - result -} - /// Get all currently mounted filesystems. /// /// `opt` excludes certain filesystems from consideration and allows for the synchronization of filesystems before running; see @@ -347,11 +325,17 @@ fn get_all_filesystems(opt: &Options) -> UResult> { } } - // The list of all mounted filesystems. - // - // Filesystems excluded by the command-line options are - // not considered. - let mounts: Vec = filter_mount_list(read_fs_list()?, opt); + let mut mounts = vec![]; + for mi in read_fs_list()? { + // TODO The running time of the `is_best()` function is linear + // in the length of `result`. That makes the running time of + // this loop quadratic in the length of `vmi`. This could be + // improved by a more efficient implementation of `is_best()`, + // but `vmi` is probably not very long in practice. + if is_included(&mi, opt) && is_best(&mounts, &mi) { + mounts.push(mi); + } + } // Convert each `MountInfo` into a `Filesystem`, which contains // both the mount information and usage information. @@ -394,7 +378,7 @@ where if is_included(&fs.mount_info, opt) { result.push(fs); } - }, + } Err(FsError::InvalidPath) => { show!(USimpleError::new( 1, @@ -881,16 +865,4 @@ mod tests { assert!(is_included(&m, &opt)); } } - - mod filter_mount_list { - - use crate::{Options, filter_mount_list}; - - #[test] - fn test_empty() { - let opt = Options::default(); - let mount_infos = vec![]; - assert!(filter_mount_list(mount_infos, &opt).is_empty()); - } - } } From dab13ed74ab2ea130f6f247a993554a2cf383016 Mon Sep 17 00:00:00 2001 From: Sergei Patiakin Date: Fri, 14 Mar 2025 22:45:04 +0100 Subject: [PATCH 3/3] df: add unit test for issue 6194 --- src/uu/df/src/df.rs | 4 ++-- tests/by-util/test_df.rs | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index 6209d1c1641..650ada614d9 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -12,8 +12,8 @@ use blocks::HumanReadable; use clap::builder::ValueParser; use table::HeaderMode; use uucore::display::Quotable; -use uucore::error::{get_exit_code, UError, UResult, USimpleError}; -use uucore::fsext::{read_fs_list, MountInfo}; +use uucore::error::{UError, UResult, USimpleError, get_exit_code}; +use uucore::fsext::{MountInfo, read_fs_list}; use uucore::parse_size::ParseSizeError; use uucore::{format_usage, help_about, help_section, help_usage, show}; diff --git a/tests/by-util/test_df.rs b/tests/by-util/test_df.rs index d3692a7f0dd..29cc0eeb019 100644 --- a/tests/by-util/test_df.rs +++ b/tests/by-util/test_df.rs @@ -299,6 +299,12 @@ fn test_type_option_with_file() { .fails() .stderr_contains("no file systems processed"); + // Assume the mount point at /dev has a different filesystem type to the mount point at / + new_ucmd!() + .args(&["-t", fs_type, "/dev"]) + .fails() + .stderr_contains("no file systems processed"); + let fs_types = new_ucmd!() .arg("--output=fstype") .succeeds()