From a1f46d785f267297931b139274d3bc4155d22dd3 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Tue, 9 Jun 2020 21:57:05 +0200 Subject: [PATCH] rm "I/O error" error msg; just keep >0 exit status (re: 9011fa93) The bug was really that I/O errors in output builtins were undetectable by any means. Having a >0 exit status is sufficient. Adding an error message risks making existing ksh scripts noisier, or even breaking them if they redirect stderr to stdout. Note to self: in future, implement the minimum change necessary to fix bugs, nothing more. The fact that I needed to add four extra 2>/dev/null to the regression tests should have been a hint. src/cmd/ksh93/bltins/print.c, src/cmd/ksh93/data/msg.c, src/cmd/ksh93/include/io.h: - Remove "I/O error" message. src/cmd/ksh93/tests/builtins.sh: - Update to check for exit status only. src/cmd/ksh93/tests/basic.sh, src/cmd/ksh93/tests/coprocess.sh: - Revert four new '2>/dev/null' to suppress the error message. (cherry picked from commit 5e17be24d18455b575b6e98bc631c6935ffc795a) --- NEWS | 2 +- src/cmd/ksh93/bltins/print.c | 2 -- src/cmd/ksh93/data/msg.c | 1 - src/cmd/ksh93/include/io.h | 1 - src/cmd/ksh93/tests/basic.sh | 4 ++-- src/cmd/ksh93/tests/builtins.sh | 4 ++-- src/cmd/ksh93/tests/coprocess.sh | 4 ++-- 7 files changed, 7 insertions(+), 11 deletions(-) diff --git a/NEWS b/NEWS index a97a53f0e..82f1824fd 100644 --- a/NEWS +++ b/NEWS @@ -164,7 +164,7 @@ Any uppercase BUG_* names are modernish shell bug IDs. Ref.: https://github.com/att/ast/issues/425 https://github.com/att/ast/pull/442 -- Fix BUG_PUTIOERR: Output builtins now correctly detect and report +- Fix BUG_PUTIOERR: Output builtins now correctly detect input/output errors. This allows scripts to check for a nonzero exit status on the 'print', 'printf' and 'echo' builtins and prevent possible infinite loops if SIGPIPE is ignored. diff --git a/src/cmd/ksh93/bltins/print.c b/src/cmd/ksh93/bltins/print.c index 6d2f30203..d00aca628 100644 --- a/src/cmd/ksh93/bltins/print.c +++ b/src/cmd/ksh93/bltins/print.c @@ -373,8 +373,6 @@ skip2: if (sfsync(outfile) < 0) exitval = 1; } - if (exitval) - errormsg(SH_DICT, 2, e_io); return(exitval); } diff --git a/src/cmd/ksh93/data/msg.c b/src/cmd/ksh93/data/msg.c index bab44c11e..655e509cc 100644 --- a/src/cmd/ksh93/data/msg.c +++ b/src/cmd/ksh93/data/msg.c @@ -89,7 +89,6 @@ const char e_access[] = "permission denied"; const char e_direct[] = "bad directory"; const char e_file[] = "%s: bad file unit number"; const char e_redirect[] = "redirection failed"; -const char e_io[] = "I/O error"; const char e_trap[] = "%s: bad trap"; const char e_readonly[] = "%s: is read only"; const char e_badfield[] = "%d: negative field size"; diff --git a/src/cmd/ksh93/include/io.h b/src/cmd/ksh93/include/io.h index 5836d0dc0..f82c5ca10 100644 --- a/src/cmd/ksh93/include/io.h +++ b/src/cmd/ksh93/include/io.h @@ -97,7 +97,6 @@ extern const char e_tmpcreate[]; extern const char e_exists[]; extern const char e_file[]; extern const char e_redirect[]; -extern const char e_io[]; extern const char e_formspec[]; extern const char e_badregexp[]; extern const char e_open[]; diff --git a/src/cmd/ksh93/tests/basic.sh b/src/cmd/ksh93/tests/basic.sh index b73ec9474..d4a0a7bd6 100755 --- a/src/cmd/ksh93/tests/basic.sh +++ b/src/cmd/ksh93/tests/basic.sh @@ -446,7 +446,7 @@ done | while read sec; do ( "$binsleep" "$sec"; "$binsleep" "$sec") done s=SECONDS set -o pipefail for ((i=0; i < 30; i++)) -do print hello 2>/dev/null +do print hello sleep .01 done | "$binsleep" .1 (( (SECONDS-s) < .2 )) || err_exit 'early termination not causing broken pipe' @@ -486,7 +486,7 @@ bintrue=$(whence -p true) set -o pipefail float start=$SECONDS end for ((i=0; i < 2; i++)) -do print foo 2>/dev/null +do print foo sleep .15 done | { read; $bintrue; end=$SECONDS ;} (( (SECONDS-start) < .1 )) && err_exit "pipefail not waiting for pipe to finish" diff --git a/src/cmd/ksh93/tests/builtins.sh b/src/cmd/ksh93/tests/builtins.sh index 58ed91cff..0fd7a2810 100755 --- a/src/cmd/ksh93/tests/builtins.sh +++ b/src/cmd/ksh93/tests/builtins.sh @@ -662,8 +662,8 @@ actual=$( ) | true } 2>&1 ) -expect=$': print: I/O error\n1' -if [[ $actual != *"$expect" ]] +expect='1' +if [[ $actual != "$expect" ]] then err_exit "I/O error not detected: expected $(printf %q "$expect"), got $(printf %q "$actual"))" fi diff --git a/src/cmd/ksh93/tests/coprocess.sh b/src/cmd/ksh93/tests/coprocess.sh index 1cc473334..fc0dfd7b0 100755 --- a/src/cmd/ksh93/tests/coprocess.sh +++ b/src/cmd/ksh93/tests/coprocess.sh @@ -272,7 +272,7 @@ _COSHELL_msgfd=5 function cop { read - print ok 2>/dev/null + print ok } exp=ok @@ -348,7 +348,7 @@ wait function cop { read - print ok 2>/dev/null + print ok } exp=ok cop |&