From 5cf7139467b02f9474532d9a266840e15e5df460 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Wed, 23 Feb 2022 21:28:07 -0500 Subject: [PATCH] df: fix block size header for multiples of 1024 Correct the column header printed by `df` when the `--block-size` argument has a value that is a multiple of 1024. After this commit, the header looks like "1K" or "4M" or "117G", etc., depending on the particular value of the block size. For example: $ df --block-size=1024 | head -n1 Filesystem 1K-blocks Used Available Use% Mounted on $ df --block-size=2048 | head -n1 Filesystem 2K-blocks Used Available Use% Mounted on $ df --block-size=3072 | head -n1 Filesystem 3K-blocks Used Available Use% Mounted on $ df --block-size=4096 | head -n1 Filesystem 4K-blocks Used Available Use% Mounted on --- src/uu/df/src/blocks.rs | 159 ++++++++++++++++++++++++++++++++++++--- src/uu/df/src/df.rs | 33 ++++++-- src/uu/df/src/table.rs | 29 ++++--- tests/by-util/test_df.rs | 29 +++++++ 4 files changed, 225 insertions(+), 25 deletions(-) diff --git a/src/uu/df/src/blocks.rs b/src/uu/df/src/blocks.rs index bb6a86df5..10ec22012 100644 --- a/src/uu/df/src/blocks.rs +++ b/src/uu/df/src/blocks.rs @@ -3,8 +3,72 @@ // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. //! Types for representing and displaying block sizes. -use crate::{OPT_HUMAN_READABLE, OPT_HUMAN_READABLE_2}; +use crate::{OPT_BLOCKSIZE, OPT_HUMAN_READABLE, OPT_HUMAN_READABLE_2}; use clap::ArgMatches; +use std::fmt; +use std::num::ParseIntError; + +/// The first ten powers of 1024. +const IEC_BASES: [u128; 10] = [ + 1, + 1_024, + 1_048_576, + 1_073_741_824, + 1_099_511_627_776, + 1_125_899_906_842_624, + 1_152_921_504_606_846_976, + 1_180_591_620_717_411_303_424, + 1_208_925_819_614_629_174_706_176, + 1_237_940_039_285_380_274_899_124_224, +]; + +/// Suffixes for the first nine multi-byte unit suffixes. +const SUFFIXES: [char; 9] = ['B', 'K', 'M', 'G', 'T', 'P', 'E', 'Z', 'Y']; + +/// Convert a multiple of 1024 into a string like "12K" or "34M". +/// +/// # Examples +/// +/// Powers of 1024 become "1K", "1M", "1G", etc. +/// +/// ```rust,ignore +/// assert_eq!(to_magnitude_and_suffix_1024(1024).unwrap(), "1K"); +/// assert_eq!(to_magnitude_and_suffix_1024(1024 * 1024).unwrap(), "1M"); +/// assert_eq!(to_magnitude_and_suffix_1024(1024 * 1024 * 1024).unwrap(), "1G"); +/// ``` +/// +/// Multiples of those powers affect the magnitude part of the +/// returned string: +/// +/// ```rust,ignore +/// assert_eq!(to_magnitude_and_suffix_1024(123 * 1024).unwrap(), "123K"); +/// assert_eq!(to_magnitude_and_suffix_1024(456 * 1024 * 1024).unwrap(), "456M"); +/// assert_eq!(to_magnitude_and_suffix_1024(789 * 1024 * 1024 * 1024).unwrap(), "789G"); +/// ``` +fn to_magnitude_and_suffix_1024(n: u128) -> Result { + // Find the smallest power of 1024 that is larger than `n`. That + // number indicates which units and suffix to use. + for i in 0..IEC_BASES.len() - 1 { + if n < IEC_BASES[i + 1] { + return Ok(format!("{}{}", n / IEC_BASES[i], SUFFIXES[i])); + } + } + Err(()) +} + +/// Convert a number into a magnitude and a multi-byte unit suffix. +/// +/// # Errors +/// +/// If the number is too large to represent. +fn to_magnitude_and_suffix(n: u128) -> Result { + if n % 1024 == 0 { + to_magnitude_and_suffix_1024(n) + } else { + // TODO Implement this, probably using code from `numfmt`. + Ok("1kB".into()) + } +} /// A block size to use in condensing the display of a large number of bytes. /// @@ -43,14 +107,91 @@ impl Default for BlockSize { } } -impl From<&ArgMatches> for BlockSize { - fn from(matches: &ArgMatches) -> Self { - if matches.is_present(OPT_HUMAN_READABLE) { - Self::HumanReadableBinary - } else if matches.is_present(OPT_HUMAN_READABLE_2) { - Self::HumanReadableDecimal - } else { - Self::default() +pub(crate) fn block_size_from_matches(matches: &ArgMatches) -> Result { + if matches.is_present(OPT_HUMAN_READABLE) { + Ok(BlockSize::HumanReadableBinary) + } else if matches.is_present(OPT_HUMAN_READABLE_2) { + Ok(BlockSize::HumanReadableDecimal) + } else if matches.is_present(OPT_BLOCKSIZE) { + let s = matches.value_of(OPT_BLOCKSIZE).unwrap(); + Ok(BlockSize::Bytes(s.parse()?)) + } else { + Ok(Default::default()) + } +} + +impl fmt::Display for BlockSize { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Self::HumanReadableBinary => write!(f, "Size"), + Self::HumanReadableDecimal => write!(f, "Size"), + Self::Bytes(n) => match to_magnitude_and_suffix(*n as u128) { + Ok(s) => write!(f, "{}-blocks", s), + Err(_) => Err(fmt::Error), + }, } } } + +#[cfg(test)] +mod tests { + + use crate::blocks::{to_magnitude_and_suffix, BlockSize}; + + #[test] + fn test_to_magnitude_and_suffix_powers_of_1024() { + assert_eq!(to_magnitude_and_suffix(1024).unwrap(), "1K"); + assert_eq!(to_magnitude_and_suffix(2048).unwrap(), "2K"); + assert_eq!(to_magnitude_and_suffix(4096).unwrap(), "4K"); + assert_eq!(to_magnitude_and_suffix(1024 * 1024).unwrap(), "1M"); + assert_eq!(to_magnitude_and_suffix(2 * 1024 * 1024).unwrap(), "2M"); + assert_eq!(to_magnitude_and_suffix(1024 * 1024 * 1024).unwrap(), "1G"); + assert_eq!( + to_magnitude_and_suffix(34 * 1024 * 1024 * 1024).unwrap(), + "34G" + ); + } + + // TODO We have not yet implemented this behavior, but when we do, + // uncomment this test. + + // #[test] + // fn test_to_magnitude_and_suffix_not_powers_of_1024() { + // assert_eq!(to_magnitude_and_suffix(1).unwrap(), "1B"); + // assert_eq!(to_magnitude_and_suffix(999).unwrap(), "999B"); + + // assert_eq!(to_magnitude_and_suffix(1000).unwrap(), "1kB"); + // assert_eq!(to_magnitude_and_suffix(1001).unwrap(), "1.1kB"); + // assert_eq!(to_magnitude_and_suffix(1023).unwrap(), "1.1kB"); + // assert_eq!(to_magnitude_and_suffix(1025).unwrap(), "1.1kB"); + // assert_eq!(to_magnitude_and_suffix(999_000).unwrap(), "999kB"); + + // assert_eq!(to_magnitude_and_suffix(999_001).unwrap(), "1MB"); + // assert_eq!(to_magnitude_and_suffix(999_999).unwrap(), "1MB"); + // assert_eq!(to_magnitude_and_suffix(1_000_000).unwrap(), "1MB"); + // assert_eq!(to_magnitude_and_suffix(1_000_001).unwrap(), "1.1MB"); + // assert_eq!(to_magnitude_and_suffix(1_100_000).unwrap(), "1.1MB"); + // assert_eq!(to_magnitude_and_suffix(1_100_001).unwrap(), "1.2MB"); + // assert_eq!(to_magnitude_and_suffix(1_900_000).unwrap(), "1.9MB"); + // assert_eq!(to_magnitude_and_suffix(1_900_001).unwrap(), "2MB"); + // assert_eq!(to_magnitude_and_suffix(9_900_000).unwrap(), "9.9MB"); + // assert_eq!(to_magnitude_and_suffix(9_900_001).unwrap(), "10MB"); + // assert_eq!(to_magnitude_and_suffix(999_000_000).unwrap(), "999MB"); + + // assert_eq!(to_magnitude_and_suffix(999_000_001).unwrap(), "1GB"); + // assert_eq!(to_magnitude_and_suffix(1_000_000_000).unwrap(), "1GB"); + // // etc. + // } + + #[test] + fn test_block_size_display() { + assert_eq!(format!("{}", BlockSize::HumanReadableBinary), "Size"); + assert_eq!(format!("{}", BlockSize::HumanReadableDecimal), "Size"); + assert_eq!(format!("{}", BlockSize::Bytes(1024)), "1K-blocks"); + assert_eq!(format!("{}", BlockSize::Bytes(2 * 1024)), "2K-blocks"); + assert_eq!( + format!("{}", BlockSize::Bytes(3 * 1024 * 1024)), + "3M-blocks" + ); + } +} diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index 5e3812375..65091b4cc 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -9,20 +9,22 @@ mod blocks; mod table; +use uucore::error::{UResult, USimpleError}; +use uucore::format_usage; #[cfg(unix)] use uucore::fsext::statfs; use uucore::fsext::{read_fs_list, FsUsage, MountInfo}; -use uucore::{error::UResult, format_usage}; use clap::{crate_version, App, AppSettings, Arg, ArgMatches}; use std::collections::HashSet; +use std::fmt; use std::iter::FromIterator; #[cfg(windows)] use std::path::Path; -use crate::blocks::BlockSize; +use crate::blocks::{block_size_from_matches, BlockSize}; use crate::table::{DisplayRow, Header, Row}; static ABOUT: &str = "Show information about the file system on which each FILE resides,\n\ @@ -78,19 +80,36 @@ struct Options { show_total: bool, } +enum OptionsError { + InvalidBlockSize, +} + +impl fmt::Display for OptionsError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + // TODO This should include the raw string provided as the argument. + // + // TODO This needs to vary based on whether `--block-size` + // or `-B` were provided. + Self::InvalidBlockSize => write!(f, "invalid --block-size argument"), + } + } +} + impl Options { /// Convert command-line arguments into [`Options`]. - fn from(matches: &ArgMatches) -> Self { - Self { + fn from(matches: &ArgMatches) -> Result { + Ok(Self { show_local_fs: matches.is_present(OPT_LOCAL), show_all_fs: matches.is_present(OPT_ALL), show_listed_fs: false, show_fs_type: matches.is_present(OPT_PRINT_TYPE), show_inode_instead: matches.is_present(OPT_INODES), - block_size: BlockSize::from(matches), + block_size: block_size_from_matches(matches) + .map_err(|_| OptionsError::InvalidBlockSize)?, fs_selector: FsSelector::from(matches), show_total: matches.is_present(OPT_TOTAL), - } + }) } } @@ -260,7 +279,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } } - let opt = Options::from(&matches); + let opt = Options::from(&matches).map_err(|e| USimpleError::new(1, format!("{}", e)))?; let mounts = read_fs_list(); let data: Vec = filter_mount_list(mounts, &paths, &opt) diff --git a/src/uu/df/src/table.rs b/src/uu/df/src/table.rs index 1b1c61d1f..6d4a3bdd1 100644 --- a/src/uu/df/src/table.rs +++ b/src/uu/df/src/table.rs @@ -306,13 +306,12 @@ impl fmt::Display for Header<'_> { write!(f, "{0: >12} ", "IFree")?; write!(f, "{0: >5} ", "IUse%")?; } else { - // TODO Support arbitrary positive scaling factors (from - // the `--block-size` command-line argument). - if let BlockSize::Bytes(_) = self.options.block_size { - write!(f, "{0: >12} ", "1k-blocks")?; - } else { - write!(f, "{0: >12} ", "Size")?; - }; + // `Display` is implemented for `BlockSize`, but `Display` + // only works when formatting an object into an empty + // format, `{}`. So we use `format!()` first to create the + // string, then use `write!()` to align the string and pad + // with spaces. + write!(f, "{0: >12} ", format!("{}", self.options.block_size))?; write!(f, "{0: >12} ", "Used")?; write!(f, "{0: >12} ", "Available")?; #[cfg(target_os = "macos")] @@ -335,7 +334,7 @@ mod tests { let options = Default::default(); assert_eq!( Header::new(&options).to_string(), - "Filesystem 1k-blocks Used Available Use% Mounted on " + "Filesystem 1K-blocks Used Available Use% Mounted on " ); } @@ -347,7 +346,7 @@ mod tests { }; assert_eq!( Header::new(&options).to_string(), - "Filesystem Type 1k-blocks Used Available Use% Mounted on " + "Filesystem Type 1K-blocks Used Available Use% Mounted on " ); } @@ -363,6 +362,18 @@ mod tests { ); } + #[test] + fn test_header_display_block_size_1024() { + let options = Options { + block_size: BlockSize::Bytes(3 * 1024), + ..Default::default() + }; + assert_eq!( + Header::new(&options).to_string(), + "Filesystem 3K-blocks Used Available Use% Mounted on " + ); + } + #[test] fn test_header_display_human_readable_binary() { let options = Options { diff --git a/tests/by-util/test_df.rs b/tests/by-util/test_df.rs index 513423766..455c75c7c 100644 --- a/tests/by-util/test_df.rs +++ b/tests/by-util/test_df.rs @@ -28,6 +28,8 @@ fn test_df_compatible_si() { #[test] fn test_df_output() { + // TODO These should fail because `-total` should have two dashes, + // not just one. But they don't fail. if cfg!(target_os = "macos") { new_ucmd!().arg("-H").arg("-total").succeeds(). stdout_only("Filesystem Size Used Available Capacity Use% Mounted on \n"); @@ -121,4 +123,31 @@ fn test_total() { // TODO We could also check here that the use percentage matches. } +#[test] +fn test_block_size_1024() { + fn get_header(block_size: u64) -> String { + // TODO When #3057 is resolved, we should just use + // + // new_ucmd!().arg("--output=size").succeeds().stdout_move_str(); + // + // instead of parsing the entire `df` table as a string. + let output = new_ucmd!() + .args(&["-B", &format!("{}", block_size)]) + .succeeds() + .stdout_move_str(); + let first_line = output.lines().next().unwrap(); + let mut column_labels = first_line.split_whitespace(); + let size_column_label = column_labels.nth(1).unwrap(); + size_column_label.into() + } + + assert_eq!(get_header(1024), "1K-blocks"); + assert_eq!(get_header(2048), "2K-blocks"); + assert_eq!(get_header(4096), "4K-blocks"); + assert_eq!(get_header(1024 * 1024), "1M-blocks"); + assert_eq!(get_header(2 * 1024 * 1024), "2M-blocks"); + assert_eq!(get_header(1024 * 1024 * 1024), "1G-blocks"); + assert_eq!(get_header(34 * 1024 * 1024 * 1024), "34G-blocks"); +} + // ToDO: more tests...