diff --git a/NEWS b/NEWS index a907545f3..59d1aea9a 100644 --- a/NEWS +++ b/NEWS @@ -12,6 +12,11 @@ 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 + 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. + 2020-05-13: - Fix BUG_CASELIT: an undocumented 'case' pattern matching misbehaviour that diff --git a/TODO b/TODO index ae9327cb3..c92aed97b 100644 --- a/TODO +++ b/TODO @@ -92,10 +92,5 @@ https://github.com/modernish/modernish/tree/0.16/lib/modernish/cap/ example, "$*" joins positional parameters on the first byte of IFS instead of the first character. -- BUG_PUTIOERR: Shell builtins that output strings (echo, printf, ksh/zsh - print), and thus also modernish put and putln, do not check for I/O errors - on output. This means a script cannot check for them, and a script process - in a pipe can get stuck in an infinite loop if SIGPIPE is ignored. - - BUG_TESTERR1A: test/[ exits with a non-error false status (1) if an invalid argument is given to an operator. diff --git a/src/cmd/ksh93/bltins/print.c b/src/cmd/ksh93/bltins/print.c index ee6f2fdf0..6d2f30203 100644 --- a/src/cmd/ksh93/bltins/print.c +++ b/src/cmd/ksh93/bltins/print.c @@ -336,9 +336,11 @@ skip2: * https://github.com/att/ast/issues/425 */ if(!sflag && sffileno(outfile)!=sffileno(sfstderr)) - sfsync(outfile); + if (sfsync(outfile) < 0) + exitval = 1; sfpool(sfstderr,pool,SF_WRITE); - exitval = pdata.err; + if (pdata.err) + exitval = 1; } else if(vflag) { @@ -353,7 +355,10 @@ skip2: { /* echo style print */ if(nflag && !argv[0]) - sfsync((Sfio_t*)0); + { + if (sfsync((Sfio_t*)0) < 0) + exitval = 1; + } else if(sh_echolist(shp,outfile,rflag,argv) && !nflag) sfputc(outfile,'\n'); } @@ -365,8 +370,11 @@ skip2: else if(n&SF_SHARE) { sfset(outfile,SF_SHARE|SF_PUBLIC,1); - sfsync(outfile); + 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 a4eead98f..d56e2d32a 100644 --- a/src/cmd/ksh93/data/msg.c +++ b/src/cmd/ksh93/data/msg.c @@ -89,6 +89,7 @@ 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 f82c5ca10..5836d0dc0 100644 --- a/src/cmd/ksh93/include/io.h +++ b/src/cmd/ksh93/include/io.h @@ -97,6 +97,7 @@ 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 4613baee2..f8b4794e7 100755 --- a/src/cmd/ksh93/tests/basic.sh +++ b/src/cmd/ksh93/tests/basic.sh @@ -431,7 +431,7 @@ done | while read sec; do ( $sleep $sec; $sleep $sec) done s=SECONDS set -o pipefail for ((i=0; i < 30; i++)) -do print hello +do print hello 2>/dev/null sleep .1 done | $sleep 1 (( (SECONDS-s) < 2 )) || err_exit 'early termination not causing broken pipe' @@ -470,7 +470,7 @@ bintrue=$(whence -p true) set -o pipefail float start=$SECONDS end for ((i=0; i < 2; i++)) -do print foo +do print foo 2>/dev/null sleep 1.5 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 3b191546c..4ad461f4a 100755 --- a/src/cmd/ksh93/tests/builtins.sh +++ b/src/cmd/ksh93/tests/builtins.sh @@ -639,4 +639,26 @@ read baz <<< 'foo\\\\bar' : ~root [[ $(builtin) == *.sh.tilde* ]] && err_exit 'builtin contains .sh.tilde' +# ====== +# Check that I/O errors are detected +actual=$( + { + ( + trap "" PIPE + for ((i = SECONDS + 1; SECONDS < i; )); do + print hi || { + print $? >&2 + exit + } + done + ) | true + } 2>&1 +) +expect=$': print: I/O error\n1' +if [[ $actual != *"$expect" ]] +then + err_exit "I/O error not detected (expected '$expect', got '$actual')" +fi + +# ====== exit $((Errors<125?Errors:125)) diff --git a/src/cmd/ksh93/tests/coprocess.sh b/src/cmd/ksh93/tests/coprocess.sh index 2b0b50d0e..93a47e8dd 100755 --- a/src/cmd/ksh93/tests/coprocess.sh +++ b/src/cmd/ksh93/tests/coprocess.sh @@ -266,7 +266,7 @@ _COSHELL_msgfd=5 function cop { read - print ok + print ok 2>/dev/null } exp=ok @@ -342,7 +342,7 @@ wait function cop { read - print ok + print ok 2>/dev/null } exp=ok cop |&