Skip to content

Commit a2d74e5

Browse files
committed
Address review comments: use Release/Acquire ordering, add comments, use match
1 parent 1b8cbf9 commit a2d74e5

File tree

2 files changed

+9
-7
lines changed

2 files changed

+9
-7
lines changed

src/uu/seq/src/seq.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
103103
// signal disposition for proper pipeline behavior (GNU compatibility).
104104
#[cfg(unix)]
105105
if !signals::sigpipe_was_ignored() {
106+
// Ignore the return value: if setting signal handler fails, we continue anyway.
107+
// The worst case is we don't get proper SIGPIPE behavior, but seq will still work.
106108
let _ = signals::enable_pipe_errors();
107109
}
108110

@@ -223,8 +225,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
223225
padding,
224226
);
225227

226-
if let Err(err) = result {
227-
if err.kind() == std::io::ErrorKind::BrokenPipe {
228+
match result {
229+
Ok(()) => Ok(()),
230+
Err(err) if err.kind() == std::io::ErrorKind::BrokenPipe => {
228231
// GNU seq prints the Broken pipe message but still exits with status 0
229232
// unless SIGPIPE was explicitly ignored, in which case it should fail.
230233
let err = err.map_err_context(|| "write error".into());
@@ -233,11 +236,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
233236
if signals::sigpipe_was_ignored() {
234237
uucore::error::set_exit_code(1);
235238
}
236-
return Ok(());
239+
Ok(())
237240
}
238-
return Err(err.map_err_context(|| "write error".into()));
241+
Err(err) => Err(err.map_err_context(|| "write error".into())),
239242
}
240-
Ok(())
241243
}
242244

243245
pub fn uu_app() -> Command {

src/uucore/src/lib/features/signals.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,7 @@ pub unsafe extern "C" fn capture_sigpipe_state() {
448448
if unsafe { libc::sigaction(libc::SIGPIPE, ptr::null(), current.as_mut_ptr()) } == 0 {
449449
// SAFETY: sigaction succeeded, so current is initialized
450450
let ignored = unsafe { current.assume_init() }.sa_sigaction == libc::SIG_IGN;
451-
SIGPIPE_WAS_IGNORED.store(ignored, Ordering::Relaxed);
451+
SIGPIPE_WAS_IGNORED.store(ignored, Ordering::Release);
452452
}
453453
}
454454

@@ -480,7 +480,7 @@ macro_rules! init_sigpipe_capture {
480480
/// Returns whether SIGPIPE was ignored at process startup.
481481
#[cfg(unix)]
482482
pub fn sigpipe_was_ignored() -> bool {
483-
SIGPIPE_WAS_IGNORED.load(Ordering::Relaxed)
483+
SIGPIPE_WAS_IGNORED.load(Ordering::Acquire)
484484
}
485485

486486
#[cfg(not(unix))]

0 commit comments

Comments
 (0)