Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

Re: Interrupting globs (Re: Something rotten in tar completion)



On Fri, 5 Dec 2014 20:34:17 +0000
Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx> wrote:
> On the other problem I came up with, that eval is resetting errflag even
> if you've interrupted: how about the following?  Add a bit to errflag to
> signify that the user interrupted the shell rather than that some
> internal error (e.g. syntax) occurred.  Only reset this new bit in a few
> key places: the main command loop when executing, the top of ZLE when
> editing being the obvious places.  Convert other "errflag = 0"
> assignments case by case so that they just remove bit 0; then eval can
> continue to do its job of acting as a sandbox but without screwing up
> the behaviour of interrupts.  I think doing that is fairly mechanical
> and it achieves what's needed without compromising anything else.

Here's my first go; it does seem to do what I want, and as a by-product
fixes all the little race conditions we've always had that meant you
couldn't interrupt chunks of code that were executed in any kind of
sandbox because the condition got reset afterwards.  I think a few of
these have been annoying me over the years.

The general strategy is to use the bit ERRFLAG_ERROR for internal
failures and ERRFLAG_INT for user interrupts.  There are only two
cases of the latter: on an untrapped SIGINT, the obvious case, and also
on a trapped SIGINT or SIGQUIT where we've been told to behave as if the
trap didn't trap the error condition.  That's straightforward for
SIGINT, less so for SIGQUIT but I took my cue from the fact that Bart
thought it worthwhile trapping SIGQUIT as an interactive "no, I really
mean abort" in completion, which implies that if we trap it we want it
to work at least as well as SIGINT.

Correspondingly, most of the time only the ERRFLAG_ERROR bit gets
reset.  ERRFLAG_INT gets reset only in the following cases:

- in the main command loop.  This is what causes the shell not to exit but
instead go back to the main command loop when you ^C a command.

- at the start of zleread, so we can read the next thing to do whatever
just happened.  I'm not sure this is particularly useful since
in this case you'd typically expect the previous condition to have
occurred and it could mean e.g. you ignore an interrupt that
occurred just before a "vared".

- when we just finished completion.  This is needed so that the cases
that got this whole business kicked off behave as now (but more
reliably) --- a ^C gets you back to the command line, but the command
line is not trashed as it would be if you ^Ced outside completion (try
it if you're confused).  There's a race here, but it's no worse than it
ever was.

To ensure ERRFLAG_INT doesn't get reset unnecessarily there are a number
of cases where restoring errflag to a previously saved value keeps the
ERRFLAG_INT bit if it got set in the meanwhile.  I hope the rationale
here is obvious --- the ERRFLAG_ERROR is an internal state that needs
resetting, the ERRFLAG_INT an asynchronous condition where the user
doesn't care what the internal state is.

By the way, looking at the patch below you might wonder if it wouldn't
be more efficient to add a separate flag for interrupt error conditions
to test.  It wouldn't --- there are many more cases where errflag is
tested than when it is set, not affected by the patch below.

I suspect we'll just have to try this out and see how it works.

pws


diff --git a/Src/Modules/zpty.c b/Src/Modules/zpty.c
index 63c79a7..7b6130c 100644
--- a/Src/Modules/zpty.c
+++ b/Src/Modules/zpty.c
@@ -308,7 +308,7 @@ newptycmd(char *nam, char *pname, char **args, int echo, int nblock)
 
     prog = parse_string(zjoin(args, ' ', 1), 0);
     if (!prog) {
-	errflag = 0;
+	errflag &= ~ERRFLAG_ERROR;
 	scriptname = oscriptname;
 	ineval = oineval;
 	return 1;
diff --git a/Src/Modules/zutil.c b/Src/Modules/zutil.c
index 1cca0c4..c894950 100644
--- a/Src/Modules/zutil.c
+++ b/Src/Modules/zutil.c
@@ -301,7 +301,8 @@ setstypat(Style s, char *pat, Patprog prog, char **vals, int eval)
 	int ef = errflag;
 
 	eprog = parse_string(zjoin(vals, ' ', 1), 0);
-	errflag = ef;
+	/* Keep any user interrupt error status */
+	errflag = ef | (errflag & ERRFLAG_INT);
 
 	if (!eprog)
 	{
@@ -394,10 +395,11 @@ evalstyle(Stypat p)
     unsetparam("reply");
     execode(p->eval, 1, 0, "style");
     if (errflag) {
-	errflag = ef;
+	/* Keep any user interrupt error status */
+	errflag = ef | (errflag & ERRFLAG_INT);
 	return NULL;
     }
-    errflag = ef;
+    errflag = ef | (errflag & ERRFLAG_INT);
 
     queue_signals();
     if ((ret = getaparam("reply")))
diff --git a/Src/Zle/compcore.c b/Src/Zle/compcore.c
index 35d410c..b0c6e06 100644
--- a/Src/Zle/compcore.c
+++ b/Src/Zle/compcore.c
@@ -1671,7 +1671,7 @@ set_comp_sep(void)
     noaliases = ona;
     strinend();
     inpop();
-    errflag = 0;
+    errflag &= ~ERRFLAG_ERROR;
     noerrs = ne;
     lexrestore();
     wb = owb;
diff --git a/Src/Zle/compctl.c b/Src/Zle/compctl.c
index 0b7a324..d15c2d1 100644
--- a/Src/Zle/compctl.c
+++ b/Src/Zle/compctl.c
@@ -1879,7 +1879,7 @@ ccmakehookfn(UNUSED(Hookdef dummy), struct ccmakedat *dat)
 	if (!m || !(m = m->next))
 	    break;
 
-	errflag = 0;
+	errflag &= ~ERRFLAG_ERROR;
     }
     redup(osi, 0);
     dat->lst = 1;
@@ -2121,7 +2121,7 @@ getreal(char *str)
     if (!errflag && nonempty(l) &&
 	((char *) peekfirst(l)) && ((char *) peekfirst(l))[0])
 	return dupstring(peekfirst(l));
-    errflag = 0;
+    errflag &= ~ERRFLAG_ERROR;
 
     return dupstring(str);
 }
@@ -2599,7 +2599,7 @@ makecomplistlist(Compctl cc, char *s, int incmd, int compadd)
 	makecomplistflags(cc, s, incmd, compadd);
 
     /* Reset some information variables for the next try. */
-    errflag = 0;
+    errflag &= ~ERRFLAG_ERROR;
     offs = oloffs;
     wb = owb;
     we = owe;
@@ -2847,7 +2847,7 @@ sep_comp_string(char *ss, char *s, int noffs)
     noaliases = ona;
     strinend();
     inpop();
-    errflag = 0;
+    errflag &= ~ERRFLAG_ERROR;
     noerrs = ne;
     lexrestore();
     wb = owb;
@@ -3725,7 +3725,7 @@ makecomplistflags(Compctl cc, char *s, int incmd, int compadd)
 	noaliases = ona;
 	strinend();
 	inpop();
-	errflag = 0;
+	errflag &= ~ERRFLAG_ERROR;
 	lexrestore();
 	/* Fine, now do full expansion. */
 	prefork(foo, 0);
diff --git a/Src/Zle/compresult.c b/Src/Zle/compresult.c
index fcceb67..93438a0 100644
--- a/Src/Zle/compresult.c
+++ b/Src/Zle/compresult.c
@@ -1092,7 +1092,7 @@ do_single(Cmatch m)
 			noerrs = 1;
 			parsestr(p);
 			singsub(&p);
-			errflag = 0;
+			errflag &= ~ERRFLAG_ERROR;
 			noerrs = ne;
 		    }
 		} else {
diff --git a/Src/Zle/textobjects.c b/Src/Zle/textobjects.c
index 85d014b..37d2c0a 100644
--- a/Src/Zle/textobjects.c
+++ b/Src/Zle/textobjects.c
@@ -275,7 +275,7 @@ selectargument(UNUSED(char **args))
     noaliases = ona;
     strinend();
     inpop();
-    errflag = 0;
+    errflag &= ~ERRFLAG_ERROR;
     noerrs = ne;
     lexrestore();
     zlemetacs = ocs;
diff --git a/Src/Zle/zle_hist.c b/Src/Zle/zle_hist.c
index 9f65994..88623bb 100644
--- a/Src/Zle/zle_hist.c
+++ b/Src/Zle/zle_hist.c
@@ -853,8 +853,10 @@ pushlineoredit(char **args)
 	free(zhline);
     }
     ret = pushline(args);
-    if (!isfirstln)
-	errflag = done = 1;
+    if (!isfirstln) {
+	errflag |= ERRFLAG_ERROR;
+	done = 1;
+    }
     clearlist = 1;
     return ret;
 }
diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c
index caa052b..616f354 100644
--- a/Src/Zle/zle_main.c
+++ b/Src/Zle/zle_main.c
@@ -744,7 +744,7 @@ raw_getbyte(long do_keytmout, char *cptr)
 			}
 			if (errflag) {
 			    /* No sensible way of handling errors here */
-			    errflag = 0;
+			    errflag &= ~ERRFLAG_ERROR;
 			    /*
 			     * Paranoia: don't run the hooks again this
 			     * time.
@@ -882,7 +882,7 @@ getbyte(long do_keytmout, int *timeout)
 		die = 0;
 		if (!errflag && !retflag && !breaks && !exit_pending)
 		    continue;
-		errflag = 0;
+		errflag &= ~ERRFLAG_ERROR;
 		breaks = obreaks;
 		errno = old_errno;
 		return lastchar = EOF;
@@ -1075,7 +1075,7 @@ zlecore(void)
 		DECCS();
 	    handleundo();
 	} else {
-	    errflag = 1;
+	    errflag |= ERRFLAG_ERROR;
 	    break;
 	}
 #ifdef HAVE_POLL
@@ -1233,6 +1233,10 @@ zleread(char **lp, char **rp, int flags, int context, char *init, char *finish)
 
     zleactive = 1;
     resetneeded = 1;
+    /*
+     * Start of the main zle read.
+     * Fully reset error conditions, including error interrupt.
+     */
     errflag = retflag = 0;
     lastcol = -1;
     initmodifier(&zmod);
@@ -1658,7 +1662,7 @@ bin_vared(char *name, char **args, Options ops, UNUSED(int func))
     }
     if (!t || errflag) {
 	/* error in editing */
-	errflag = 0;
+	errflag &= ~ERRFLAG_ERROR;
 	breaks = obreaks;
 	if (t)
 	    zsfree(t);
@@ -1778,7 +1782,7 @@ recursiveedit(UNUSED(char **args))
     zrefresh();
     zlecore();
 
-    locerror = errflag;
+    locerror = errflag ? 1 : 0;
     errflag = done = eofsent = 0;
 
     return locerror;
diff --git a/Src/Zle/zle_misc.c b/Src/Zle/zle_misc.c
index d432acf..23286fc 100644
--- a/Src/Zle/zle_misc.c
+++ b/Src/Zle/zle_misc.c
@@ -1041,7 +1041,7 @@ copyprevshellword(UNUSED(char **args))
 int
 sendbreak(UNUSED(char **args))
 {
-    errflag = 1;
+    errflag |= ERRFLAG_ERROR;
     return 1;
 }
 
diff --git a/Src/Zle/zle_tricky.c b/Src/Zle/zle_tricky.c
index b15d91c..864f804 100644
--- a/Src/Zle/zle_tricky.c
+++ b/Src/Zle/zle_tricky.c
@@ -829,7 +829,7 @@ docomplete(int lst)
 	    if (olst == COMP_EXPAND_COMPLETE &&
 		!strcmp(ol, zlemetaline)) {
 		zlemetacs = ocs;
-		errflag = 0;
+		errflag &= ~ERRFLAG_ERROR;
 
 		if (!compfunc) {
 		    char *p;
@@ -877,6 +877,19 @@ docomplete(int lst)
 
     active = 0;
     makecommaspecial(0);
+
+    /*
+     * As a special case, we reset user interrupts here.
+     * That's because completion is an intensive piece of
+     * computation that the user might want to interrupt separately
+     * from anything else going on.  If they do, they probably
+     * want to keep the line edit buffer intact.
+     *
+     * There's a race here that the user might hit ^C just
+     * after completion exited anyway, but that's inevitable.
+     */
+    errflag &= ~ERRFLAG_INT;
+
     return dat[1];
 }
 
@@ -1394,7 +1407,8 @@ get_comp_string(void)
     }
     strinend();
     inpop();
-    errflag = lexflags = 0;
+    lexflags = 0;
+    errflag &= ~ERRFLAG_ERROR;
     if (parbegin != -1) {
 	/* We are in command or process substitution if we are not in
 	 * a $((...)). */
@@ -2917,7 +2931,7 @@ getcurcmd(void)
     popheap();
     strinend();
     inpop();
-    errflag = 0;
+    errflag &= ~ERRFLAG_ERROR;
     unmetafy_line();
     lexrestore();
 
diff --git a/Src/Zle/zle_utils.c b/Src/Zle/zle_utils.c
index 08a32c3..de91182 100644
--- a/Src/Zle/zle_utils.c
+++ b/Src/Zle/zle_utils.c
@@ -1715,7 +1715,8 @@ zlecallhook(char *name, char *arg)
     execzlefunc(thingy, args, 1);
     unrefthingy(thingy);
 
-    errflag = saverrflag;
+    /* Retain any user interrupt error status */
+    errflag = saverrflag | (errflag & ERRFLAG_INT);
     retflag = savretflag;
 }
 
diff --git a/Src/builtin.c b/Src/builtin.c
index c2af51f..ad3a192 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -422,7 +422,7 @@ execbuiltin(LinkList args, Builtin bn)
 	argc -= argv - argarr;
 
 	if (errflag) {
-	    errflag = 0;
+	    errflag &= ~ERRFLAG_ERROR;
 	    return 1;
 	}
 
@@ -3136,7 +3136,7 @@ bin_unset(char *name, char **argv, Options ops, int func)
 		    }
 		}
 		returnval = errflag;
-		errflag = 0;
+		errflag &= ~ERRFLAG_ERROR;
 	    } else {
 		zerrnam(name, "%s: invalid element for unset", s);
 		returnval = 1;
@@ -4242,7 +4242,7 @@ bin_print(char *name, char **args, Options ops, int func)
 		if (*argp) {
 		    width = (int)mathevali(*argp++);
 		    if (errflag) {
-			errflag = 0;
+			errflag &= ~ERRFLAG_ERROR;
 			ret = 1;
 		    }
 		}
@@ -4272,7 +4272,7 @@ bin_print(char *name, char **args, Options ops, int func)
 		    if (*argp) {
 			prec = (int)mathevali(*argp++);
 			if (errflag) {
-			    errflag = 0;
+			    errflag &= ~ERRFLAG_ERROR;
 			    ret = 1;
 			}
 		    }
@@ -4452,7 +4452,7 @@ bin_print(char *name, char **args, Options ops, int func)
 			zlongval = (curarg) ? mathevali(curarg) : 0;
 			if (errflag) {
 			    zlongval = 0;
-			    errflag = 0;
+			    errflag &= ~ERRFLAG_ERROR;
 			    ret = 1;
 			}
 			print_val(zlongval)
@@ -4481,7 +4481,7 @@ bin_print(char *name, char **args, Options ops, int func)
 			} else doubleval = 0;
 			if (errflag) {
 			    doubleval = 0;
-			    errflag = 0;
+			    errflag &= ~ERRFLAG_ERROR;
 			    ret = 1;
 			}
 			print_val(doubleval)
@@ -4494,7 +4494,7 @@ bin_print(char *name, char **args, Options ops, int func)
 			zulongval = (curarg) ? mathevali(curarg) : 0;
 			if (errflag) {
 			    zulongval = 0;
-			    errflag = 0;
+			    errflag &= ~ERRFLAG_ERROR;
 			    ret = 1;
 			}
 			print_val(zulongval)
@@ -4871,7 +4871,7 @@ zexit(int val, int from_where)
     in_exit = -1;
     /*
      * We want to do all remaining processing regardless of preceding
-     * errors.
+     * errors, even user interrupts.
      */
     errflag = 0;
 
@@ -5074,7 +5074,7 @@ eval(char **argv)
     if (fpushed)
 	funcstack = funcstack->prev;
 
-    errflag = 0;
+    errflag &= ~ERRFLAG_ERROR;
     scriptname = oscriptname;
     ineval = oineval;
 
@@ -6101,7 +6101,7 @@ bin_test(char *name, char **argv, UNUSED(Options ops), int func)
     condlex = zshlex;
 
     if (errflag) {
-	errflag = 0;
+	errflag &= ~ERRFLAG_ERROR;
 	lexrestore();
 	return 1;
     }
@@ -6278,7 +6278,7 @@ bin_let(UNUSED(char *name), char **argv, UNUSED(Options ops), UNUSED(int func))
     while (*argv)
 	val = matheval(*argv++);
     /* Errors in math evaluation in let are non-fatal. */
-    errflag = 0;
+    errflag &= ~ERRFLAG_ERROR;
     /* should test for fabs(val.u.d) < epsilon? */
     return (val.type == MN_INTEGER) ? val.u.l == 0 : val.u.d == 0.0;
 }
diff --git a/Src/exec.c b/Src/exec.c
index a5f8771..c18f3cd 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -59,7 +59,7 @@ mod_export int noerrs;
 /**/
 int nohistsave;
 
-/* error/break flag */
+/* error flag: bits from enum errflag_bits */
 
 /**/
 mod_export int errflag;
@@ -1601,7 +1601,8 @@ execpline(Estate state, wordcode slcode, int how, int last1)
 			    (killpg(jobtab[list_pipe_job].gleader, 0) == -1 ? 2 : 1);
 			list_pipe_pid = pid;
 			list_pipe_start = bgtime;
-			nowait = errflag = 1;
+			nowait = 1;
+			errflag |= ERRFLAG_ERROR;
 			breaks = loops;
 			close(synch[1]);
 			read_loop(synch[0], &dummy, 1);
@@ -1634,7 +1635,10 @@ execpline(Estate state, wordcode slcode, int how, int last1)
 			list_pipe_child = 1;
 			opts[INTERACTIVE] = 0;
 			if (errbrk_saved) {
-			    errflag = prev_errflag;
+			    /*
+			     * Keep any user interrupt bit in errflag.
+			     */
+			    errflag = prev_errflag | (errflag & ERRFLAG_INT);
 			    breaks = prev_breaks;
 			}
 			break;
@@ -1719,12 +1723,14 @@ execpline2(Estate state, wordcode pcode,
 
 	    if (pipe(synch) < 0) {
 		zerr("pipe failed: %e", errno);
-		lastval = errflag = 1;
+		lastval = 1;
+		errflag |= ERRFLAG_ERROR;
 		return;
 	    } else if ((pid = zfork(&bgtime)) == -1) {
 		close(synch[0]);
 		close(synch[1]);
-		lastval = errflag = 1;
+		lastval = 1;
+		errflag |= ERRFLAG_ERROR;
 		return;
 	    } else if (pid) {
 		char dummy, *text;
@@ -2560,7 +2566,8 @@ execcmd(Estate state, int input, int output, int how, int last1)
 		while (next && *next == '-' && strlen(next) >= 2) {
 		    if (!firstnode(args)) {
 			zerr("exec requires a command to execute");
-			errflag = lastval = 1;
+			lastval = 1;
+			errflag |= ERRFLAG_ERROR;
 			goto done;
 		    }
 		    uremnode(args, firstnode(args));
@@ -2577,12 +2584,14 @@ execcmd(Estate state, int input, int output, int how, int last1)
 			    } else {
 				if (!firstnode(args)) {
 				    zerr("exec requires a command to execute");
-				    errflag = lastval = 1;
+				    lastval = 1;
+				    errflag |= ERRFLAG_ERROR;
 				    goto done;
 				}
 				if (!nextnode(firstnode(args))) {
 				    zerr("exec flag -a requires a parameter");
-				    errflag = lastval = 1;
+				    lastval = 1;
+				    errflag |= ERRFLAG_ERROR;
 				    goto done;
 				}
 				exec_argv0 = (char *)
@@ -2598,7 +2607,8 @@ execcmd(Estate state, int input, int output, int how, int last1)
 			    break;
 			default:
 			    zerr("unknown exec flag -%c", *cmdopt);
-			    errflag = lastval = 1;
+			    lastval = 1;
+			    errflag |= ERRFLAG_ERROR;
 			    return;
 			}
 		    }
@@ -2661,7 +2671,8 @@ execcmd(Estate state, int input, int output, int how, int last1)
 		    } else if (!nullcmd || !*nullcmd || opts[CSHNULLCMD] ||
 			       (cflags & BINF_PREFIX)) {
 			zerr("redirection with no command");
-			errflag = lastval = 1;
+			lastval = 1;
+			errflag |= ERRFLAG_ERROR;
 			return;
 		    } else if (!nullcmd || !*nullcmd || opts[SHNULLCMD]) {
 			if (!args)
@@ -2691,7 +2702,7 @@ execcmd(Estate state, int input, int output, int how, int last1)
 		    if (varspc)
 			addvars(state, varspc, 0);
 		    if (errflag)
-			lastval = errflag;
+			lastval = errflag ? 1 : 0;
 		    else
 			lastval = cmdoutval;
 		    if (isset(XTRACE)) {
@@ -2795,7 +2806,7 @@ execcmd(Estate state, int input, int output, int how, int last1)
 	    }
 	}
 	if (!nextnode(firstnode(args)))
-	    errflag = 1;
+	    errflag |= ERRFLAG_ERROR;
     }
 
     if (type == WC_FUNCDEF) {
@@ -2940,7 +2951,8 @@ execcmd(Estate state, int input, int output, int how, int last1)
 	} else if ((pid = zfork(&bgtime)) == -1) {
 	    close(synch[0]);
 	    close(synch[1]);
-	    lastval = errflag = 1;
+	    lastval = 1;
+	    errflag |= ERRFLAG_ERROR;
 	    goto fatal;
 	}
 	if (pid) {
@@ -3529,7 +3541,7 @@ execcmd(Estate state, int input, int output, int how, int last1)
 		else
 		    exit(1);
 	    }
-	    errflag = 1;
+	    errflag |= ERRFLAG_ERROR;
 	}
     }
     if (newxtrerr) {
@@ -3759,8 +3771,10 @@ gethere(char **strp, int typ)
 
 	parsestr(buf);
 
-	if (!errflag)
-	    errflag = ef;
+	if (!errflag) {
+	    /* Retain any user interrupt error */
+	    errflag = ef | (errflag & ERRFLAG_INT);
+	}
     }
     s = dupstring(buf);
     zfree(buf, bsiz);
@@ -3854,7 +3868,7 @@ getoutput(char *cmd, int qt)
 	return readoutput(stream, qt);
     }
     if (mpipe(pipes) < 0) {
-	errflag = 1;
+	errflag |= ERRFLAG_ERROR;
 	cmdoutpid = 0;
 	return NULL;
     }
@@ -3864,7 +3878,7 @@ getoutput(char *cmd, int qt)
 	/* fork error */
 	zclose(pipes[0]);
 	zclose(pipes[1]);
-	errflag = 1;
+	errflag |= ERRFLAG_ERROR;
 	cmdoutpid = 0;
 	child_unblock();
 	return NULL;
@@ -4274,7 +4288,7 @@ execcond(Estate state, UNUSED(int do_exec))
      * into a shell error.
      */
     if (stat == 2)
-	errflag = 1;
+	errflag |= ERRFLAG_ERROR;
     cmdpop();
     if (isset(XTRACE)) {
 	fprintf(xtrerr, " ]]\n");
@@ -4314,7 +4328,7 @@ execarith(Estate state, UNUSED(int do_exec))
 	fflush(xtrerr);
     }
     if (errflag) {
-	errflag = 0;
+	errflag &= ~ERRFLAG_ERROR;
 	return 2;
     }
     /* should test for fabs(val.u.d) < epsilon? */
@@ -4932,7 +4946,7 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
 						(name = fname)))) {
 	    zwarn("%s: function not defined by file", name);
 	    if (noreturnval)
-		errflag = 1;
+		errflag |= ERRFLAG_ERROR;
 	    else
 		lastval = 1;
 	    goto doneshfunc;
diff --git a/Src/glob.c b/Src/glob.c
index ca7bc44..f5ac47a 100644
--- a/Src/glob.c
+++ b/Src/glob.c
@@ -682,7 +682,7 @@ parsecomplist(char *instr)
 	/* Now get the next path component if there is one. */
 	l1 = (Complist) zhalloc(sizeof *l1);
 	if ((l1->next = parsecomplist(instr)) == NULL) {
-	    errflag = 1;
+	    errflag |= ERRFLAG_ERROR;
 	    return NULL;
 	}
 	l1->pat = patcompile(NULL, compflags | PAT_ANY, NULL);
@@ -728,7 +728,7 @@ parsecomplist(char *instr)
 	    return (ef && !l1->next) ? NULL : l1;
 	}
     }
-    errflag = 1;
+    errflag |= ERRFLAG_ERROR;
     return NULL;
 }
 
@@ -1790,7 +1790,7 @@ zglob(LinkList list, LinkNode np, int nountok)
 	    insertlinknode(list, node, ostr);
 	    return;
 	}
-	errflag = 0;
+	errflag &= ~ERRFLAG_ERROR;
 	zerr("bad pattern: %s", ostr);
 	return;
     }
@@ -1873,7 +1873,8 @@ zglob(LinkList list, LinkNode np, int nountok)
 				tmpptr->sortstrs[iexec] = tmpptr->name;
 			}
 
-			errflag = ef;
+			/* Retain any user interrupt error status */
+			errflag = ef | (errflag & ERRFLAG_INT);
 			lastval = lv;
 		    } else {
 			/* Failed, let's be safe */
@@ -3733,7 +3734,8 @@ qualsheval(char *name, UNUSED(struct stat *buf), UNUSED(off_t days), char *str)
 	execode(prog, 1, 0, "globqual");
 
 	ret = lastval;
-	errflag = ef;
+	/* Retain any user interrupt error status */
+	errflag = ef | (errflag & ERRFLAG_INT);
 	lastval = lv;
 
 	if (!(inserts = getaparam("reply")) &&
diff --git a/Src/hist.c b/Src/hist.c
index 7fe843a..a006170 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -287,7 +287,8 @@ ihgetc(void)
 	c = histsubchar(c);
 	if (c < 0) {
 	    /* bad expansion */
-	    errflag = lexstop = 1;
+	    lexstop = 1;
+	    errflag |= ERRFLAG_ERROR;
 	    return ' ';
 	}
     }
@@ -721,7 +722,7 @@ histsubchar(int c)
 		    noerrs = 1;
 		    parse_subst_string(sline);
 		    noerrs = one;
-		    errflag = oef;
+		    errflag = oef | (errflag & ERRFLAG_INT);
 		    remnulargs(sline);
 		    untokenize(sline);
 		}
@@ -880,7 +881,8 @@ hbegin(int dohist)
     char *hf;
 
     isfirstln = isfirstch = 1;
-    errflag = histdone = 0;
+    errflag &= ~ERRFLAG_ERROR;
+    histdone = 0;
     if (!dohist)
 	stophist = 2;
     else if (dohist != 2)
@@ -3182,7 +3184,7 @@ bufferwords(LinkList list, char *buf, int *index, int flags)
     noaliases = ona;
     strinend();
     inpop();
-    errflag = 0;
+    errflag &= ~ERRFLAG_ERROR;
     nocomments = onc;
     noerrs = ne;
     lexrestore();
diff --git a/Src/init.c b/Src/init.c
index 6551661..3cbd4e5 100644
--- a/Src/init.c
+++ b/Src/init.c
@@ -123,6 +123,11 @@ loop(int toplevel, int justonce)
 		    hbegin(1);
 		else
 		    stophist = hstop;
+		/*
+		 * Reset all errors, including user interupts.
+		 * This is what allows ^C in an interactive shell
+		 * to return us to the command line.
+		 */
 		errflag = 0;
 	    }
 	}
@@ -178,7 +183,15 @@ loop(int toplevel, int justonce)
 
 		/* The only permanent storage is from getpermtext() */
 		zsfree(cmdstr);
-		errflag = 0;
+		/*
+		 * Note this does *not* remove a user interrupt error
+		 * condition, even though we're at the top level loop:
+		 * that would be inconsistent with the case where
+		 * we didn't execute a preexec function.  This is
+		 * an implementation detail that an interrupting user
+		 * does't care about.
+		 */
+		errflag &= ~ERRFLAG_ERROR;
 	    }
 	    if (stopmsg)	/* unset 'you have stopped jobs' flag */
 		stopmsg--;
@@ -689,7 +702,7 @@ init_term(void)
     {
 	if (isset(INTERACTIVE))
 	    zerr("can't find terminal definition for %s", term);
-	errflag = 0;
+	errflag &= ~ERRFLAG_ERROR;
 	termflags |= TERM_BAD;
 	return 0;
     } else {
@@ -1336,7 +1349,7 @@ source(char *s)
 
     if (prog) {
 	pushheap();
-	errflag = 0;
+	errflag &= ~ERRFLAG_ERROR;
 	execode(prog, 1, 0, "filecode");
 	popheap();
 	if (errflag)
@@ -1379,7 +1392,7 @@ source(char *s)
     lineno = oldlineno;              /* our current lineno                   */
     loops = oloops;                  /* the # of nested loops we are in      */
     dosetopt(SHINSTDIN, oldshst, 1, opts); /* SHINSTDIN option               */
-    errflag = 0;
+    errflag &= ~ERRFLAG_ERROR;
     if (!exit_pending)
 	retflag = 0;
     scriptname = old_scriptname;
diff --git a/Src/input.c b/Src/input.c
index 4ac7e6e..9552331 100644
--- a/Src/input.c
+++ b/Src/input.c
@@ -291,7 +291,8 @@ inputline(void)
     }
     if (errflag) {
 	free(ingetcline);
-	return lexstop = errflag = 1;
+	errflag |= ERRFLAG_ERROR;
+	return lexstop = 1;
     }
     if (isset(VERBOSE)) {
 	/* Output the whole line read so far. */
diff --git a/Src/jobs.c b/Src/jobs.c
index 6663a40..6e47e5e 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -509,7 +509,7 @@ update_job(Job jn)
 			prev_errflag = errflag;
 		    }
 		    breaks = loops;
-		    errflag = 1;
+		    errflag |= ERRFLAG_ERROR;
 		    inerrflush();
 		}
 	    } else {
@@ -526,7 +526,7 @@ update_job(Job jn)
 	    prev_errflag = errflag;
 	}
 	breaks = loops;
-	errflag = 1;
+	errflag |= ERRFLAG_ERROR;
 	inerrflush();
     }
     if (somestopped && jn->stat & STAT_SUPERJOB)
@@ -581,7 +581,7 @@ update_job(Job jn)
 		    breaks = loops;
 	    } else {
 		breaks = loops;
-		errflag = 1;
+		errflag |= ERRFLAG_ERROR;
 	    }
 	    check_cursh_sig(sig);
 	}
@@ -1444,12 +1444,7 @@ zwaitjob(int job, int wait_cmd)
 		restore_queue_signals(q);
 		return 128 + last_signal;
 	    }
-	    /* Commenting this out makes ^C-ing a job started by a function
-	       stop the whole function again.  But I guess it will stop
-	       something else from working properly, we have to find out
-	       what this might be.  --oberon
-
-	    errflag = 0; */
+	    errflag &= ~ERRFLAG_ERROR;
 	    if (subsh) {
 		killjb(jn, SIGCONT);
 		jn->stat &= ~STAT_STOPPED;
diff --git a/Src/lex.c b/Src/lex.c
index 1a854f5..68a892a 100644
--- a/Src/lex.c
+++ b/Src/lex.c
@@ -385,7 +385,7 @@ lexrestore(void)
     ecnfunc = ln->ecnfunc;
     hlinesz = ln->hlinesz;
     toklineno = ln->toklineno;
-    errflag = 0;
+    errflag &= ~ERRFLAG_ERROR;
     free(ln);
 
     unqueue_signals();
@@ -1725,7 +1725,8 @@ parse_subst_string(char *s)
     inpop();
     DPUTS(cmdsp, "BUG: parse_subst_string: cmdstack not empty.");
     lexrestore();
-    errflag = err;
+    /* Keep any interrupt error status */
+    errflag = err | (errflag & ERRFLAG_INT);
     if (ctok == LEXERR) {
 	untokenize(s);
 	return 1;
diff --git a/Src/loop.c b/Src/loop.c
index 82d2fe3..69805ea 100644
--- a/Src/loop.c
+++ b/Src/loop.c
@@ -259,7 +259,8 @@ execselect(Estate state, UNUSED(int do_exec))
 				   0, ZLCON_SELECT);
 		    if (errflag)
 			str = NULL;
-		    errflag = oef;
+		    /* Keep any user interrupt error status */
+		    errflag = oef | (errflag & ERRFLAG_INT);
 	    	} else {
 		    str = promptexpand(prompt3, 0, NULL, NULL, NULL);
 		    zputs(str, stderr);
@@ -671,7 +672,7 @@ exectry(Estate state, int do_exec)
     /* The always clause. */
     save_try_errflag = try_errflag;
     try_errflag = (zlong)errflag;
-    errflag = 0;
+    errflag &= ~ERRFLAG_ERROR;
     save_retflag = retflag;
     retflag = 0;
     save_breaks = breaks;
@@ -682,7 +683,10 @@ exectry(Estate state, int do_exec)
     state->pc = always;
     execlist(state, 1, do_exec);
 
-    errflag = try_errflag ? 1 : 0;
+    if (try_errflag)
+	errflag |= ERRFLAG_ERROR;
+    else
+	errflag &= ~ERRFLAG_ERROR;
     try_errflag = save_try_errflag;
     if (!retflag)
 	retflag = save_retflag;
diff --git a/Src/params.c b/Src/params.c
index 61edc5d..bdace79 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -2654,7 +2654,7 @@ assignsparam(char *s, char *val, int flags)
     if (!isident(s)) {
 	zerr("not an identifier: %s", s);
 	zsfree(val);
-	errflag = 1;
+	errflag |= ERRFLAG_ERROR;
 	return NULL;
     }
     queue_signals();
@@ -2783,7 +2783,7 @@ assignaparam(char *s, char **val, int flags)
     if (!isident(s)) {
 	zerr("not an identifier: %s", s);
 	freearray(val);
-	errflag = 1;
+	errflag |= ERRFLAG_ERROR;
 	return NULL;
     }
     queue_signals();
@@ -2799,7 +2799,7 @@ assignaparam(char *s, char **val, int flags)
 	    zerr("%s: attempt to set slice of associative array",
 		 v->pm->node.nam);
 	    freearray(val);
-	    errflag = 1;
+	    errflag |= ERRFLAG_ERROR;
 	    return NULL;
 	}
 	v = NULL;
@@ -2870,13 +2870,13 @@ sethparam(char *s, char **val)
     if (!isident(s)) {
 	zerr("not an identifier: %s", s);
 	freearray(val);
-	errflag = 1;
+	errflag |= ERRFLAG_ERROR;
 	return NULL;
     }
     if (strchr(s, '[')) {
 	freearray(val);
 	zerr("nested associative arrays not yet supported");
-	errflag = 1;
+	errflag |= ERRFLAG_ERROR;
 	return NULL;
     }
     if (unset(EXECOPT))
@@ -2916,7 +2916,7 @@ setnparam(char *s, mnumber val)
 
     if (!isident(s)) {
 	zerr("not an identifier: %s", s);
-	errflag = 1;
+	errflag |= ERRFLAG_ERROR;
 	return NULL;
     }
     if (unset(EXECOPT))
diff --git a/Src/parse.c b/Src/parse.c
index 4ceeb4e..c1709e0 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -71,13 +71,14 @@ struct heredocs *hdocs;
 
 #define YYERROR(O)  { tok = LEXERR; ecused = (O); return 0; }
 #define YYERRORV(O) { tok = LEXERR; ecused = (O); return; }
-#define COND_ERROR(X,Y) do { \
-  zwarn(X,Y); \
-  herrflush(); \
-  if (noerrs != 2) \
-    errflag = 1; \
-  YYERROR(ecused) \
-} while(0)
+#define COND_ERROR(X,Y) \
+    do {					\
+	zwarn(X,Y);				\
+	herrflush();				\
+	if (noerrs != 2)			\
+	    errflag |= ERRFLAG_ERROR;		\
+	YYERROR(ecused)				\
+	    } while(0)
 
 
 /* 
@@ -506,7 +507,7 @@ par_event(void)
 	yyerror(1);
 	herrflush();
 	if (noerrs != 2)
-	    errflag = 1;
+	    errflag |= ERRFLAG_ERROR;
 	ecused--;
 	return 0;
     } else {
@@ -2339,7 +2340,7 @@ yyerror(int noerr)
 	    zwarn("parse error");
     }
     if (!noerr && noerrs != 2)
-	errflag = 1;
+	errflag |= ERRFLAG_ERROR;
 }
 
 /*
@@ -3031,7 +3032,7 @@ build_dump(char *nam, char *dump, char **files, int ali, int map, int flags)
 	file = metafy(file, flen, META_REALLOC);
 
 	if (!(prog = parse_string(file, 1)) || errflag) {
-	    errflag = 0;
+	    errflag &= ~ERRFLAG_ERROR;
 	    close(dfd);
 	    zfree(file, flen);
 	    zwarnnam(nam, "can't read file: %s", *files);
@@ -3141,7 +3142,7 @@ build_cur_dump(char *nam, char *dump, char **names, int match, int map,
 	    for (hn = shfunctab->nodes[i]; hn; hn = hn->next)
 		if (cur_add_func(nam, (Shfunc) hn, lnames, progs,
 				 &hlen, &tlen, what)) {
-		    errflag = 0;
+		    errflag &= ~ERRFLAG_ERROR;
 		    close(dfd);
 		    unlink(dump);
 		    return 1;
@@ -3166,7 +3167,7 @@ build_cur_dump(char *nam, char *dump, char **names, int match, int map,
 			pattry(pprog, hn->nam) &&
 			cur_add_func(nam, (Shfunc) hn, lnames, progs,
 				     &hlen, &tlen, what)) {
-			errflag = 0;
+			errflag &= ~ERRFLAG_ERROR;
 			close(dfd);
 			unlink(dump);
 			return 1;
@@ -3177,13 +3178,13 @@ build_cur_dump(char *nam, char *dump, char **names, int match, int map,
 	    if (errflag ||
 		!(shf = (Shfunc) shfunctab->getnode(shfunctab, *names))) {
 		zwarnnam(nam, "unknown function: %s", *names);
-		errflag = 0;
+		errflag &= ~ERRFLAG_ERROR;
 		close(dfd);
 		unlink(dump);
 		return 1;
 	    }
 	    if (cur_add_func(nam, shf, lnames, progs, &hlen, &tlen, what)) {
-		errflag = 0;
+		errflag &= ~ERRFLAG_ERROR;
 		close(dfd);
 		unlink(dump);
 		return 1;
@@ -3192,7 +3193,7 @@ build_cur_dump(char *nam, char *dump, char **names, int match, int map,
     }
     if (empty(progs)) {
 	zwarnnam(nam, "no functions");
-	errflag = 0;
+	errflag &= ~ERRFLAG_ERROR;
 	close(dfd);
 	unlink(dump);
 	return 1;
diff --git a/Src/prompt.c b/Src/prompt.c
index 0cc9ef9..3552575 100644
--- a/Src/prompt.c
+++ b/Src/prompt.c
@@ -192,8 +192,11 @@ promptexpand(char *s, int ns, char *rs, char *Rs, unsigned int *txtchangep)
 	if (*s == Nularg && s[1] == '\0')
 	    *s = '\0';
 
-	/* Ignore errors and status change in prompt substitution */
-	errflag = olderr;
+	/*
+	 * Ignore errors and status change in prompt substitution.
+	 * However, keep any user interrupt error that occurred.
+	 */
+	errflag = olderr | (errflag & ERRFLAG_INT);
 	lastval = oldval;
     }
 
diff --git a/Src/signals.c b/Src/signals.c
index e728505..899f121 100644
--- a/Src/signals.c
+++ b/Src/signals.c
@@ -619,7 +619,7 @@ zhandler(int sig)
 		zexit(SIGINT, 1);
             if (list_pipe || chline || simple_pline) {
                 breaks = loops;
-                errflag = 1;
+                errflag |= ERRFLAG_INT;
 		inerrflush();
 		check_cursh_sig(SIGINT);
             }
@@ -640,6 +640,11 @@ zhandler(int sig)
 	    if (idle >= 0 && idle < tmout)
 		alarm(tmout - idle);
 	    else {
+		/*
+		 * We want to exit now.
+		 * Cancel all errors, including a user interrupt
+		 * which is now redundant.
+		 */
 		errflag = noerrs = 0;
 		zwarn("timeout");
 		stopmsg = 1;
@@ -1267,7 +1272,18 @@ dotrapargs(int sig, int *sigtr, void *sigfn)
 	!(isfunc && new_trap_return == 0)) {
 	if (isfunc) {
 	    breaks = loops;
-	    errflag = 1;
+	    /*
+	     * For SIGINT we behave the same as the default behaviour
+	     * i.e. we set the error bit indicating an interrupt.
+	     * We do this with SIGQUIT, too, even though we don't
+	     * handle SIGQUIT by default.  That's to try to make
+	     * it behave a bit more like its normal behaviour when
+	     * the trap handler has told us that's what it wants.
+	     */
+	    if (sig == SIGINT || sig == SIGQUIT)
+		errflag |= ERRFLAG_INT;
+	    else
+		errflag |= ERRFLAG_ERROR;
 	}
 	lastval = new_trap_return;
 	/* return triggered */
@@ -1282,8 +1298,12 @@ dotrapargs(int sig, int *sigtr, void *sigfn)
 	     */
 	    lastval = olastval;
 	}
-	if (try_tryflag)
-	    errflag = traperr;
+	if (try_tryflag) {
+	    if (traperr)
+		errflag |= ERRFLAG_ERROR;
+	    else
+		errflag &= ~ERRFLAG_ERROR;
+	}
 	breaks += obreaks;
 	/* return not triggered: restore old flag */
 	retflag = oretflag;
diff --git a/Src/subst.c b/Src/subst.c
index 61aa1c1..43932c2 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -2822,7 +2822,8 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags)
 		haserr = parse_subst_string(s);
 		noerrs = one;
 		if (!quoteerr) {
-		    errflag = oef;
+		    /* Retain user interrupt error status */
+		    errflag = oef | (errflag & ERRFLAG_INT);
 		    if (haserr)
 			shtokenize(s);
 		} else if (haserr || errflag) {
@@ -3249,8 +3250,10 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags)
 		haserr = 1;
 	}
 	noerrs = one;
-	if (!quoteerr)
-	    errflag = oef;
+	if (!quoteerr) {
+	    /* Retain user interrupt error status */
+	    errflag = oef | (errflag & ERRFLAG_INT);
+	}
 	if (haserr || errflag)
 	    return NULL;
     }
@@ -3483,8 +3486,10 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags)
 		    untokenize(*ap);
 		}
 		noerrs = one;
-		if (!quoteerr)
-		    errflag = oef;
+		if (!quoteerr) {
+		    /* Retain any user interrupt error status */
+		    errflag = oef | (errflag & ERRFLAG_INT);
+		}
 		else if (haserr || errflag) {
 		    zerr("parse error in parameter value");
 		    return NULL;
@@ -3516,8 +3521,10 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags)
 		    noerrs = 1;
 		haserr = parse_subst_string(val);
 		noerrs = one;
-		if (!quoteerr)
-		    errflag = oef;
+		if (!quoteerr) {
+		    /* Retain any user interrupt error status */
+		    errflag = oef | (errflag & ERRFLAG_INT);
+		}
 		else if (haserr || errflag) {
 		    zerr("parse error in parameter value");
 		    return NULL;
@@ -4086,7 +4093,8 @@ modify(char **str, char **ptr)
 			    noerrs = 1;
 			    parse_subst_string(copy);
 			    noerrs = one;
-			    errflag = oef;
+			    /* Retain any user interrupt error status */
+			    errflag = oef | (errflag & ERRFLAG_INT);
 			    remnulargs(copy);
 			    untokenize(copy);
 			}
@@ -4161,7 +4169,8 @@ modify(char **str, char **ptr)
 			noerrs = 1;
 			parse_subst_string(*str);
 			noerrs = one;
-			errflag = oef;
+			/* Retain any user interrupt error status */
+			errflag = oef | (errflag & ERRFLAG_INT);
 			remnulargs(*str);
 			untokenize(*str);
 		    }
diff --git a/Src/utils.c b/Src/utils.c
index 9268147..d6bb614 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -153,7 +153,7 @@ VA_DCL
 
     if (errflag || noerrs) {
 	if (noerrs < 2)
-	    errflag = 1;
+	    errflag |= ERRFLAG_ERROR;
 	return;
     }
 
@@ -161,7 +161,7 @@ VA_DCL
     VA_GET_ARG(ap, fmt, const char *);
     zwarning(NULL, fmt, ap);
     va_end(ap);
-    errflag = 1;
+    errflag |= ERRFLAG_ERROR;
 }
 
 /**/
@@ -181,7 +181,7 @@ VA_DCL
     VA_GET_ARG(ap, fmt, const char *);
     zwarning(cmd, fmt, ap);
     va_end(ap);
-    errflag = 1;
+    errflag |= ERRFLAG_ERROR;
 }
 
 /**/
@@ -330,7 +330,7 @@ zerrmsg(FILE *file, const char *fmt, va_list ap)
 		num = va_arg(ap, int);
 		if (num == EINTR) {
 		    fputs("interrupt\n", file);
-		    errflag = 1;
+		    errflag |= ERRFLAG_ERROR;
 		    return;
 		}
 		errmsg = strerror(num);
diff --git a/Src/zsh.h b/Src/zsh.h
index 031deaf..b366e0f 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -2623,6 +2623,20 @@ enum trap_state {
 #define IN_EVAL_TRAP() \
     (intrap && !trapisfunc && traplocallevel == locallevel)
 
+/*
+ * Bits in the errflag variable.
+ */
+enum errflag_bits {
+    /*
+     * Standard internal error bit.
+     */
+    ERRFLAG_ERROR = 1,
+    /*
+     * User interrupt.
+     */
+    ERRFLAG_INT = 2
+};
+
 /***********/
 /* Sorting */
 /***********/



Messages sorted by: Reverse Date, Date, Thread, Author