sort: reject --batch-size arguments above the fd soft limit

src/uu/sort/src/sort.rs

@@ -1520,6 +1520,16 @@ pub(crate) fn fd_soft_limit() -> Option<usize> {
     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<usize> {
+    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<usize> {
     fn count_dir(path: &str) -> Option<usize> {
@@ -2259,54 +2269,51 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         .map(String::from);
 
     if let Some(n_merge) = matches.get_one::<String>(options::BATCH_SIZE) {
-        match n_merge.parse::<usize>() {
-            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::<usize>() {
+            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));

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!();