From 2fba27b939f0d7c91666da457fab591edac1fdf7 Mon Sep 17 00:00:00 2001 From: cerdelen Date: Thu, 5 Feb 2026 15:35:26 +0100 Subject: [PATCH 1/5] du: Extend size_format flag override tests --- tests/by-util/test_du.rs | 49 ++++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/tests/by-util/test_du.rs b/tests/by-util/test_du.rs index 2be6ef02c36..0b2c65497f5 100644 --- a/tests/by-util/test_du.rs +++ b/tests/by-util/test_du.rs @@ -2064,24 +2064,42 @@ fn test_block_size_args_override() { let ts = TestScenario::new(util_name!()); let at = &ts.fixtures; let dir = "override_args_dir"; + let nested_dir = "override_args_dir/nested_dir"; + let nested_dir_2 = "override_args_dir/nested_dir_2"; - at.mkdir(dir); - let fpath = at.plus(format!("{dir}/file")); + at.mkdir_all(nested_dir); + at.mkdir_all(nested_dir_2); + let fpath = at.plus(format!("{nested_dir}/file")); std::fs::File::create(fpath) .expect("cannot create test file") .set_len(100_000_000) .expect("cannot set file size"); - let fpath2 = at.plus(format!("{dir}/file_2")); + let fpath2 = at.plus(format!("{nested_dir}/file_2")); std::fs::File::create(fpath2) .expect("cannot create test file") .set_len(100_000_000) .expect("cannot set file size"); + let fpath = at.plus(format!("{nested_dir_2}/file_3")); + std::fs::File::create(fpath) + .expect("cannot create test file") + .set_len(100_000) + .expect("cannot set file size"); + + let fpath2 = at.plus(format!("{nested_dir_2}/file_4")); + std::fs::File::create(fpath2) + .expect("cannot create test file") + .set_len(100) + .expect("cannot set file size"); + let test_cases = [ - (["-sk", "-m"], "-sm"), - (["-sk", "-b"], "-sb"), - (["-sm", "-k"], "-sk"), + (["-sk", "-m"], vec!["-sm"]), + (["-sk", "-b"], vec!["-sb"]), + (["-sm", "-k"], vec!["-sk"]), + (["-sk", "--si"], vec!["-s", "--si"]), + (["-sk", "-h"], vec!["-s", "-h"]), + (["-sm", "--block-size=128"], vec!["-s", "--block-size=128"]), ]; for (idx, (overwriting_args, expected)) in test_cases.into_iter().enumerate() { @@ -2095,7 +2113,7 @@ fn test_block_size_args_override() { let single_args = ts .ucmd() .arg(dir) - .arg(expected) + .args(&expected) .succeeds() .stdout_move_str(); @@ -2111,23 +2129,30 @@ fn test_block_override_b_still_has_apparent_size() { let ts = TestScenario::new(util_name!()); let at = &ts.fixtures; let dir = "override_args_dir"; + let nested_dir = "override_args_dir/nested_dir"; - at.mkdir(dir); - let fpath = at.plus(format!("{dir}/file")); + at.mkdir_all(nested_dir); + let fpath = at.plus(format!("{nested_dir}/file")); std::fs::File::create(fpath) .expect("cannot create test file") .set_len(100_000_000) .expect("cannot set file size"); - let fpath2 = at.plus(format!("{dir}/file_2")); + let fpath2 = at.plus(format!("{nested_dir}/file_2")); std::fs::File::create(fpath2) .expect("cannot create test file") .set_len(100_000_000) .expect("cannot set file size"); let test_cases = [ - (["-sb", "-m"], ["-sm", "--apparent-size"]), - (["-sb", "-k"], ["-sk", "--apparent-size"]), + (["-b", "-m"], ["-m", "--apparent-size"]), + (["-b", "-k"], ["-k", "--apparent-size"]), + (["-b", "--si"], ["--si", "--apparent-size"]), + (["-b", "-h"], ["-h", "--apparent-size"]), + ( + ["-b", "--block-size=128"], + ["--block-size=128", "--apparent-size"], + ), ]; for (idx, (overwriting_args, expected)) in test_cases.into_iter().enumerate() { From 43d9dd8e82c99137c5030a7488db555afb38b877 Mon Sep 17 00:00:00 2001 From: cerdelen Date: Thu, 5 Feb 2026 15:36:24 +0100 Subject: [PATCH 2/5] du: Extend size_override implementation for '-h' '--si' '-h' '--block-size' --- src/uu/du/src/du.rs | 73 +++++++++++++++++++++++++++++---------------- 1 file changed, 48 insertions(+), 25 deletions(-) diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs index a1b3596cc4c..289c38f4fe8 100644 --- a/src/uu/du/src/du.rs +++ b/src/uu/du/src/du.rs @@ -963,7 +963,7 @@ fn read_files_from(file_name: &OsStr) -> Result, std::io::Error> { Ok(paths) } -fn get_block_size_arg_index_if_present(matches: &ArgMatches, flag: &str) -> Option { +fn get_size_format_flag_arg_index_if_present(matches: &ArgMatches, flag: &str) -> Option { if matches.get_flag(flag) { // Indices of returns index even if flag is not present, thats why we need to if guard it matches @@ -974,27 +974,64 @@ fn get_block_size_arg_index_if_present(matches: &ArgMatches, flag: &str) -> Opti } } -fn handle_block_size_arg_override(matches: &ArgMatches) -> Option { +fn get_size_format_string_arg_index_if_present(matches: &ArgMatches, flag: &str) -> Option { + if matches.get_one::(flag).is_some() { + // Indices of returns index even if flag is not present, thats why we need to if guard it + matches + .indices_of(flag) + .and_then(|mut indices| indices.next_back()) + } else { + None + } +} + +fn handle_block_size_arg_override(matches: &ArgMatches) -> UResult { let candidates = [ ( - SizeFormat::BlockSize(1), - get_block_size_arg_index_if_present(matches, options::BYTES), + Some(SizeFormat::BlockSize(1)), + get_size_format_flag_arg_index_if_present(matches, options::BYTES), + ), + ( + Some(SizeFormat::BlockSize(1024)), + get_size_format_flag_arg_index_if_present(matches, options::BLOCK_SIZE_1K), + ), + ( + Some(SizeFormat::BlockSize(1024 * 1024)), + get_size_format_flag_arg_index_if_present(matches, options::BLOCK_SIZE_1M), + ), + ( + Some(SizeFormat::HumanBinary), + get_size_format_flag_arg_index_if_present(matches, options::HUMAN_READABLE), ), ( - SizeFormat::BlockSize(1024), - get_block_size_arg_index_if_present(matches, options::BLOCK_SIZE_1K), + Some(SizeFormat::HumanDecimal), + get_size_format_flag_arg_index_if_present(matches, options::SI), ), + // for block size we want to handle this not in closure but in else block later ( - SizeFormat::BlockSize(1024 * 1024), - get_block_size_arg_index_if_present(matches, options::BLOCK_SIZE_1M), + None, + get_size_format_string_arg_index_if_present(matches, options::BLOCK_SIZE), ), ]; - candidates + let ret = candidates .into_iter() .filter(|(_, idx)| idx.is_some()) .max_by_key(|&(_, idx)| idx.unwrap_or(0)) - .map(|(size_format, _)| size_format) + .map(|(size_format, _)| size_format); + + if let Some(Some(size_format)) = ret { + Ok(size_format) + } else { + // we handle block_size option here manually + let block_size_str = matches.get_one::(options::BLOCK_SIZE); + let block_size = read_block_size(block_size_str.map(AsRef::as_ref))?; + if block_size == 0 { + return Err(std::io::Error::other(translate!("du-error-invalid-block-size-argument", "option" => options::BLOCK_SIZE, "value" => block_size_str.map_or("???BUG", |v| v).quote())) + .into()); + } + Ok(SizeFormat::BlockSize(block_size)) + } } #[uucore::main] @@ -1048,21 +1085,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .map_or(MetadataTimeField::Modification, |s| s.as_str().into()) }); - let size_format = if matches.get_flag(options::HUMAN_READABLE) { - SizeFormat::HumanBinary - } else if matches.get_flag(options::SI) { - SizeFormat::HumanDecimal - } else if let Some(size_format) = handle_block_size_arg_override(&matches) { - size_format - } else { - let block_size_str = matches.get_one::(options::BLOCK_SIZE); - let block_size = read_block_size(block_size_str.map(AsRef::as_ref))?; - if block_size == 0 { - return Err(std::io::Error::other(translate!("du-error-invalid-block-size-argument", "option" => options::BLOCK_SIZE, "value" => block_size_str.map_or("???BUG", |v| v).quote())) - .into()); - } - SizeFormat::BlockSize(block_size) - }; + let size_format = handle_block_size_arg_override(&matches)?; let traversal_options = TraversalOptions { all: matches.get_flag(options::ALL), From edf51cc271e0d54b8239f67e14fa7c69dcaf696e Mon Sep 17 00:00:00 2001 From: cerdelen Date: Thu, 5 Feb 2026 16:00:55 +0100 Subject: [PATCH 3/5] du: Extend size_format flag override tests --- tests/by-util/test_du.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/by-util/test_du.rs b/tests/by-util/test_du.rs index 0b2c65497f5..a96347fec26 100644 --- a/tests/by-util/test_du.rs +++ b/tests/by-util/test_du.rs @@ -2100,6 +2100,9 @@ fn test_block_size_args_override() { (["-sk", "--si"], vec!["-s", "--si"]), (["-sk", "-h"], vec!["-s", "-h"]), (["-sm", "--block-size=128"], vec!["-s", "--block-size=128"]), + (["--block-size=128", "-b"], vec!["-b"]), + (["--si", "-b"], vec!["-b"]), + (["-h", "-b"], vec!["-b"]), ]; for (idx, (overwriting_args, expected)) in test_cases.into_iter().enumerate() { From 1f5f6121a74b9ccb04ec913610d7b127ae6e3955 Mon Sep 17 00:00:00 2001 From: cerdelen Date: Fri, 6 Feb 2026 22:20:58 +0100 Subject: [PATCH 4/5] du: Extend size_format override tests for correct error return with invalid block-size value regardless of override --- tests/by-util/test_du.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/by-util/test_du.rs b/tests/by-util/test_du.rs index a96347fec26..5d081c90b1e 100644 --- a/tests/by-util/test_du.rs +++ b/tests/by-util/test_du.rs @@ -2179,3 +2179,27 @@ fn test_block_override_b_still_has_apparent_size() { ); } } + +#[test] +fn test_overriding_block_size_arg_with_invalid_value_still_errors() { + new_ucmd!() + .args(&["--block-size=abc", "-m"]) + .fails_with_code(1) + .stderr_contains("invalid --block-size argument 'abc'"); + new_ucmd!() + .args(&["--block-size=abc", "-k"]) + .fails_with_code(1) + .stderr_contains("invalid --block-size argument 'abc'"); + new_ucmd!() + .args(&["--block-size=abc", "-b"]) + .fails_with_code(1) + .stderr_contains("invalid --block-size argument 'abc'"); + new_ucmd!() + .args(&["--block-size=abc", "-h"]) + .fails_with_code(1) + .stderr_contains("invalid --block-size argument 'abc'"); + new_ucmd!() + .args(&["--block-size=abc", "--si"]) + .fails_with_code(1) + .stderr_contains("invalid --block-size argument 'abc'"); +} From e06d1fedfc4aee400ec40b9c31383356ff52a14a Mon Sep 17 00:00:00 2001 From: cerdelen Date: Fri, 6 Feb 2026 22:21:30 +0100 Subject: [PATCH 5/5] du: Refactor size-format parsing code --- src/uu/du/src/du.rs | 60 +++++++++++++++++---------------------------- 1 file changed, 23 insertions(+), 37 deletions(-) diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs index 289c38f4fe8..46c51440d31 100644 --- a/src/uu/du/src/du.rs +++ b/src/uu/du/src/du.rs @@ -963,75 +963,61 @@ fn read_files_from(file_name: &OsStr) -> Result, std::io::Error> { Ok(paths) } -fn get_size_format_flag_arg_index_if_present(matches: &ArgMatches, flag: &str) -> Option { - if matches.get_flag(flag) { - // Indices of returns index even if flag is not present, thats why we need to if guard it +fn get_size_format_flag_arg_index_if_present(matches: &ArgMatches, arg: &str) -> Option { + if let Some(clap::parser::ValueSource::CommandLine) = matches.value_source(arg) { matches - .indices_of(flag) + .indices_of(arg) .and_then(|mut indices| indices.next_back()) } else { None } } -fn get_size_format_string_arg_index_if_present(matches: &ArgMatches, flag: &str) -> Option { - if matches.get_one::(flag).is_some() { - // Indices of returns index even if flag is not present, thats why we need to if guard it - matches - .indices_of(flag) - .and_then(|mut indices| indices.next_back()) - } else { - None +fn parse_block_size_arg_or_default_fallback(matches: &ArgMatches) -> UResult { + let block_size_str = matches.get_one::(options::BLOCK_SIZE); + let block_size = read_block_size(block_size_str.map(AsRef::as_ref))?; + if block_size == 0 { + return Err(std::io::Error::other(translate!("du-error-invalid-block-size-argument", "option" => options::BLOCK_SIZE, "value" => block_size_str.map_or("???BUG", |v| v).quote())) + .into()); } + Ok(SizeFormat::BlockSize(block_size)) } -fn handle_block_size_arg_override(matches: &ArgMatches) -> UResult { +fn parse_size_format(matches: &ArgMatches) -> UResult { + let block_size_value_or_default_fallback = parse_block_size_arg_or_default_fallback(matches)?; let candidates = [ ( - Some(SizeFormat::BlockSize(1)), + SizeFormat::BlockSize(1), get_size_format_flag_arg_index_if_present(matches, options::BYTES), ), ( - Some(SizeFormat::BlockSize(1024)), + SizeFormat::BlockSize(1024), get_size_format_flag_arg_index_if_present(matches, options::BLOCK_SIZE_1K), ), ( - Some(SizeFormat::BlockSize(1024 * 1024)), + SizeFormat::BlockSize(1024 * 1024), get_size_format_flag_arg_index_if_present(matches, options::BLOCK_SIZE_1M), ), ( - Some(SizeFormat::HumanBinary), + SizeFormat::HumanBinary, get_size_format_flag_arg_index_if_present(matches, options::HUMAN_READABLE), ), ( - Some(SizeFormat::HumanDecimal), + SizeFormat::HumanDecimal, get_size_format_flag_arg_index_if_present(matches, options::SI), ), - // for block size we want to handle this not in closure but in else block later ( - None, - get_size_format_string_arg_index_if_present(matches, options::BLOCK_SIZE), + block_size_value_or_default_fallback.clone(), + get_size_format_flag_arg_index_if_present(matches, options::BLOCK_SIZE), ), ]; - let ret = candidates + Ok(candidates .into_iter() .filter(|(_, idx)| idx.is_some()) .max_by_key(|&(_, idx)| idx.unwrap_or(0)) - .map(|(size_format, _)| size_format); - - if let Some(Some(size_format)) = ret { - Ok(size_format) - } else { - // we handle block_size option here manually - let block_size_str = matches.get_one::(options::BLOCK_SIZE); - let block_size = read_block_size(block_size_str.map(AsRef::as_ref))?; - if block_size == 0 { - return Err(std::io::Error::other(translate!("du-error-invalid-block-size-argument", "option" => options::BLOCK_SIZE, "value" => block_size_str.map_or("???BUG", |v| v).quote())) - .into()); - } - Ok(SizeFormat::BlockSize(block_size)) - } + .map(|(size_format, _)| size_format) + .unwrap_or(block_size_value_or_default_fallback)) } #[uucore::main] @@ -1085,7 +1071,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .map_or(MetadataTimeField::Modification, |s| s.as_str().into()) }); - let size_format = handle_block_size_arg_override(&matches)?; + let size_format = parse_size_format(&matches)?; let traversal_options = TraversalOptions { all: matches.get_flag(options::ALL),