Re: PATCH: exit status

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

On Fri, 10 Nov 2006 21:28:44 -0800
Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> Aside:  Here's an actual bug:
> 
>    zsh -fc 'trap "echo \$?" EXIT; echo ${example?oops}'
> 
> The EXIT trap is not called when the shell exits on the error caused
> by the ${var?message} construct.

There are two bugs: we go straight to exit() instead of zexit(), and the
EXIT trap doesn't get run if there has just been an error.  Presumably
it needs to be run unconditionally.  (In fact, now I think about it, I'm
a little bit suspicious that testing for "errflag" before running a
trap, which is right down in the trap handling code, is ever the right
thing to do.)

There's another bug lurking:  the exit might be within a forked
subshell, so I've made it use _exit() in that case.

> (Bash prints "127" in this case;
> also, the exit status of zsh on that construct is 1 rather than 127,
> but I don't know why bash chose 127.)

The only mention I can see in the standard is for a command not found,
which doesn't apply, but I presume status 127 is being used generally to
signal an error in shell processing rather than a command return status.

> On Nov 10,  4:09pm, Peter Stephenson wrote:
> }
> } I can see arguments for both sides; it turns whether you consider the
> } "exit" command itself to have been executed before the exit traps run,
> } which is a bit Zen-like.
> 
> Think of it as a C program.  If
> 
>     int question_mark = 0;
>     atexit() { printf("%d\n", question_mark); }
>     main() { question_mark = exit(3); }
> 
> is a valid program and, when run, prints "3", then I'm on your side.
> Otherwise I'm with Cragun.

You certainly wouldn't expect exit() to return to main() after running,
regardless of the point at which atexit() takes place, so I don't think
this is parallel.  All of which goes to show that there's no killer
argument (that I've heard).

Index: Src/builtin.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
retrieving revision 1.169
diff -u -r1.169 builtin.c
--- Src/builtin.c	10 Nov 2006 09:59:27 -0000	1.169
+++ Src/builtin.c	11 Nov 2006 13:05:12 -0000
@@ -4413,6 +4413,11 @@
      * indicate we shouldn't do any recursive processing.
      */
     in_exit = -1;
+    /*
+     * We want to do all remaining processing regardless of preceeding
+     * errors.
+     */
+    errflag = 0;
 
     if (isset(MONITOR)) {
 	/* send SIGHUP to any jobs left running  */
Index: Src/subst.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/subst.c,v
retrieving revision 1.67
diff -u -r1.67 subst.c
--- Src/subst.c	7 Nov 2006 22:47:07 -0000	1.67
+++ Src/subst.c	11 Nov 2006 13:05:16 -0000
@@ -2591,8 +2591,17 @@
 	    if (vunset) {
 		*idend = '\0';
 		zerr("%s: %s", idbeg, *s ? s : "parameter not set");
-		if (!interact)
-		    exit(1);
+		if (!interact) {
+		    if (mypid == getpid()) {
+			/*
+			 * paranoia: don't check for jobs, but there shouldn't
+			 * be any if not interactive.
+			 */
+			stopmsg = 1;
+			zexit(1, 0);
+		    } else
+			_exit(1);
+		}
 		return NULL;
 	    }
 	    break;

-- 
Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/