diff --git a/NEWS b/NEWS index 51dd10539..8ab758819 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,18 @@ For full details, see the git log at: https://github.com/ksh93/ksh Any uppercase BUG_* names are modernish shell bug IDs. +2020-09-09: + +- Fixed BUG_LOOPRET2 and related bugs. The 'exit' and 'return' commands without + an argument now correctly default to passing down the exit status of the + last-run command. Tests like the following, in which the last-run command is + 'false', now correctly output 1 instead of 0: + fn() { return || true; }; false; fn; echo "$?" + fn() { while return; do true; done; }; false; fn; echo "$?" + fn() { for i in 1; do return; done; }; false; fn; echo "$?" + fn() { case 1 in 1) return ;; esac; }; false; fn; echo "$?" + fn() { { return; } 2>&1; }; false; fn; echo "$?" + 2020-09-05: - Fixed erroneous syntax errors in parameter expansions such as ${var:-wor)d} diff --git a/TODO b/TODO index 8ffc2c499..df46db562 100644 --- a/TODO +++ b/TODO @@ -49,9 +49,3 @@ https://github.com/modernish/modernish/tree/0.16/lib/modernish/cap/ parameter expansion inserts any IFS separator characters, those characters are erroneously interpreted as wildcards when quoted "$*" is used as the glob pattern. - -- BUG_LOOPRET2: If a 'return' command is given without a status argument - within the set of conditional commands in a 'while' or 'until' loop (i.e., - between 'while'/'until' and 'do'), the exit status passed down from the - previous command is ignored and the function returns with status 0 - instead. diff --git a/src/cmd/ksh93/bltins/cflow.c b/src/cmd/ksh93/bltins/cflow.c index 9396de789..34a3e0f6e 100644 --- a/src/cmd/ksh93/bltins/cflow.c +++ b/src/cmd/ksh93/bltins/cflow.c @@ -64,7 +64,7 @@ done: errormsg(SH_DICT,ERROR_usage(2),"%s",optusage((char*)0)); pp->mode = (**argv=='e'?SH_JMPEXIT:SH_JMPFUN); argv += opt_info.index; - n = (((arg= *argv)?(int)strtol(arg, (char**)0, 10):shp->oldexit)); + n = (arg = *argv) ? (int)strtol(arg, (char**)0, 10) : shp->savexit; if(n<0 || n==256 || n > SH_EXITMASK+shp->gd->sigmax+1) n &= ((unsigned int)n)&SH_EXITMASK; /* return outside of function, dotscript and profile is exit */ diff --git a/src/cmd/ksh93/include/defs.h b/src/cmd/ksh93/include/defs.h index df5cdfbf6..59a4b28ca 100644 --- a/src/cmd/ksh93/include/defs.h +++ b/src/cmd/ksh93/include/defs.h @@ -155,7 +155,6 @@ struct shared Sfio_t *heredocs; /* current here-doc temp file */ \ Sfio_t *funlog; /* for logging function definitions */ \ int **fdptrs; /* pointer to file numbers */ \ - int savexit; \ char *lastarg; \ char *lastpath; /* last alsolute path found */ \ int path_err; /* last error on path search */ \ @@ -181,7 +180,6 @@ struct shared char *prefix; /* prefix for compound assignment */ \ sigjmp_buf *jmplist; /* longjmp return stack */ \ char *fifo; /* fifo name for process sub */ \ - int oldexit; \ pid_t bckpid; /* background process id */ \ pid_t cpid; \ pid_t spid; /* subshell process id */ \ diff --git a/src/cmd/ksh93/include/shell.h b/src/cmd/ksh93/include/shell.h index 2a3c5acdc..77d908dd0 100644 --- a/src/cmd/ksh93/include/shell.h +++ b/src/cmd/ksh93/include/shell.h @@ -140,7 +140,8 @@ struct Shell_s Dt_t *bltin_tree; /* for builtin commands */ Shscope_t *topscope; /* pointer to top-level scope */ int inlineno; /* line number of current input file */ - int exitval; /* most recent exit value */ + int exitval; /* exit status of the command currently being run */ + int savexit; /* $? == exit status of the last command executed */ unsigned char trapnote; /* set when trap/signal is pending */ char shcomp; /* set when running shcomp */ unsigned int subshell; /* set for virtual subshell */ diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index e06fe1525..f24a1351f 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -17,4 +17,4 @@ * David Korn * * * ***********************************************************************/ -#define SH_RELEASE "93u+m 2020-09-05" +#define SH_RELEASE "93u+m 2020-09-09" diff --git a/src/cmd/ksh93/sh/fault.c b/src/cmd/ksh93/sh/fault.c index 5d138a23b..6af0b003a 100644 --- a/src/cmd/ksh93/sh/fault.c +++ b/src/cmd/ksh93/sh/fault.c @@ -614,7 +614,6 @@ void sh_done(void *ptr, register int sig) if(t=shp->st.trapcom[0]) { shp->st.trapcom[0]=0; /*should free but not long */ - shp->oldexit = savxit; sh_trap(t,0); savxit = shp->exitval; } diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c index 504afb5ca..b74b00430 100644 --- a/src/cmd/ksh93/sh/subshell.c +++ b/src/cmd/ksh93/sh/subshell.c @@ -610,7 +610,6 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub) /* trap on EXIT not handled by child */ char *trap=shp->st.trapcom[0]; shp->st.trapcom[0] = 0; /* prevent recursion */ - shp->oldexit = shp->exitval; sh_trap(trap,0); free(trap); } diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c index 6c620badd..122a1db8e 100644 --- a/src/cmd/ksh93/sh/xec.c +++ b/src/cmd/ksh93/sh/xec.c @@ -972,8 +972,6 @@ int sh_exec(register const Shnode_t *t, int flags) if(was_monitor&flags) sh_onstate(SH_MONITOR); type = t->tre.tretyp; - if(!shp->intrap) - shp->oldexit=shp->exitval; shp->exitval=0; shp->lastsig = 0; shp->lastpath = 0; diff --git a/src/cmd/ksh93/tests/basic.sh b/src/cmd/ksh93/tests/basic.sh index 455efcfe2..71693b0c1 100755 --- a/src/cmd/ksh93/tests/basic.sh +++ b/src/cmd/ksh93/tests/basic.sh @@ -428,6 +428,88 @@ if env x-a=y >/dev/null 2>&1 then [[ $(env 'x-a=y' $SHELL -c 'env | grep x-a') == *x-a=y* ]] || err_exit 'invalid environment variables not preserved' fi +foo() { return; } +false +foo && err_exit "'return' within function does not preserve exit status" +false +foo & wait "$!" && err_exit "'return' within function does not preserve exit status (bg job)" + +foo() { false; return || :; } +foo && err_exit "'return ||' does not preserve exit status" + +foo() { false; return && :; } +foo && err_exit "'return &&' does not preserve exit status" + +foo() { false; while return; do true; done; } +foo && err_exit "'while return' does not preserve exit status" + +foo() { false; while return; do true; done 2>&1; } +foo && err_exit "'while return' with redirection does not preserve exit status" + +foo() { false; until return; do true; done; } +foo && err_exit "'until return' does not preserve exit status" + +foo() { false; until return; do true; done 2>&1; } +foo && err_exit "'until return' with redirection does not preserve exit status" + +foo() { false; for i in 1; do return; done; } +foo && err_exit "'return' within 'for' does not preserve exit status" + +foo() { false; for i in 1; do return; done 2>&1; } +foo && err_exit "'return' within 'for' with redirection does not preserve exit status" + +foo() { false; { return; } 2>&1; } +foo && err_exit "'return' within { block; } with redirection does not preserve exit status" + +foo() ( exit ) +false +foo && err_exit "'exit' within function does not preserve exit status" +false +foo & wait "$!" && err_exit "'exit' within function does not preserve exit status (bg job)" + +foo() ( false; exit || : ) +foo && err_exit "'exit ||' does not preserve exit status" + +foo() ( false; exit && : ) +foo && err_exit "'exit &&' does not preserve exit status" + +foo() ( false; while exit; do true; done ) +foo && err_exit "'while exit' does not preserve exit status" + +foo() ( false; while exit; do true; done 2>&1 ) +foo && err_exit "'while exit' with redirection does not preserve exit status" + +foo() ( false; until exit; do true; done ) +foo && err_exit "'until exit' does not preserve exit status" + +foo() ( false; until exit; do true; done 2>&1 ) +foo && err_exit "'until exit' with redirection does not preserve exit status" + +foo() ( false; for i in 1; do exit; done ) +foo && err_exit "'exit' within 'for' does not preserve exit status" + +foo() ( false; for i in 1; do exit; done 2>&1 ) +foo && err_exit "'exit' within 'for' with redirection does not preserve exit status" + +foo() ( false; { exit; } 2>&1 ) +foo && err_exit "'exit' within { block; } with redirection does not preserve exit status" + +set -- +false +for i in "$@"; do :; done || err_exit 'empty loop does not reset exit status ("$@")' +false +for i do :; done || err_exit 'empty loop does not reset exit status ("$@" shorthand)' +false +for i in; do :; done || err_exit "empty loop does not reset exit status (empty 'in' list)" + +false +case 1 in 1) ;; esac || err_exit "empty case does not reset exit status" +false +case 1 in 2) echo whoa ;; esac || err_exit "non-matching case does not reset exit status" + +false +2>&1 || err_exit "lone redirection does not reset exit status" + float s=SECONDS for i in .1 .2 do print $i