Re: bug: $? after empty command

[Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>]

On Fri, 10 Jul 2009 10:02:28 +0100
Peter Stephenson <pws@xxxxxxx> wrote:
> On Thu, 9 Jul 2009 21:41:51 +0000 (UTC)
> Eric Blake <ebb9@xxxxxxx> 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@xxxxxxxxxxxx>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/