From 4190fe22ef3ea96a65fcd7bb9a5d9dbe43c0b10e Mon Sep 17 00:00:00 2001 From: Sergei Patiakin Date: Thu, 17 Apr 2025 22:06:20 +0200 Subject: [PATCH] fix: df: filter filesystem types after mount point resolution (#7452) Closes: #6194 --- src/uu/df/src/df.rs | 84 +++++++++++++--------------------------- tests/by-util/test_df.rs | 6 +++ 2 files changed, 33 insertions(+), 57 deletions(-) diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index 71c580a15..9e2bb6920 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -12,7 +12,7 @@ use blocks::HumanReadable; use clap::builder::ValueParser; use table::HeaderMode; use uucore::display::Quotable; -use uucore::error::{UError, UResult, USimpleError}; +use uucore::error::{UError, UResult, USimpleError, get_exit_code}; use uucore::fsext::{MountInfo, read_fs_list}; use uucore::parser::parse_size::ParseSizeError; use uucore::{format_usage, help_about, help_section, help_usage, show}; @@ -222,7 +222,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; } @@ -285,28 +288,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 @@ -323,11 +304,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. @@ -358,29 +345,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, @@ -402,6 +379,11 @@ where } } } + if get_exit_code() == 0 && result.is_empty() { + show!(USimpleError::new(1, "no file systems processed")); + return Ok(result); + } + Ok(result) } @@ -853,16 +835,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()); - } - } } diff --git a/tests/by-util/test_df.rs b/tests/by-util/test_df.rs index 95131e556..d9d632961 100644 --- a/tests/by-util/test_df.rs +++ b/tests/by-util/test_df.rs @@ -303,6 +303,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()