ls: round block counts up when scaling to a block size

src/uu/ls/src/ls.rs

@@ -1342,15 +1342,13 @@ fn write_directory_entries<O: LsOutput>(
     output: &mut O,
 ) -> UResult<()> {
     if config.format == Format::Long || config.alloc_size {
-        let total_size: u64 = entries
+        // GNU adds up the allocated bytes and scales the sum once, so a
+        // partly used block is rounded up for the total, not once per file.
+        let total_bytes: u64 = entries
             .iter()
-            .map(|item| {
-                item.metadata()
-                    .as_ref()
-                    .map_or(0, |md| get_block_size(md, config))
-            })
+            .map(|item| item.metadata().as_ref().map_or(0, |md| get_block_bytes(md)))
             .sum();
-        output.write_total(total_size, config)?;
+        output.write_total(scale_block_bytes(total_bytes, config), config)?;
     }
 
     if matches!(output.stream_mode(), StreamMode::Streaming) {
@@ -1564,23 +1562,22 @@ fn get_metadata_with_deref_opt(p_buf: &Path, dereference: bool) -> std::io::Resu
     }
 }
 
+/// Allocated size of a file in bytes, before it is scaled to the block size.
+///
+/// This is the figure the `total` line sums: GNU scales the sum once, so
+/// scaling every file first and adding the results would round several times
+/// over and overshoot the total.
 #[allow(unused_variables)]
-fn get_block_size(md: &Metadata, config: &Config) -> u64 {
+fn get_block_bytes(md: &Metadata) -> u64 {
     /* GNU ls will display sizes in terms of block size
        md.len() will differ from this value when the file has some holes
     */
     #[cfg(unix)]
     {
-        use uucore::format::human::SizeFormat;
-
-        let raw_blocks = if md.file_type().is_char_device() || md.file_type().is_block_device() {
+        if md.file_type().is_char_device() || md.file_type().is_block_device() {
             0u64
         } else {
             md.blocks() * 512
-        };
-        match config.size_format {
-            SizeFormat::Binary | SizeFormat::Decimal => raw_blocks,
-            SizeFormat::Bytes => raw_blocks / config.block_size,
         }
     }
     #[cfg(not(unix))]
@@ -1590,6 +1587,35 @@ fn get_block_size(md: &Metadata, config: &Config) -> u64 {
     }
 }
 
+/// Scale an allocated size in bytes to `config.block_size`.
+///
+/// A partly used block still occupies a whole block, so the division rounds
+/// up: with `--block-size=1000`, 4096 allocated bytes are five blocks, not
+/// four. `-h`/`--si` keep the byte count, which the human-readable formatter
+/// scales itself.
+#[allow(unused_variables)]
+fn scale_block_bytes(bytes: u64, config: &Config) -> u64 {
+    #[cfg(unix)]
+    {
+        use uucore::format::human::SizeFormat;
+
+        match config.size_format {
+            SizeFormat::Binary | SizeFormat::Decimal => bytes,
+            SizeFormat::Bytes => bytes.div_ceil(config.block_size),
+        }
+    }
+    // On non-unix `get_block_bytes` falls back to the file size rather than an
+    // allocation figure, which was never scaled here. Left as it was.
+    #[cfg(not(unix))]
+    {
+        bytes
+    }
+}
+
+fn get_block_size(md: &Metadata, config: &Config) -> u64 {
+    scale_block_bytes(get_block_bytes(md), config)
+}
+
 #[cfg(unix)]
 fn file_is_executable(md: &Metadata) -> bool {
     // Mode always returns u32, but the flags might not be, based on the platform

tests/by-util/test_ls.rs

@@ -6078,6 +6078,46 @@ fn test_posixly_correct_and_block_size_env_vars_with_k() {
         .stdout_contains(" 1024 ");
 }
 
+#[test]
+#[cfg(unix)]
+fn test_ls_block_size_rounds_up() {
+    // A block that is only partly used still occupies a whole block, so
+    // scaling an allocation to the block size rounds up. The total scales the
+    // sum of the allocations once; rounding every file first and adding the
+    // results overshoots it.
+    use std::os::unix::fs::MetadataExt;
+
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.mkdir("dir");
+    for name in ["dir/a", "dir/b", "dir/c"] {
+        at.write(name, "x");
+    }
+
+    let allocated: Vec<u64> = ["dir/a", "dir/b", "dir/c"]
+        .iter()
+        .map(|name| at.metadata(name).blocks() * 512)
+        .collect();
+    // An allocation that is a whole number of blocks, or none at all, cannot
+    // tell rounding up from rounding down.
+    if allocated
+        .iter()
+        .any(|bytes| bytes == &0 || bytes % 1000 == 0)
+    {
+        return;
+    }
+    let sizes: Vec<u64> = allocated.iter().map(|bytes| bytes.div_ceil(1000)).collect();
+    let total = allocated.iter().sum::<u64>().div_ceil(1000);
+
+    ucmd.arg("-s")
+        .arg("--block-size=1000")
+        .arg("dir")
+        .succeeds()
+        .stdout_is(format!(
+            "total {total}\n{} a\n{} b\n{} c\n",
+            sizes[0], sizes[1], sizes[2]
+        ));
+}
+
 #[test]
 fn test_ls_invalid_block_size() {
     new_ucmd!()