diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 6639b744d5..ba8c1e5bce 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -1520,6 +1520,16 @@ pub(crate) fn fd_soft_limit() -> Option { None } +/// The largest `--batch-size` argument that can be honoured with the current file +/// descriptor soft limit, or `None` if that limit is unknown. +/// +/// Three descriptors are always in use (stdin, stdout and stderr) and are therefore +/// not available for merge inputs. +fn max_merge_batch_size() -> Option { + const RESERVED_STDIO: usize = 3; + fd_soft_limit().map(|limit| limit.saturating_sub(RESERVED_STDIO)) +} + #[cfg(unix)] pub(crate) fn current_open_fd_count() -> Option { fn count_dir(path: &str) -> Option { @@ -2259,54 +2269,51 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .map(String::from); if let Some(n_merge) = matches.get_one::(options::BATCH_SIZE) { - match n_merge.parse::() { - Ok(parsed_value) => { - if parsed_value < 2 { - show_error!( - "{}", - translate!("sort-invalid-batch-size-arg", "arg" => n_merge) - ); - return Err(UUsageError::new( - 2, - translate!("sort-minimum-batch-size-two"), - )); - } - settings.merge_batch_size = parsed_value; + // `None` means the value does not even fit in a `usize`, which is always too large. + let parsed_value = match n_merge.parse::() { + Ok(parsed_value) => Some(parsed_value), + Err(e) if *e.kind() == IntErrorKind::PosOverflow => None, + Err(_) => { + return Err(UUsageError::new( + 2, + translate!("sort-invalid-batch-size-arg", "arg" => n_merge), + )); } - Err(e) => { - let error_message = if *e.kind() == IntErrorKind::PosOverflow { - let batch_too_large = translate!( - "sort-batch-size-too-large", - "arg" => n_merge.quote() - ); + }; - #[cfg(target_os = "linux")] - { - show_error!("{batch_too_large}"); - - translate!( - "sort-maximum-batch-size-rlimit", - "rlimit" => { - fd_soft_limit().ok_or_else(|| { - UUsageError::new(2, translate!("sort-failed-fetch-rlimit")) - })? - } - ) - } - #[cfg(not(target_os = "linux"))] - { - batch_too_large - } - } else { - translate!( - "sort-invalid-batch-size-arg", - "arg" => n_merge, - ) - }; + if parsed_value.is_some_and(|value| value < 2) { + show_error!( + "{}", + translate!("sort-invalid-batch-size-arg", "arg" => n_merge) + ); + return Err(UUsageError::new( + 2, + translate!("sort-minimum-batch-size-two"), + )); + } - return Err(UUsageError::new(2, error_message)); - } + let max_batch_size = max_merge_batch_size(); + let too_large = match (parsed_value, max_batch_size) { + (None, _) => true, + (Some(value), Some(max)) => value > max, + (Some(_), None) => false, + }; + if too_large { + let batch_too_large = translate!( + "sort-batch-size-too-large", + "arg" => n_merge.quote() + ); + let error_message = match max_batch_size { + Some(max) => { + show_error!("{batch_too_large}"); + translate!("sort-maximum-batch-size-rlimit", "rlimit" => max) + } + None => batch_too_large, + }; + return Err(UUsageError::new(2, error_message)); } + + settings.merge_batch_size = parsed_value.unwrap_or(usize::MAX); } settings.line_ending = LineEnding::from_zero_flag(matches.get_flag(options::ZERO_TERMINATED)); diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index f12850f28e..49608b7a40 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -1549,6 +1549,66 @@ fn test_merge_batch_size_with_limit() { .stdout_only_fixture("merge_ints_interleaved.expected"); } +#[test] +// TODO(#7542): Re-enable on Android once we figure out why setting limit is broken. +#[cfg(target_os = "linux")] +fn test_batch_size_above_fd_limit_is_rejected() { + use rlimit::Resource; + // Only stdin, stdout and stderr are unavailable for merge inputs, so the + // largest acceptable --batch-size is the soft limit minus 3, here 27 - 3. + let limit_fd = 27; + let (at, mut ucmd) = at_and_ucmd!(); + at.write("gamma.txt", "delta\nalpha\n"); + ucmd.limit(Resource::NOFILE, limit_fd, limit_fd) + .arg("--batch-size=31") + .arg("gamma.txt") + .fails_with_code(2) + .stderr_contains("--batch-size argument '31' too large") + // 24 is forced by the limit above: 27 - 3 reserved descriptors. + .stderr_contains("maximum --batch-size argument with current rlimit is 24"); +} + +#[test] +#[cfg(target_os = "linux")] +fn test_batch_size_at_fd_limit_is_accepted() { + use rlimit::Resource; + let limit_fd = 27; + let (at, mut ucmd) = at_and_ucmd!(); + at.write("gamma.txt", "delta\nalpha\n"); + // 24 is the largest value the limit above allows, and sorting must still happen. + ucmd.limit(Resource::NOFILE, limit_fd, limit_fd) + .arg("--batch-size=24") + .arg("gamma.txt") + .succeeds() + .stdout_only("alpha\ndelta\n"); +} + +#[test] +#[cfg(target_os = "linux")] +fn test_merge_more_files_than_fd_limit() { + use rlimit::Resource; + let (at, mut ucmd) = at_and_ucmd!(); + // 40 single-line files cannot all be open at once with a soft limit of 24, + // so sort has to merge them in several batches through temporary files. + let count = 40; + let mut names = Vec::new(); + for i in 0..count { + let name = format!("fdlimit_{i:02}.txt"); + at.write(&name, &format!("{i:02}\n")); + names.push(name); + } + let mut expected = String::new(); + for i in 0..count { + writeln!(expected, "{i:02}").unwrap(); + } + let limit_fd = 24; + ucmd.limit(Resource::NOFILE, limit_fd, limit_fd) + .arg("-m") + .args(&names) + .succeeds() + .stdout_only(expected); +} + #[test] fn test_sigpipe_panic() { let mut cmd = new_ucmd!();