Merge pull request #3554 from cakebaker/ticket_3422

df: fix rounding behavior in humanreadable mode

Cargo.lock

@@ -2219,7 +2219,6 @@ name = "uu_df"
 version = "0.0.14"
 dependencies = [
  "clap 3.1.18",
- "number_prefix",
  "unicode-width",
  "uucore",
 ]

src/uu/df/Cargo.toml

@@ -16,7 +16,6 @@ path = "src/df.rs"
 
 [dependencies]
 clap = { version = "3.1", features = ["wrap_help", "cargo"] }
-number_prefix = "0.4"
 uucore = { version=">=0.0.11", package="uucore", path="../../uucore", features=["libc", "fsext"] }
 unicode-width = "0.1.9"
 

src/uu/df/src/blocks.rs

@@ -26,9 +26,7 @@ const IEC_BASES: [u128; 10] = [
     1_237_940_039_285_380_274_899_124_224,
 ];
 
-/// Suffixes for the first nine multi-byte unit suffixes.
+/// The first ten powers of 1000.
-const SUFFIXES: [char; 9] = ['B', 'K', 'M', 'G', 'T', 'P', 'E', 'Z', 'Y'];
-
 const SI_BASES: [u128; 10] = [
     1,
     1_000,
@@ -42,93 +40,68 @@ const SI_BASES: [u128; 10] = [
     1_000_000_000_000_000_000_000_000_000,
 ];
 
-// we use "kB" instead of "KB" because of GNU df
+/// A SuffixType determines whether the suffixes are 1000 or 1024 based, and whether they are
-const SI_SUFFIXES: [&str; 9] = ["B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB"];
+/// intended for HumanReadable mode or not.
-
+#[derive(Clone, Copy)]
-/// Convert a multiple of 1024 into a string like "12K" or "34M".
+pub(crate) enum SuffixType {
-///
+    Iec,
-/// # Examples
+    Si,
-///
+    HumanReadable(HumanReadable),
-/// 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<String, ()> {
-    // 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 string like "12kB" or "34MB".
+impl SuffixType {
-///
+    /// The first ten powers of 1024 and 1000, respectively.
-/// Powers of 1000 become "1kB", "1MB", "1GB", etc.
+    fn bases(&self) -> [u128; 10] {
-///
+        match self {
-/// The returned string has a maximum length of 5 chars, for example: "1.1kB", "999kB", "1MB".
+            Self::Iec | Self::HumanReadable(HumanReadable::Binary) => IEC_BASES,
-fn to_magnitude_and_suffix_not_powers_of_1024(n: u128) -> Result<String, ()> {
+            Self::Si | Self::HumanReadable(HumanReadable::Decimal) => SI_BASES,
-    let mut i = 0;
+        }
-
-    while SI_BASES[i + 1] - SI_BASES[i] < n && i < SI_SUFFIXES.len() {
-        i += 1;
     }
 
-    let quot = n / SI_BASES[i];
+    /// Suffixes for the first nine multi-byte unit suffixes.
-    let rem = n % SI_BASES[i];
+    fn suffixes(&self) -> [&'static str; 9] {
-    let suffix = SI_SUFFIXES[i];
+        match self {
-
+            // we use "kB" instead of "KB", same as GNU df
-    if rem == 0 {
+            Self::Si => ["B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB"],
-        Ok(format!("{}{}", quot, suffix))
+            Self::Iec => ["B", "K", "M", "G", "T", "P", "E", "Z", "Y"],
-    } else {
+            Self::HumanReadable(HumanReadable::Binary) => {
-        let tenths_place = rem / (SI_BASES[i] / 10);
+                ["", "K", "M", "G", "T", "P", "E", "Z", "Y"]
-
+            }
-        if rem % (SI_BASES[i] / 10) == 0 {
+            Self::HumanReadable(HumanReadable::Decimal) => {
-            Ok(format!("{}.{}{}", quot, tenths_place, suffix))
+                ["", "k", "M", "G", "T", "P", "E", "Z", "Y"]
-        } else if tenths_place + 1 == 10 || quot >= 10 {
+            }
-            Ok(format!("{}{}", quot + 1, suffix))
-        } else {
-            Ok(format!("{}.{}{}", quot, tenths_place + 1, suffix))
         }
     }
 }
 
 /// Convert a number into a magnitude and a multi-byte unit suffix.
 ///
-/// # Errors
+/// The returned string has a maximum length of 5 chars, for example: "1.1kB", "999kB", "1MB".
-///
+pub(crate) fn to_magnitude_and_suffix(n: u128, suffix_type: SuffixType) -> String {
-/// If the number is too large to represent.
+    let bases = suffix_type.bases();
-fn to_magnitude_and_suffix(n: u128) -> Result<String, ()> {
+    let suffixes = suffix_type.suffixes();
-    if n % 1024 == 0 && n % 1000 != 0 {
+    let mut i = 0;
-        to_magnitude_and_suffix_1024(n)
+
-    } else {
+    while bases[i + 1] - bases[i] < n && i < suffixes.len() {
-        to_magnitude_and_suffix_not_powers_of_1024(n)
+        i += 1;
     }
-}
 
-/// A mode to use in condensing the display of a large number of bytes.
+    let quot = n / bases[i];
-pub(crate) enum SizeFormat {
+    let rem = n % bases[i];
-    HumanReadable(HumanReadable),
+    let suffix = suffixes[i];
-    StaticBlockSize,
-}
 
-impl Default for SizeFormat {
+    if rem == 0 {
-    fn default() -> Self {
+        format!("{}{}", quot, suffix)
-        Self::StaticBlockSize
+    } else {
+        let tenths_place = rem / (bases[i] / 10);
+
+        if rem % (bases[i] / 10) == 0 {
+            format!("{}.{}{}", quot, tenths_place, suffix)
+        } else if tenths_place + 1 == 10 || quot >= 10 {
+            format!("{}{}", quot + 1, suffix)
+        } else {
+            format!("{}.{}{}", quot, tenths_place + 1, suffix)
+        }
     }
 }
 
@@ -207,10 +180,15 @@ pub(crate) fn block_size_from_matches(matches: &ArgMatches) -> Result<BlockSize,
 impl fmt::Display for BlockSize {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
-            Self::Bytes(n) => match to_magnitude_and_suffix(*n as u128) {
+            Self::Bytes(n) => {
-                Ok(s) => write!(f, "{}", s),
+                let s = if n % 1024 == 0 && n % 1000 != 0 {
-                Err(_) => Err(fmt::Error),
+                    to_magnitude_and_suffix(*n as u128, SuffixType::Iec)
-            },
+                } else {
+                    to_magnitude_and_suffix(*n as u128, SuffixType::Si)
+                };
+
+                write!(f, "{}", s)
+            }
         }
     }
 }
@@ -220,56 +198,64 @@ mod tests {
 
     use std::env;
 
-    use crate::blocks::{to_magnitude_and_suffix, BlockSize};
+    use crate::blocks::{to_magnitude_and_suffix, BlockSize, SuffixType};
 
     #[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(1024, SuffixType::Iec), "1K");
-        assert_eq!(to_magnitude_and_suffix(2048).unwrap(), "2K");
+        assert_eq!(to_magnitude_and_suffix(2048, SuffixType::Iec), "2K");
-        assert_eq!(to_magnitude_and_suffix(4096).unwrap(), "4K");
+        assert_eq!(to_magnitude_and_suffix(4096, SuffixType::Iec), "4K");
-        assert_eq!(to_magnitude_and_suffix(1024 * 1024).unwrap(), "1M");
+        assert_eq!(to_magnitude_and_suffix(1024 * 1024, SuffixType::Iec), "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(),
+            to_magnitude_and_suffix(2 * 1024 * 1024, SuffixType::Iec),
+            "2M"
+        );
+        assert_eq!(
+            to_magnitude_and_suffix(1024 * 1024 * 1024, SuffixType::Iec),
+            "1G"
+        );
+        assert_eq!(
+            to_magnitude_and_suffix(34 * 1024 * 1024 * 1024, SuffixType::Iec),
             "34G"
         );
     }
 
     #[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(1, SuffixType::Si), "1B");
-        assert_eq!(to_magnitude_and_suffix(999).unwrap(), "999B");
+        assert_eq!(to_magnitude_and_suffix(999, SuffixType::Si), "999B");
 
-        assert_eq!(to_magnitude_and_suffix(1000).unwrap(), "1kB");
+        assert_eq!(to_magnitude_and_suffix(1000, SuffixType::Si), "1kB");
-        assert_eq!(to_magnitude_and_suffix(1001).unwrap(), "1.1kB");
+        assert_eq!(to_magnitude_and_suffix(1001, SuffixType::Si), "1.1kB");
-        assert_eq!(to_magnitude_and_suffix(1023).unwrap(), "1.1kB");
+        assert_eq!(to_magnitude_and_suffix(1023, SuffixType::Si), "1.1kB");
-        assert_eq!(to_magnitude_and_suffix(1025).unwrap(), "1.1kB");
+        assert_eq!(to_magnitude_and_suffix(1025, SuffixType::Si), "1.1kB");
-        assert_eq!(to_magnitude_and_suffix(10_001).unwrap(), "11kB");
+        assert_eq!(to_magnitude_and_suffix(10_001, SuffixType::Si), "11kB");
-        assert_eq!(to_magnitude_and_suffix(999_000).unwrap(), "999kB");
+        assert_eq!(to_magnitude_and_suffix(999_000, SuffixType::Si), "999kB");
 
-        assert_eq!(to_magnitude_and_suffix(999_001).unwrap(), "1MB");
+        assert_eq!(to_magnitude_and_suffix(999_001, SuffixType::Si), "1MB");
-        assert_eq!(to_magnitude_and_suffix(999_999).unwrap(), "1MB");
+        assert_eq!(to_magnitude_and_suffix(999_999, SuffixType::Si), "1MB");
-        assert_eq!(to_magnitude_and_suffix(1_000_000).unwrap(), "1MB");
+        assert_eq!(to_magnitude_and_suffix(1_000_000, SuffixType::Si), "1MB");
-        assert_eq!(to_magnitude_and_suffix(1_000_001).unwrap(), "1.1MB");
+        assert_eq!(to_magnitude_and_suffix(1_000_001, SuffixType::Si), "1.1MB");
-        assert_eq!(to_magnitude_and_suffix(1_100_000).unwrap(), "1.1MB");
+        assert_eq!(to_magnitude_and_suffix(1_100_000, SuffixType::Si), "1.1MB");
-        assert_eq!(to_magnitude_and_suffix(1_100_001).unwrap(), "1.2MB");
+        assert_eq!(to_magnitude_and_suffix(1_100_001, SuffixType::Si), "1.2MB");
-        assert_eq!(to_magnitude_and_suffix(1_900_000).unwrap(), "1.9MB");
+        assert_eq!(to_magnitude_and_suffix(1_900_000, SuffixType::Si), "1.9MB");
-        assert_eq!(to_magnitude_and_suffix(1_900_001).unwrap(), "2MB");
+        assert_eq!(to_magnitude_and_suffix(1_900_001, SuffixType::Si), "2MB");
-        assert_eq!(to_magnitude_and_suffix(9_900_000).unwrap(), "9.9MB");
+        assert_eq!(to_magnitude_and_suffix(9_900_000, SuffixType::Si), "9.9MB");
-        assert_eq!(to_magnitude_and_suffix(9_900_001).unwrap(), "10MB");
+        assert_eq!(to_magnitude_and_suffix(9_900_001, SuffixType::Si), "10MB");
-        assert_eq!(to_magnitude_and_suffix(999_000_000).unwrap(), "999MB");
+        assert_eq!(
+            to_magnitude_and_suffix(999_000_000, SuffixType::Si),
+            "999MB"
+        );
 
-        assert_eq!(to_magnitude_and_suffix(999_000_001).unwrap(), "1GB");
+        assert_eq!(to_magnitude_and_suffix(999_000_001, SuffixType::Si), "1GB");
-        assert_eq!(to_magnitude_and_suffix(1_000_000_000).unwrap(), "1GB");
+        assert_eq!(
-        assert_eq!(to_magnitude_and_suffix(1_000_000_001).unwrap(), "1.1GB");
+            to_magnitude_and_suffix(1_000_000_000, SuffixType::Si),
-    }
+            "1GB"
-
+        );
-    #[test]
+        assert_eq!(
-    fn test_to_magnitude_and_suffix_multiples_of_1000_and_1024() {
+            to_magnitude_and_suffix(1_000_000_001, SuffixType::Si),
-        assert_eq!(to_magnitude_and_suffix(128_000).unwrap(), "128kB");
+            "1.1GB"
-        assert_eq!(to_magnitude_and_suffix(1000 * 1024).unwrap(), "1.1MB");
+        );
-        assert_eq!(to_magnitude_and_suffix(1_000_000_000_000).unwrap(), "1TB");
     }
 
     #[test]
@@ -279,6 +265,13 @@ mod tests {
         assert_eq!(format!("{}", BlockSize::Bytes(3 * 1024 * 1024)), "3M");
     }
 
+    #[test]
+    fn test_block_size_display_multiples_of_1000_and_1024() {
+        assert_eq!(format!("{}", BlockSize::Bytes(128_000)), "128kB");
+        assert_eq!(format!("{}", BlockSize::Bytes(1000 * 1024)), "1.1MB");
+        assert_eq!(format!("{}", BlockSize::Bytes(1_000_000_000_000)), "1TB");
+    }
+
     #[test]
     fn test_default_block_size() {
         assert_eq!(BlockSize::Bytes(1024), BlockSize::default());

src/uu/df/src/df.rs

@@ -11,7 +11,7 @@ mod columns;
 mod filesystem;
 mod table;
 
-use blocks::{HumanReadable, SizeFormat};
+use blocks::HumanReadable;
 use table::HeaderMode;
 use uucore::display::Quotable;
 use uucore::error::{UError, UResult, USimpleError};
@@ -71,7 +71,7 @@ static OUTPUT_FIELD_LIST: [&str; 12] = [
 struct Options {
     show_local_fs: bool,
     show_all_fs: bool,
-    size_format: SizeFormat,
+    human_readable: Option<HumanReadable>,
     block_size: BlockSize,
     header_mode: HeaderMode,
 
@@ -100,7 +100,7 @@ impl Default for Options {
             show_local_fs: Default::default(),
             show_all_fs: Default::default(),
             block_size: Default::default(),
-            size_format: Default::default(),
+            human_readable: Default::default(),
             header_mode: Default::default(),
             include: Default::default(),
             exclude: Default::default(),
@@ -200,13 +200,13 @@ impl Options {
                     HeaderMode::Default
                 }
             },
-            size_format: {
+            human_readable: {
                 if matches.is_present(OPT_HUMAN_READABLE_BINARY) {
-                    SizeFormat::HumanReadable(HumanReadable::Binary)
+                    Some(HumanReadable::Binary)
                 } else if matches.is_present(OPT_HUMAN_READABLE_DECIMAL) {
-                    SizeFormat::HumanReadable(HumanReadable::Decimal)
+                    Some(HumanReadable::Decimal)
                 } else {
-                    SizeFormat::StaticBlockSize
+                    None
                 }
             },
             include,

src/uu/df/src/table.rs

@@ -7,10 +7,9 @@
 //!
 //! A table ([`Table`]) comprises a header row ([`Header`]) and a
 //! collection of data rows ([`Row`]), one per filesystem.
-use number_prefix::NumberPrefix;
 use unicode_width::UnicodeWidthStr;
 
-use crate::blocks::{HumanReadable, SizeFormat};
+use crate::blocks::{to_magnitude_and_suffix, SuffixType};
 use crate::columns::{Alignment, Column};
 use crate::filesystem::Filesystem;
 use crate::{BlockSize, Options};
@@ -213,38 +212,26 @@ impl<'a> RowFormatter<'a> {
         Self { row, options }
     }
 
-    /// Get a human readable string giving the scaled version of the input number.
-    fn scaled_human_readable(&self, size: u64, human_readable: HumanReadable) -> String {
-        let number_prefix = match human_readable {
-            HumanReadable::Decimal => NumberPrefix::decimal(size as f64),
-            HumanReadable::Binary => NumberPrefix::binary(size as f64),
-        };
-        match number_prefix {
-            NumberPrefix::Standalone(bytes) => bytes.to_string(),
-            NumberPrefix::Prefixed(prefix, bytes) => format!("{:.1}{}", bytes, prefix.symbol()),
-        }
-    }
-
     /// Get a string giving the scaled version of the input number.
     ///
     /// The scaling factor is defined in the `options` field.
     fn scaled_bytes(&self, size: u64) -> String {
-        match self.options.size_format {
+        if let Some(h) = self.options.human_readable {
-            SizeFormat::HumanReadable(h) => self.scaled_human_readable(size, h),
+            to_magnitude_and_suffix(size.into(), SuffixType::HumanReadable(h))
-            SizeFormat::StaticBlockSize => {
+        } else {
             let BlockSize::Bytes(d) = self.options.block_size;
             (size as f64 / d as f64).ceil().to_string()
         }
     }
-    }
 
     /// Get a string giving the scaled version of the input number.
     ///
     /// The scaling factor is defined in the `options` field.
     fn scaled_inodes(&self, size: u64) -> String {
-        match self.options.size_format {
+        if let Some(h) = self.options.human_readable {
-            SizeFormat::HumanReadable(h) => self.scaled_human_readable(size, h),
+            to_magnitude_and_suffix(size.into(), SuffixType::HumanReadable(h))
-            SizeFormat::StaticBlockSize => size.to_string(),
+        } else {
+            size.to_string()
         }
     }
 
@@ -450,7 +437,7 @@ impl fmt::Display for Table {
 #[cfg(test)]
 mod tests {
 
-    use crate::blocks::{HumanReadable, SizeFormat};
+    use crate::blocks::HumanReadable;
     use crate::columns::Column;
     use crate::table::{Header, HeaderMode, Row, RowFormatter};
     use crate::{BlockSize, Options};
@@ -715,7 +702,7 @@ mod tests {
     #[test]
     fn test_row_formatter_with_human_readable_si() {
         let options = Options {
-            size_format: SizeFormat::HumanReadable(HumanReadable::Decimal),
+            human_readable: Some(HumanReadable::Decimal),
             columns: COLUMNS_WITH_FS_TYPE.to_vec(),
             ..Default::default()
         };
@@ -734,22 +721,14 @@ mod tests {
         let fmt = RowFormatter::new(&row, &options);
         assert_eq!(
             fmt.get_values(),
-            vec!(
+            vec!("my_device", "my_type", "4k", "1k", "3k", "25%", "my_mount")
-                "my_device",
-                "my_type",
-                "4.0k",
-                "1.0k",
-                "3.0k",
-                "25%",
-                "my_mount"
-            )
         );
     }
 
     #[test]
     fn test_row_formatter_with_human_readable_binary() {
         let options = Options {
-            size_format: SizeFormat::HumanReadable(HumanReadable::Binary),
+            human_readable: Some(HumanReadable::Binary),
             columns: COLUMNS_WITH_FS_TYPE.to_vec(),
             ..Default::default()
         };
@@ -768,15 +747,7 @@ mod tests {
         let fmt = RowFormatter::new(&row, &options);
         assert_eq!(
             fmt.get_values(),
-            vec!(
+            vec!("my_device", "my_type", "4K", "1K", "3K", "25%", "my_mount")
-                "my_device",
-                "my_type",
-                "4.0Ki",
-                "1.0Ki",
-                "3.0Ki",
-                "25%",
-                "my_mount"
-            )
         );
     }