From 6371eb8298b5dfaa9a18fcdb25b6affc7f567f04 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sun, 13 Mar 2022 14:42:15 -0400 Subject: [PATCH 1/4] df: use for loop over filesystems to display rows Replace a loop over `Row` objects with a loop over `Filesystem` objects when it comes time to display each row of the output table. This helps with code structure, since the `Filesystem` is the primary object of interest in the `df` program. This makes it easier for us to independently vary how the list of filesystems is produced and how the list of filesystems is displayed. --- src/uu/df/src/df.rs | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index b3244ff18..8f81b3efe 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -282,18 +282,29 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // Get Point for each input_path get_point_list(&mounts, &paths) }; - let data: Vec = op_mount_points + + // Get the list of filesystems to display in the output table. + let filesystems: Vec = op_mount_points .into_iter() .filter_map(Filesystem::new) - .filter(|fs| fs.usage.blocks != 0 || opt.show_all_fs || opt.show_listed_fs) - .map(Into::into) .collect(); - println!("{}", Header::new(&opt)); + // The running total of filesystem sizes and usage. + // + // This accumulator is computed in case we need to display the + // total counts in the last row of the table. let mut total = Row::new("total"); - for row in data { - println!("{}", DisplayRow::new(&row, &opt)); - total += row; + + println!("{}", Header::new(&opt)); + for filesystem in filesystems { + // If the filesystem is not empty, or if the options require + // showing all filesystems, then print the data as a row in + // the output table. + if opt.show_all_fs || opt.show_listed_fs || filesystem.usage.blocks > 0 { + let row = Row::from(filesystem); + println!("{}", DisplayRow::new(&row, &opt)); + total += row; + } } if opt.show_total { println!("{}", DisplayRow::new(&total, &opt)); From 6f3dcf599844408f2d7ec6efb69202eabb9435a4 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sun, 13 Mar 2022 14:35:55 -0400 Subject: [PATCH 2/4] df: add Filesystem::from_path() function Add the `Filesystem::from_path()` function which produces the filesystem on which a given path is mounted. The `Filesystem` is taken from a list of candidate `MountInfo` objects. --- src/uu/df/src/filesystem.rs | 137 +++++++++++++++++++++++++++++++++++- 1 file changed, 136 insertions(+), 1 deletion(-) diff --git a/src/uu/df/src/filesystem.rs b/src/uu/df/src/filesystem.rs index abea48fad..0c044a83f 100644 --- a/src/uu/df/src/filesystem.rs +++ b/src/uu/df/src/filesystem.rs @@ -7,7 +7,6 @@ //! A [`Filesystem`] struct represents a device containing a //! filesystem mounted at a particular directory. It also includes //! information on amount of space available and amount of space used. -#[cfg(windows)] use std::path::Path; #[cfg(unix)] @@ -30,6 +29,40 @@ pub(crate) struct Filesystem { pub usage: FsUsage, } +/// Find the mount info that best matches a given filesystem path. +/// +/// This function returns the element of `mounts` on which `path` is +/// mounted. If there are no matches, this function returns +/// [`None`]. If there are two or more matches, then the single +/// [`MountInfo`] with the longest mount directory is returned. +/// +/// If `canonicalize` is `true`, then the `path` is canonicalized +/// before checking whether it matches any mount directories. +/// +/// # See also +/// +/// * [`Path::canonicalize`] +/// * [`MountInfo::mount_dir`] +fn mount_info_from_path

( + mounts: &[MountInfo], + path: P, + // This is really only used for testing purposes. + canonicalize: bool, +) -> Option<&MountInfo> +where + P: AsRef, +{ + // TODO Refactor this function with `Stater::find_mount_point()` + // in the `stat` crate. + let path = if canonicalize { + path.as_ref().canonicalize().ok()? + } else { + path.as_ref().to_path_buf() + }; + let matches = mounts.iter().filter(|mi| path.starts_with(&mi.mount_dir)); + matches.max_by_key(|mi| mi.mount_dir.len()) +} + impl Filesystem { // TODO: resolve uuid in `mount_info.dev_name` if exists pub(crate) fn new(mount_info: MountInfo) -> Option { @@ -52,4 +85,106 @@ impl Filesystem { let usage = FsUsage::new(Path::new(&_stat_path)); Some(Self { mount_info, usage }) } + + /// Find and create the filesystem that best matches a given path. + /// + /// This function returns a new `Filesystem` derived from the + /// element of `mounts` on which `path` is mounted. If there are + /// no matches, this function returns [`None`]. If there are two + /// or more matches, then the single [`Filesystem`] with the + /// longest mount directory is returned. + /// + /// The `path` is canonicalized before checking whether it matches + /// any mount directories. + /// + /// # See also + /// + /// * [`Path::canonicalize`] + /// * [`MountInfo::mount_dir`] + /// + pub(crate) fn from_path

(mounts: &[MountInfo], path: P) -> Option + where + P: AsRef, + { + let canonicalize = true; + let mount_info = mount_info_from_path(mounts, path, canonicalize)?; + // TODO Make it so that we do not need to clone the `mount_info`. + let mount_info = (*mount_info).clone(); + Self::new(mount_info) + } +} + +#[cfg(test)] +mod tests { + + mod mount_info_from_path { + + use uucore::fsext::MountInfo; + + use crate::filesystem::mount_info_from_path; + + // Create a fake `MountInfo` with the given directory name. + fn mount_info(mount_dir: &str) -> MountInfo { + MountInfo { + dev_id: Default::default(), + dev_name: Default::default(), + fs_type: Default::default(), + mount_dir: String::from(mount_dir), + mount_option: Default::default(), + mount_root: Default::default(), + remote: Default::default(), + dummy: Default::default(), + } + } + + // Check whether two `MountInfo` instances are equal. + fn mount_info_eq(m1: &MountInfo, m2: &MountInfo) -> bool { + m1.dev_id == m2.dev_id + && m1.dev_name == m2.dev_name + && m1.fs_type == m2.fs_type + && m1.mount_dir == m2.mount_dir + && m1.mount_option == m2.mount_option + && m1.mount_root == m2.mount_root + && m1.remote == m2.remote + && m1.dummy == m2.dummy + } + + #[test] + fn test_empty_mounts() { + assert!(mount_info_from_path(&[], "/", false).is_none()); + } + + #[test] + fn test_exact_match() { + let mounts = [mount_info("/foo")]; + let actual = mount_info_from_path(&mounts, "/foo", false).unwrap(); + assert!(mount_info_eq(actual, &mounts[0])); + } + + #[test] + fn test_prefix_match() { + let mounts = [mount_info("/foo")]; + let actual = mount_info_from_path(&mounts, "/foo/bar", false).unwrap(); + assert!(mount_info_eq(actual, &mounts[0])); + } + + #[test] + fn test_multiple_matches() { + let mounts = [mount_info("/foo"), mount_info("/foo/bar")]; + let actual = mount_info_from_path(&mounts, "/foo/bar", false).unwrap(); + assert!(mount_info_eq(actual, &mounts[1])); + } + + #[test] + fn test_no_match() { + let mounts = [mount_info("/foo")]; + assert!(mount_info_from_path(&mounts, "/bar", false).is_none()); + } + + #[test] + fn test_partial_match() { + let mounts = [mount_info("/foo/bar")]; + assert!(mount_info_from_path(&mounts, "/foo/baz", false).is_none()); + } + } } From f4ca963dcabffdff2e2930370846804613aea47d Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sun, 13 Mar 2022 14:51:12 -0400 Subject: [PATCH 3/4] df: separate functions for two main modes of df Split the code for getting a list of `Filesystem` objects into two separate functions: one for getting the list of all filesystems (when running `df` with no arguments) and one for getting only named filesystems (when running `df` with one or more arguments). This does not change the behavior of `df` only the organization of the code. --- src/uu/df/src/df.rs | 83 +++++++++++++++++++++++---------------------- 1 file changed, 43 insertions(+), 40 deletions(-) diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index 8f81b3efe..338648e10 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -230,39 +230,49 @@ fn filter_mount_list(vmi: Vec, opt: &Options) -> Vec { result } -/// Assign 1 `MountInfo` entry to each path -/// `lofs` entries are skipped and dummy mount points are skipped -/// Only the longest matching prefix for that path is considered -/// `lofs` is for Solaris style loopback filesystem and is present in Solaris and FreeBSD. -/// It works similar to symlinks -fn get_point_list(vmi: &[MountInfo], paths: &[String]) -> Vec { +/// Get all currently mounted filesystems. +/// +/// `opt` excludes certain filesystems from consideration; see +/// [`Options`] for more information. +fn get_all_filesystems(opt: &Options) -> Vec { + // 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); + + // Convert each `MountInfo` into a `Filesystem`, which contains + // both the mount information and usage information. + mounts.into_iter().filter_map(Filesystem::new).collect() +} + +/// For each path, get the filesystem that contains that path. +fn get_named_filesystems

(paths: &[P]) -> Vec +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 = read_fs_list() + .into_iter() + .filter(|mi| mi.fs_type != "lofs" && !mi.dummy) + .collect(); + + // Convert each path into a `Filesystem`, which contains + // both the mount information and usage information. paths .iter() - .map(|p| { - vmi.iter() - .filter(|mi| mi.fs_type.ne("lofs")) - .filter(|mi| !mi.dummy) - .filter(|mi| p.starts_with(&mi.mount_dir)) - .max_by_key(|mi| mi.mount_dir.len()) - .unwrap() - .clone() - }) - .collect::>() + .filter_map(|p| Filesystem::from_path(&mounts, p)) + .collect() } #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uu_app().get_matches_from(args); - - // Canonicalize the input_paths and then convert to string - let paths = matches - .values_of(OPT_PATHS) - .unwrap_or_default() - .map(Path::new) - .filter_map(|v| v.canonicalize().ok()) - .filter_map(|v| v.into_os_string().into_string().ok()) - .collect::>(); - #[cfg(windows)] { if matches.is_present(OPT_INODES) { @@ -273,21 +283,14 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let opt = Options::from(&matches).map_err(|e| USimpleError::new(1, format!("{}", e)))?; - let mounts = read_fs_list(); - - let op_mount_points: Vec = if paths.is_empty() { - // Get all entries - filter_mount_list(mounts, &opt) - } else { - // Get Point for each input_path - get_point_list(&mounts, &paths) - }; - // Get the list of filesystems to display in the output table. - let filesystems: Vec = op_mount_points - .into_iter() - .filter_map(Filesystem::new) - .collect(); + let filesystems: Vec = match matches.values_of(OPT_PATHS) { + None => get_all_filesystems(&opt), + Some(paths) => { + let paths: Vec<&str> = paths.collect(); + get_named_filesystems(&paths) + } + }; // The running total of filesystem sizes and usage. // From b84ff3576660ee1c8be01831c90f30f4592b8fa5 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Wed, 16 Mar 2022 09:16:01 +0100 Subject: [PATCH 4/4] Add canonicalized to the ignore spell list --- src/uu/df/src/filesystem.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/uu/df/src/filesystem.rs b/src/uu/df/src/filesystem.rs index 0c044a83f..bd9ff34eb 100644 --- a/src/uu/df/src/filesystem.rs +++ b/src/uu/df/src/filesystem.rs @@ -7,6 +7,7 @@ //! A [`Filesystem`] struct represents a device containing a //! filesystem mounted at a particular directory. It also includes //! information on amount of space available and amount of space used. +// spell-checker:ignore canonicalized use std::path::Path; #[cfg(unix)]