* bug: $? after empty command @ 2009-07-09 13:12 Eric Blake 2009-07-09 14:41 ` Peter Stephenson 0 siblings, 1 reply; 8+ messages in thread From: Eric Blake @ 2009-07-09 13:12 UTC (permalink / raw) To: zsh-workers POSIX requires that the execution of an empty command change $? to 0, as shown here with bash: $ zsh -c 'foo=; false; $foo; echo $?' 1 $ bash -c 'foo=; false; $foo; echo $?' 0 -- Eric Blake ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: bug: $? after empty command 2009-07-09 13:12 bug: $? after empty command Eric Blake @ 2009-07-09 14:41 ` Peter Stephenson 2009-07-09 21:17 ` Eric Blake 0 siblings, 1 reply; 8+ messages in thread From: Peter Stephenson @ 2009-07-09 14:41 UTC (permalink / raw) To: zsh-workers On Thu, 9 Jul 2009 13:12:20 +0000 (UTC) Eric Blake <ebb9@byu.net> wrote: > POSIX requires that the execution of an empty command change $? to 0, as shown > here with bash: > > $ zsh -c 'foo=; false; $foo; echo $?' > 1 I'm not aware that that's deliberate shell behaviour, and indeed I wouldn't have expected it. It *is* deliberate behaviour that the status is not reset simply by hitting return at the interactive prompt, but that's actually a different issue, not affected by the following. The fix for this is quite subtle because of the case zsh -c 'false; $(exit 3); echo $?' which should print 3, but I think I've finally found a way that handles both. Index: Src/exec.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/exec.c,v retrieving revision 1.165 diff -u -r1.165 exec.c --- Src/exec.c 15 Mar 2009 01:04:50 -0000 1.165 +++ Src/exec.c 9 Jul 2009 14:38:41 -0000 @@ -151,6 +151,15 @@ /**/ int cmdoutval; +/* + * This is set by an exiting $(...) substitution to indicate we need + * to retain the status. We initialize it to zero if we think we need + * to reset the status for a command. + */ + +/**/ +int use_cmdoutval; + /* The context in which a shell function is called, see SFC_* in zsh.h. */ /**/ @@ -2262,6 +2271,14 @@ */ if (!args && varspc) lastval = errflag ? errflag : cmdoutval; + /* + * If there are arguments, we should reset the status for the + * command before execution---unless we are using the result of a + * command substitution, which will be indicated by setting + * use_cmdoutval to 1. We haven't kicked those off yet, so + * there's no race. + */ + use_cmdoutval = !args; for (i = 0; i < 10; i++) { save[i] = -2; @@ -2478,7 +2495,12 @@ lastval = 0; return; } else { - cmdoutval = lastval; + /* + * No arguments. Reset the status if there were + * arguments before and no command substitution + * has provided a status. + */ + cmdoutval = use_cmdoutval ? lastval : 0; if (varspc) addvars(state, varspc, 0); if (errflag) @@ -4674,6 +4696,7 @@ es->badcshglob = badcshglob; es->cmdoutpid = cmdoutpid; es->cmdoutval = cmdoutval; + es->use_cmdoutval = use_cmdoutval; es->trap_return = trap_return; es->trap_state = trap_state; es->trapisfunc = trapisfunc; @@ -4704,6 +4727,7 @@ badcshglob = exstack->badcshglob; cmdoutpid = exstack->cmdoutpid; cmdoutval = exstack->cmdoutval; + use_cmdoutval = exstack->use_cmdoutval; trap_return = exstack->trap_return; trap_state = exstack->trap_state; trapisfunc = exstack->trapisfunc; Index: Src/signals.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/signals.c,v retrieving revision 1.54 diff -u -r1.54 signals.c --- Src/signals.c 11 Feb 2009 20:42:16 -0000 1.54 +++ Src/signals.c 9 Jul 2009 14:38:41 -0000 @@ -494,6 +494,7 @@ *procsubval = (0200 | WTERMSIG(status)); else *procsubval = WEXITSTATUS(status); + use_cmdoutval = 1; get_usage(); cont = 1; break; Index: Src/zsh.h =================================================================== RCS file: /cvsroot/zsh/zsh/Src/zsh.h,v retrieving revision 1.158 diff -u -r1.158 zsh.h --- Src/zsh.h 2 Jul 2009 13:48:36 -0000 1.158 +++ Src/zsh.h 9 Jul 2009 14:38:41 -0000 @@ -930,6 +930,7 @@ int badcshglob; pid_t cmdoutpid; int cmdoutval; + int use_cmdoutval; int trap_return; int trap_state; int trapisfunc; Index: Test/A01grammar.ztst =================================================================== RCS file: /cvsroot/zsh/zsh/Test/A01grammar.ztst,v retrieving revision 1.22 diff -u -r1.22 A01grammar.ztst --- Test/A01grammar.ztst 6 Jul 2009 20:44:29 -0000 1.22 +++ Test/A01grammar.ztst 9 Jul 2009 14:38:41 -0000 @@ -23,6 +23,10 @@ true | false 1:Exit status of pipeline with builtins (false) + false + $nonexistent_variable +0:Executing command that evaluates to empty resets status + fn() { local foo; read foo; print $foo; } coproc fn print -p coproc test output -- Peter Stephenson <pws@csr.com> Software Engineer CSR PLC, Churchill House, Cambridge Business Park, Cowley Road Cambridge, CB4 0WZ, UK Tel: +44 (0)1223 692070 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: bug: $? after empty command 2009-07-09 14:41 ` Peter Stephenson @ 2009-07-09 21:17 ` Eric Blake 2009-07-09 21:41 ` Eric Blake 0 siblings, 1 reply; 8+ messages in thread From: Eric Blake @ 2009-07-09 21:17 UTC (permalink / raw) To: zsh-workers Peter Stephenson <pws <at> csr.com> writes: > > POSIX requires that the execution of an empty command change $? to 0, as > > shown here with bash: > > > > $ zsh -c 'foo=; false; $foo; echo $?' > > 1 > > I'm not aware that that's deliberate shell behaviour, and indeed I > wouldn't have expected it. It *is* deliberate behaviour that the status is > not reset simply by hitting return at the interactive prompt, but that's > actually a different issue, not affected by the following. Good point about making the behavior dependent on a non-empty command line, and that the exit status of command substitution is still important. I don't see your patch in CVS yet, so I haven't played with it. But just from inspection, it looks like it does not cover these related issues, which are both required by POSIX to output 0: $ zsh -c 'false; . /dev/null; echo $?' 1 $ zsh -c 'false; ``; echo $?' 1 -- Eric Blake ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: bug: $? after empty command 2009-07-09 21:17 ` Eric Blake @ 2009-07-09 21:41 ` Eric Blake 2009-07-10 9:02 ` Peter Stephenson 0 siblings, 1 reply; 8+ messages in thread From: Eric Blake @ 2009-07-09 21:41 UTC (permalink / raw) To: zsh-workers Eric Blake <ebb9 <at> byu.net> writes: > But just from inspection, > it looks like it does not cover these related issues, which are both required > by POSIX to output 0: > > $ zsh -c 'false; . /dev/null; echo $?' > 1 > $ zsh -c 'false; ``; echo $?' > 1 And another one that POSIX requires to output 0: $ zsh -c 'false; sleep& echo $?' 1 -- Eric Blake ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: bug: $? after empty command 2009-07-09 21:41 ` Eric Blake @ 2009-07-10 9:02 ` Peter Stephenson 2009-07-10 22:05 ` Peter Stephenson 0 siblings, 1 reply; 8+ messages in thread From: Peter Stephenson @ 2009-07-10 9:02 UTC (permalink / raw) To: zsh-workers On Thu, 9 Jul 2009 21:41:51 +0000 (UTC) Eric Blake <ebb9@byu.net> wrote: > Eric Blake <ebb9 <at> byu.net> writes: > > > But just from inspection, > > it looks like it does not cover these related issues, which are both required > > by POSIX to output 0: > > > > $ zsh -c 'false; . /dev/null; echo $?' > > 1 > > $ zsh -c 'false; ``; echo $?' > > 1 > > And another one that POSIX requires to output 0: > > $ zsh -c 'false; sleep& echo $?' > 1 These still need fixing. Also, I don't think it affects POSIX, because empty functions aren't required to work, but in zsh, "fn() { }" is valid, and I suspect "fn" logically ought to reset the status, too---basically anywhere where you've run a complex command with an empty list of commands in it. -- Peter Stephenson <pws@csr.com> Software Engineer CSR PLC, Churchill House, Cambridge Business Park, Cowley Road Cambridge, CB4 0WZ, UK Tel: +44 (0)1223 692070 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: bug: $? after empty command 2009-07-10 9:02 ` Peter Stephenson @ 2009-07-10 22:05 ` Peter Stephenson 2009-07-11 16:39 ` Peter Stephenson 0 siblings, 1 reply; 8+ messages in thread From: Peter Stephenson @ 2009-07-10 22:05 UTC (permalink / raw) To: zsh-workers On Fri, 10 Jul 2009 10:02:28 +0100 Peter Stephenson <pws@csr.com> wrote: > On Thu, 9 Jul 2009 21:41:51 +0000 (UTC) > Eric Blake <ebb9@byu.net> wrote: >> Eric Blake <ebb9 <at> byu.net> writes: >> >>> But just from inspection, >>> it looks like it does not cover these related issues, which are >>> both required by POSIX to output 0: >>> >>> $ zsh -c 'false; . /dev/null; echo $?' >>> 1 >>> $ zsh -c 'false; ``; echo $?' >>> 1 >> >> And another one that POSIX requires to output 0: >> >> $ zsh -c 'false; sleep& echo $?' >> 1 > > Also, I don't think it affects POSIX, because empty functions aren't > required to work, but in zsh, "fn() { }" is valid, and I suspect "fn" > logically ought to reset the status, too---basically anywhere where you've > run a complex command with an empty list of commands in it. These four were all fairly easy; in each case there is a local context where the command in question is being executed in which the status can be reset. There may be a a more generic place, such as execlist(), for these to go, but I like the transparency of resetting them locally; execlist()/execode() are used in just too many places. However, that's a bit craven. Index: Src/exec.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/exec.c,v retrieving revision 1.166 diff -u -r1.166 exec.c --- Src/exec.c 10 Jul 2009 11:08:48 -0000 1.166 +++ Src/exec.c 10 Jul 2009 21:59:52 -0000 @@ -1372,7 +1372,8 @@ else spawnjob(); child_unblock(); - return 0; + /* Executing background code resets shell status */ + return lastval = 0; } else { if (newjob != lastwj) { Job jn = jobtab + newjob; @@ -3512,6 +3513,7 @@ return retval; } /* pid == 0 */ + lastval = 0; /* status of empty list is zero */ child_unblock(); zclose(pipes[0]); redup(pipes[1], 1); @@ -4259,6 +4261,7 @@ if (trap_state == TRAP_STATE_PRIMED) trap_return--; oldlastval = lastval; + lastval = 0; /* status of empty function is zero */ oldnumpipestats = numpipestats; if (noreturnval) { /* Index: Src/init.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/init.c,v retrieving revision 1.103 diff -u -r1.103 init.c --- Src/init.c 9 Jul 2009 19:42:20 -0000 1.103 +++ Src/init.c 10 Jul 2009 21:59:52 -0000 @@ -1131,6 +1131,7 @@ fstack.tp = FS_SOURCE; funcstack = &fstack; + lastval = 0; /* status of empty file is zero */ if (prog) { pushheap(); errflag = 0; Index: Test/A01grammar.ztst =================================================================== RCS file: /cvsroot/zsh/zsh/Test/A01grammar.ztst,v retrieving revision 1.23 diff -u -r1.23 A01grammar.ztst --- Test/A01grammar.ztst 10 Jul 2009 11:08:48 -0000 1.23 +++ Test/A01grammar.ztst 10 Jul 2009 21:59:52 -0000 @@ -27,6 +27,18 @@ $nonexistent_variable 0:Executing command that evaluates to empty resets status + false + sleep 1 & + print $? + # a tidy test is a happy test + wait $! +0:Starting background command resets status +>0 + + false + . /dev/null +0:Sourcing empty file resets status + fn() { local foo; read foo; print $foo; } coproc fn print -p coproc test output @@ -531,3 +543,13 @@ . ./bad_syntax 126: Attempt to "." file with bad syntax. ?./bad_syntax:2: parse error near `\n' + + echo 'false' >dot_false + . ./dot_false + print $? + echo 'true' >dot_true + . ./dot_true + print $? +0:Last status of successfully executed "." file is retained +>1 +>0 Index: Test/C04funcdef.ztst =================================================================== RCS file: /cvsroot/zsh/zsh/Test/C04funcdef.ztst,v retrieving revision 1.5 diff -u -r1.5 C04funcdef.ztst --- Test/C04funcdef.ztst 29 Dec 2008 04:24:36 -0000 1.5 +++ Test/C04funcdef.ztst 10 Jul 2009 21:59:52 -0000 @@ -1,5 +1,20 @@ %test + fn1() { return 1; } + fn2() { + fn1 + print $? + return 2 + } + fn2 +2:Basic status returns from functions +>1 + + fnz() { } + false + fnz +0:Empty function body resets status + function f$$ () { print regress expansion of function names } Index: Test/D08cmdsubst.ztst =================================================================== RCS file: /cvsroot/zsh/zsh/Test/D08cmdsubst.ztst,v retrieving revision 1.2 diff -u -r1.2 D08cmdsubst.ztst --- Test/D08cmdsubst.ztst 9 Dec 2007 23:53:33 -0000 1.2 +++ Test/D08cmdsubst.ztst 10 Jul 2009 21:59:52 -0000 @@ -89,3 +89,7 @@ X=$(exit 2) $(exit 0) || print $? 0:variable assignments processed after other substitutions >2 + + false + `` +0:Empty command substitution resets status -- Peter Stephenson <p.w.stephenson@ntlworld.com> Web page now at http://homepage.ntlworld.com/p.w.stephenson/ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: bug: $? after empty command 2009-07-10 22:05 ` Peter Stephenson @ 2009-07-11 16:39 ` Peter Stephenson 2009-07-11 18:43 ` Bart Schaefer 0 siblings, 1 reply; 8+ messages in thread From: Peter Stephenson @ 2009-07-11 16:39 UTC (permalink / raw) To: zsh-workers On Fri, 10 Jul 2009 23:05:54 +0100 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote: > On Fri, 10 Jul 2009 10:02:28 +0100 > Peter Stephenson <pws@csr.com> wrote: > > On Thu, 9 Jul 2009 21:41:51 +0000 (UTC) > > Eric Blake <ebb9@byu.net> wrote: > >> Eric Blake <ebb9 <at> byu.net> writes: > >>> $ zsh -c 'false; . /dev/null; echo $?' > >>> 1 > >>> $ zsh -c 'false; ``; echo $?' > >>> 1 > >> $ zsh -c 'false; sleep& echo $?' > >> 1 > > > > "fn() { }" is valid, and I suspect "fn" > > logically ought to reset the status, too > > These four were all fairly easy; in each case there is a local context > where the command in question is being executed in which the status can > be reset. There may be a a more generic place, such as execlist(), for > these to go, but I like the transparency of resetting them locally; > execlist()/execode() are used in just too many places. Change of plan: I woke up in the night and realised that doing it this way breaks the case fn() { print $?; } false fn which is a bit strange but perfectly valid and should print "1". > However, that's a bit craven. This takes the brave option, but actually it's easy except for the case of "." which needs a bit more information from down below. For both executing a list and sourcing a file line by line we only reset the status if there is no code to execute. The case of starting a background function isn't affected, in that case the foreground code is always empty. Index: Src/exec.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/exec.c,v retrieving revision 1.167 diff -u -r1.167 exec.c --- Src/exec.c 10 Jul 2009 22:10:26 -0000 1.167 +++ Src/exec.c 11 Jul 2009 16:32:42 -0000 @@ -1056,6 +1056,10 @@ /* Loop over all sets of comands separated by newline, * * semi-colon or ampersand (`sublists'). */ code = *state->pc++; + if (wc_code(code) != WC_LIST) { + /* Empty list; this returns status zero. */ + lastval = 0; + } while (wc_code(code) == WC_LIST && !breaks && !retflag && !errflag) { int donedebug; @@ -3513,7 +3517,6 @@ return retval; } /* pid == 0 */ - lastval = 0; /* status of empty list is zero */ child_unblock(); zclose(pipes[0]); redup(pipes[1], 1); @@ -4261,7 +4264,6 @@ if (trap_state == TRAP_STATE_PRIMED) trap_return--; oldlastval = lastval; - lastval = 0; /* status of empty function is zero */ oldnumpipestats = numpipestats; if (noreturnval) { /* Index: Src/init.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/init.c,v retrieving revision 1.104 diff -u -r1.104 init.c --- Src/init.c 10 Jul 2009 22:10:26 -0000 1.104 +++ Src/init.c 11 Jul 2009 16:32:42 -0000 @@ -99,11 +99,11 @@ /* keep executing lists until EOF found */ /**/ -int +enum loop_return loop(int toplevel, int justonce) { Eprog prog; - int err; + int err, non_empty = 0; pushheap(); if (!toplevel) @@ -151,6 +151,7 @@ if (hend(prog)) { int toksav = tok; + non_empty = 1; if (toplevel && (getshfunc("preexec") || paramtab->getnode(paramtab, "preexec" HOOK_SUFFIX))) { @@ -207,7 +208,11 @@ lexrestore(); popheap(); - return err; + if (err) + return LOOP_ERROR; + if (!non_empty) + return LOOP_EMPTY; + return LOOP_OK; } static char *cmd; @@ -1131,7 +1136,6 @@ fstack.tp = FS_SOURCE; funcstack = &fstack; - lastval = 0; /* status of empty file is zero */ if (prog) { pushheap(); errflag = 0; @@ -1141,8 +1145,21 @@ ret = SOURCE_ERROR; } else { /* loop through the file to be sourced */ - if (loop(0, 0)) + switch (loop(0, 0)) + { + case LOOP_OK: + /* nothing to do but compilers like a complete enum */ + break; + + case LOOP_EMPTY: + /* Empty code resets status */ + lastval = 0; + break; + + case LOOP_ERROR: ret = SOURCE_ERROR; + break; + } } funcstack = funcstack->prev; sourcelevel--; Index: Src/zsh.h =================================================================== RCS file: /cvsroot/zsh/zsh/Src/zsh.h,v retrieving revision 1.159 diff -u -r1.159 zsh.h --- Src/zsh.h 10 Jul 2009 11:08:48 -0000 1.159 +++ Src/zsh.h 11 Jul 2009 16:32:43 -0000 @@ -1726,6 +1726,17 @@ #define PRINT_WHENCE_FUNCDEF (1<<9) #define PRINT_WHENCE_WORD (1<<10) +/* Return values from loop() */ + +enum loop_return { + /* Loop executed OK */ + LOOP_OK, + /* Loop executed no code */ + LOOP_EMPTY, + /* Loop encountered an error */ + LOOP_ERROR +}; + /* Return values from source() */ enum source_return { Index: Test/A01grammar.ztst =================================================================== RCS file: /cvsroot/zsh/zsh/Test/A01grammar.ztst,v retrieving revision 1.24 diff -u -r1.24 A01grammar.ztst --- Test/A01grammar.ztst 10 Jul 2009 22:10:27 -0000 1.24 +++ Test/A01grammar.ztst 11 Jul 2009 16:32:43 -0000 @@ -553,3 +553,9 @@ 0:Last status of successfully executed "." file is retained >1 >0 + + echo 'echo $?' >dot_status + false + . ./dot_status +0:"." file sees status from previous command +>1 Index: Test/C04funcdef.ztst =================================================================== RCS file: /cvsroot/zsh/zsh/Test/C04funcdef.ztst,v retrieving revision 1.6 diff -u -r1.6 C04funcdef.ztst --- Test/C04funcdef.ztst 10 Jul 2009 22:10:27 -0000 1.6 +++ Test/C04funcdef.ztst 11 Jul 2009 16:32:43 -0000 @@ -15,6 +15,13 @@ fnz 0:Empty function body resets status + fn3() { return 3; } + fnstat() { print $?; } + fn3 + fnstat +0:Status is not reset on non-empty function body +>3 + function f$$ () { print regress expansion of function names } Index: Test/D08cmdsubst.ztst =================================================================== RCS file: /cvsroot/zsh/zsh/Test/D08cmdsubst.ztst,v retrieving revision 1.3 diff -u -r1.3 D08cmdsubst.ztst --- Test/D08cmdsubst.ztst 10 Jul 2009 22:10:27 -0000 1.3 +++ Test/D08cmdsubst.ztst 11 Jul 2009 16:32:43 -0000 @@ -93,3 +93,8 @@ false `` 0:Empty command substitution resets status + + false + echo `echo $?` +0:Non-empty command substitution inherits status +>1 -- Peter Stephenson <p.w.stephenson@ntlworld.com> Web page now at http://homepage.ntlworld.com/p.w.stephenson/ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: bug: $? after empty command 2009-07-11 16:39 ` Peter Stephenson @ 2009-07-11 18:43 ` Bart Schaefer 0 siblings, 0 replies; 8+ messages in thread From: Bart Schaefer @ 2009-07-11 18:43 UTC (permalink / raw) To: zsh-workers On Jul 11, 5:39pm, Peter Stephenson wrote: } } Change of plan: I woke up in the night and realised that doing it this } way breaks [something] I do that, too. Results in some strange sleep hours, sometimes. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-07-11 18:44 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-07-09 13:12 bug: $? after empty command Eric Blake 2009-07-09 14:41 ` Peter Stephenson 2009-07-09 21:17 ` Eric Blake 2009-07-09 21:41 ` Eric Blake 2009-07-10 9:02 ` Peter Stephenson 2009-07-10 22:05 ` Peter Stephenson 2009-07-11 16:39 ` Peter Stephenson 2009-07-11 18:43 ` Bart Schaefer
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).