fix(text): address Copilot review — signal-handler ABI, locale prompt, streaming

jgarzik · claude · jgarzik · commit 6b83b0e52a03 · 2026-06-26T01:07:33.000Z
- pr: handle_sigint is now `extern "C"` (a Rust-ABI function registered as a C
  signal handler is undefined behavior when SIGINT is delivered); register it
  via a function-pointer cast to sighandler_t.
- patch: prompt_yes_no matches a non-empty answer against the locale's YESEXPR
  (plib::locale::is_affirmative) instead of a hardcoded 'y'/'Y'.
- expand, fold: process input one line at a time via a read_until loop instead
  of read_to_end, restoring bounded-memory streaming for large inputs/pipes
  (a &lt;newline&gt; never splits a multibyte character, and the per-line/column
  state resets at each newline, so the output bytes are identical).

The unexpand review note was checked and not actioned: the code does not turn a
single leading space into a tab (it matches its doc comment); the only related
divergence is the conservative direction on the rare `-t 1` case.

posixutils-text suite: 875 passed, 0 failed; clippy clean, fmt clean.

Co-Authored-By: Claude Opus 4.8 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/text/expand.rs b/text/expand.rs
@@ -7,7 +7,7 @@
 // SPDX-License-Identifier: MIT
 //
 
-use std::io::{self, BufWriter, Read, Write};
+use std::io::{self, BufRead, BufReader, BufWriter, Write};
 use std::path::{Path, PathBuf};
 
 use clap::Parser;
@@ -87,43 +87,51 @@ fn next_stop(tablist: &TabList, p: usize) -> usize {
 
 fn expand_file(tablist: &TabList, pathname: &Path) -> io::Result<()> {
     // open file, or stdin ("-" or no operand)
-    let mut file = input_stream_dashed(pathname)?;
-    let mut data = Vec::new();
-    file.read_to_end(&mut data)?;
+    let mut reader = BufReader::new(input_stream_dashed(pathname)?);
 
     let mut writer = BufWriter::new(io::stdout());
     // 0-based column = number of column positions consumed on the current line.
     let mut p: usize = 0;
 
-    for ch in mb_char_slices(&data) {
-        match ch {
-            b"\t" => {
-                let stop = next_stop(tablist, p);
-                for _ in p..stop {
-                    writer.write_all(b" ")?;
+    // Process one line at a time so arbitrarily large inputs (e.g. a long pipe)
+    // do not have to be buffered in full. A <newline> never splits a multibyte
+    // character, so chunking on it is safe; `p` resets at each line boundary.
+    let mut data: Vec<u8> = Vec::new();
+    loop {
+        data.clear();
+        if reader.read_until(b'\n', &mut data)? == 0 {
+            break;
+        }
+        for ch in mb_char_slices(&data) {
+            match ch {
+                b"\t" => {
+                    let stop = next_stop(tablist, p);
+                    for _ in p..stop {
+                        writer.write_all(b" ")?;
+                    }
+                    p = stop;
                 }
-                p = stop;
-            }
-            b"\x08" => {
-                // backspace: column count decrements, never below zero
-                writer.write_all(ch)?;
-                p = p.saturating_sub(1);
-            }
-            b"\r" | b"\n" => {
-                writer.write_all(ch)?;
-                p = 0;
-            }
-            _ => {
-                writer.write_all(ch)?;
-                // Advance by the character's display width under LC_CTYPE.
-                // Non-printable / undecodable bytes do not advance the column.
-                let w = std::str::from_utf8(ch)
-                    .ok()
-                    .and_then(|s| s.chars().next())
-                    .map(wcwidth_char)
-                    .unwrap_or(1);
-                if w > 0 {
-                    p += w as usize;
+                b"\x08" => {
+                    // backspace: column count decrements, never below zero
+                    writer.write_all(ch)?;
+                    p = p.saturating_sub(1);
+                }
+                b"\r" | b"\n" => {
+                    writer.write_all(ch)?;
+                    p = 0;
+                }
+                _ => {
+                    writer.write_all(ch)?;
+                    // Advance by the character's display width under LC_CTYPE.
+                    // Non-printable / undecodable bytes do not advance the column.
+                    let w = std::str::from_utf8(ch)
+                        .ok()
+                        .and_then(|s| s.chars().next())
+                        .map(wcwidth_char)
+                        .unwrap_or(1);
+                    if w > 0 {
+                        p += w as usize;
+                    }
                 }
             }
         }
diff --git a/text/fold.rs b/text/fold.rs
@@ -7,7 +7,7 @@
 // SPDX-License-Identifier: MIT
 //
 
-use std::io::{self, BufWriter, Read, Write};
+use std::io::{self, BufRead, BufReader, BufWriter, Write};
 use std::path::{Path, PathBuf};
 
 use clap::Parser;
@@ -72,57 +72,65 @@ fn find_last_blank(v: &[u8]) -> Option<usize> {
 
 fn fold_file(args: &Args, pathname: &Path) -> io::Result<()> {
     // open file, or stdin ("-" or no operand)
-    let mut file = input_stream_dashed(pathname)?;
-    let mut data = Vec::new();
-    file.read_to_end(&mut data)?;
+    let mut reader = BufReader::new(input_stream_dashed(pathname)?);
 
     let width = args.width as usize;
     let mut out = BufWriter::new(io::stdout());
     let mut line: Vec<u8> = Vec::new();
     let mut col: usize = 0;
 
-    for ch in mb_char_slices(&data) {
-        if ch == b"\n" {
-            line.extend_from_slice(ch);
-            out.write_all(&line)?;
-            line.clear();
-            col = 0;
-            continue;
+    // Process one input line at a time so large inputs are not buffered in
+    // full. A <newline> never splits a multibyte character; the wrap state
+    // (`line`/`col`) carries across reads and is flushed at each newline.
+    let mut data: Vec<u8> = Vec::new();
+    loop {
+        data.clear();
+        if reader.read_until(b'\n', &mut data)? == 0 {
+            break;
         }
+        for ch in mb_char_slices(&data) {
+            if ch == b"\n" {
+                line.extend_from_slice(ch);
+                out.write_all(&line)?;
+                line.clear();
+                col = 0;
+                continue;
+            }
 
-        let mut next = char_advance(col, ch, args.bytes);
-
-        // Insert breaks while appending this character would exceed the width
-        // and the line is non-empty (a single over-wide character on an empty
-        // line is emitted as-is; the spec leaves that case undefined).
-        while next > width && !line.is_empty() {
-            let mut folded = false;
-            if args.spaces {
-                if let Some(b) = find_last_blank(&line) {
-                    out.write_all(&line[..=b])?;
+            let mut next = char_advance(col, ch, args.bytes);
+
+            // Insert breaks while appending this character would exceed the width
+            // and the line is non-empty (a single over-wide character on an empty
+            // line is emitted as-is; the spec leaves that case undefined).
+            while next > width && !line.is_empty() {
+                let mut folded = false;
+                if args.spaces {
+                    if let Some(b) = find_last_blank(&line) {
+                        out.write_all(&line[..=b])?;
+                        out.write_all(b"\n")?;
+                        let remainder = line[b + 1..].to_vec();
+                        line = remainder;
+                        // Recompute the column over the kept remainder (its
+                        // characters are complete: blanks are char boundaries).
+                        col = 0;
+                        for rc in mb_char_slices(&line) {
+                            col = char_advance(col, rc, args.bytes);
+                        }
+                        folded = true;
+                    }
+                }
+                if !folded {
+                    out.write_all(&line)?;
                     out.write_all(b"\n")?;
-                    let remainder = line[b + 1..].to_vec();
-                    line = remainder;
-                    // Recompute the column over the kept remainder (its
-                    // characters are complete: blanks are char boundaries).
+                    line.clear();
                     col = 0;
-                    for rc in mb_char_slices(&line) {
-                        col = char_advance(col, rc, args.bytes);
-                    }
-                    folded = true;
                 }
+                next = char_advance(col, ch, args.bytes);
             }
-            if !folded {
-                out.write_all(&line)?;
-                out.write_all(b"\n")?;
-                line.clear();
-                col = 0;
-            }
-            next = char_advance(col, ch, args.bytes);
-        }
 
-        line.extend_from_slice(ch);
-        col = next;
+            line.extend_from_slice(ch);
+            col = next;
+        }
     }
 
     // Trailing partial line (input without a final newline).
diff --git a/text/patch_util/file_ops.rs b/text/patch_util/file_ops.rs
@@ -112,8 +112,9 @@ pub fn prompt_yes_no(prompt: &str) -> Option<bool> {
         return None;
     }
     let answer = line.trim();
-    // Default (empty) answer is affirmative, matching the "[y]" prompt.
-    Some(answer.is_empty() || answer.starts_with('y') || answer.starts_with('Y'))
+    // Default (empty) answer is affirmative, matching the "[y]" prompt; a
+    // non-empty answer is matched against the locale's YESEXPR.
+    Some(answer.is_empty() || plib::locale::is_affirmative(answer))
 }
 
 /// Strip leading path components from a path.
diff --git a/text/pr.rs b/text/pr.rs
@@ -102,7 +102,7 @@ fn pause() -> io::Result<()> {
 /// We flush any buffered output streams, restore the default disposition, and
 /// re-raise so the process dies from the signal (and the parent observes a
 /// signal death).
-fn handle_sigint(signal_code: libc::c_int) {
+extern "C" fn handle_sigint(signal_code: libc::c_int) {
     unsafe {
         // Flush all open stdio output streams so any pending diagnostic
         // reaches the terminal before we terminate.
@@ -118,7 +118,10 @@ fn install_sigint_handler() {
     let stdout_is_terminal = unsafe { libc::isatty(libc::STDOUT_FILENO) == 1 };
     if stdout_is_terminal {
         unsafe {
-            libc::signal(libc::SIGINT, handle_sigint as *const () as usize);
+            libc::signal(
+                libc::SIGINT,
+                handle_sigint as extern "C" fn(libc::c_int) as libc::sighandler_t,
+            );
         }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -112,8 +112,9 @@ pub fn prompt_yes_no(prompt: &str) -> Option<bool> {`
`112`	`112`	`return None;`
`113`	`113`	`}`
`114`	`114`	`let answer = line.trim();`
`115`		`- // Default (empty) answer is affirmative, matching the "[y]" prompt.`
`116`		`- Some(answer.is_empty() \|\| answer.starts_with('y') \|\| answer.starts_with('Y'))`
	`115`	`+ // Default (empty) answer is affirmative, matching the "[y]" prompt; a`
	`116`	`+ // non-empty answer is matched against the locale's YESEXPR.`
	`117`	`+ Some(answer.is_empty() \|\| plib::locale::is_affirmative(answer))`
`117`	`118`	`}`
`118`	`119`
`119`	`120`	`/// Strip leading path components from a path.`