diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs index a1b3596cc4c..46c51440d31 100644 --- a/src/uu/du/src/du.rs +++ b/src/uu/du/src/du.rs @@ -963,38 +963,61 @@ 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 { - 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 handle_block_size_arg_override(matches: &ArgMatches) -> Option { +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 parse_size_format(matches: &ArgMatches) -> UResult { + let block_size_value_or_default_fallback = parse_block_size_arg_or_default_fallback(matches)?; let candidates = [ ( SizeFormat::BlockSize(1), - get_block_size_arg_index_if_present(matches, options::BYTES), + get_size_format_flag_arg_index_if_present(matches, options::BYTES), ), ( SizeFormat::BlockSize(1024), - get_block_size_arg_index_if_present(matches, options::BLOCK_SIZE_1K), + get_size_format_flag_arg_index_if_present(matches, options::BLOCK_SIZE_1K), ), ( SizeFormat::BlockSize(1024 * 1024), - get_block_size_arg_index_if_present(matches, options::BLOCK_SIZE_1M), + get_size_format_flag_arg_index_if_present(matches, options::BLOCK_SIZE_1M), + ), + ( + SizeFormat::HumanBinary, + get_size_format_flag_arg_index_if_present(matches, options::HUMAN_READABLE), + ), + ( + SizeFormat::HumanDecimal, + get_size_format_flag_arg_index_if_present(matches, options::SI), + ), + ( + block_size_value_or_default_fallback.clone(), + get_size_format_flag_arg_index_if_present(matches, options::BLOCK_SIZE), ), ]; - candidates + Ok(candidates .into_iter() .filter(|(_, idx)| idx.is_some()) .max_by_key(|&(_, idx)| idx.unwrap_or(0)) .map(|(size_format, _)| size_format) + .unwrap_or(block_size_value_or_default_fallback)) } #[uucore::main] @@ -1048,21 +1071,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 = parse_size_format(&matches)?; let traversal_options = TraversalOptions { all: matches.get_flag(options::ALL), diff --git a/tests/by-util/test_du.rs b/tests/by-util/test_du.rs index 2be6ef02c36..5d081c90b1e 100644 --- a/tests/by-util/test_du.rs +++ b/tests/by-util/test_du.rs @@ -2064,24 +2064,45 @@ 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"]), + (["--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() { @@ -2095,7 +2116,7 @@ fn test_block_size_args_override() { let single_args = ts .ucmd() .arg(dir) - .arg(expected) + .args(&expected) .succeeds() .stdout_move_str(); @@ -2111,23 +2132,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() { @@ -2151,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'"); +}