diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 6c861aa409f..e83beb60a70 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1342,15 +1342,13 @@ fn write_directory_entries( 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 diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 08233e3ebe3..4bed8c4a54a 100644 --- a/tests/by-util/test_ls.rs +++ b/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 = ["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 = allocated.iter().map(|bytes| bytes.div_ceil(1000)).collect(); + let total = allocated.iter().sum::().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!()