fix autoloaded trap bug; rejig use of trapfuncs

(now traplists); improve trap tests

9 files changed, 129 insertions, 41 deletions

diff --git a/ChangeLog b/ChangeLog
index 269ef06ef..aca8d024d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2005-02-06  Peter Stephenson  <pws@pwstephenson.fsnet.co.uk>
+
+	* (cheated and guessed) 20793: Src/builtin.c, Src/exec.c,
+	Src/hashtable.c, Src/Modules/parameter.c, Src/Module/zftp.c,
+	Src/parse.c, Src/signals.c, Test/C03traps.ztst: Fix bug that
+	autoloaded TRAPEXIT wasn't restored properly after running an
+	intermediate function; only use sigfuncs (renamed to siglists) for
+	eval-style traps; augment and fix trap tests.
+
 2005-02-04  Peter Stephenson  <pws@csr.com>
 
 	* 20787: configure.ac, Config/defs.mk.in, Doc/.distfiles,
diff --git a/Src/Modules/parameter.c b/Src/Modules/parameter.c
index e4dab1e64..966b26e6f 100644
--- a/Src/Modules/parameter.c
+++ b/Src/Modules/parameter.c
@@ -332,13 +332,12 @@ setfunction(char *name, char *val, int dis)
 
     if (!strncmp(name, "TRAP", 4) &&
 	(sn = getsignum(name + 4)) != -1) {
-	if (settrap(sn, shf->funcdef)) {
+	if (settrap(sn, NULL, ZSIG_FUNC)) {
 	    freeeprog(shf->funcdef);
 	    zfree(shf, sizeof(*shf));
 	    zsfree(val);
 	    return;
 	}
-	sigtrapped[sn] |= ZSIG_FUNC;
     }
     shfunctab->addnode(shfunctab, ztrdup(name), shf);
     zsfree(val);
diff --git a/Src/Modules/zftp.c b/Src/Modules/zftp.c
index 399070939..f97cde46f 100644
--- a/Src/Modules/zftp.c
+++ b/Src/Modules/zftp.c
@@ -436,7 +436,8 @@ zfunalarm(void)
     } else
 	alarm(0);
     if (sigtrapped[SIGALRM] || interact) {
-	if (sigfuncs[SIGALRM] || !sigtrapped[SIGALRM])
+	if (siglists[SIGALRM] || !sigtrapped[SIGALRM] ||
+	    (sigtrapped[SIGALRM] & ZSIG_FUNC))
 	    install_handler(SIGALRM);
 	else
 	    signal_ignore(SIGALRM);
@@ -452,7 +453,7 @@ static void
 zfunpipe()
 {
     if (sigtrapped[SIGPIPE]) {
-	if (sigfuncs[SIGPIPE])
+	if (siglists[SIGPIPE] || (sigtrapped[SIGPIPE] & ZSIG_FUNC))
 	    install_handler(SIGPIPE);
 	else
 	    signal_ignore(SIGPIPE);
diff --git a/Src/builtin.c b/Src/builtin.c
index 1ccbd6fb8..6fcfabfda 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -2617,14 +2617,12 @@ bin_functions(char *name, char **argv, Options ops, int func)
 	    shfunctab->addnode(shfunctab, ztrdup(*argv), shf);
 
 	    if (signum != -1) {
-		if (settrap(signum, shf->funcdef)) {
+		if (settrap(signum, NULL, ZSIG_FUNC)) {
 		    shfunctab->removenode(shfunctab, *argv);
 		    shfunctab->freenode((HashNode)shf);
 		    returnval = 1;
 		    ok = 0;
 		}
-		else
-		    sigtrapped[signum] |= ZSIG_FUNC;
 	    }
 
 	    if (ok && OPT_ISSET(ops,'X') &&
@@ -4967,10 +4965,10 @@ bin_trap(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
 		    shfunctab->printnode(hn, 0);
 		DPUTS(!hn, "BUG: I did not find any trap functions!");
 	    } else if (sigtrapped[sig]) {
-		if (!sigfuncs[sig])
+		if (!siglists[sig])
 		    printf("trap -- '' %s\n", sigs[sig]);
 		else {
-		    s = getpermtext(sigfuncs[sig], NULL);
+		    s = getpermtext(siglists[sig], NULL);
 		    printf("trap -- ");
 		    quotedzputs(s, stdout);
 		    printf(" %s\n", sigs[sig]);
@@ -5013,7 +5011,7 @@ bin_trap(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
 	    break;
 	}
 	t = dupeprog(prog, 0);
-	if (settrap(sig, t))
+	if (settrap(sig, t, 0))
 	    freeeprog(t);
     }
     return *argv != NULL;
diff --git a/Src/exec.c b/Src/exec.c
index abee8329e..3c20f9352 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -2650,8 +2650,8 @@ entersubsh(int how, int cl, int fake, int revertpgrp)
 		unsettrap(sig);
     if (!(monitor = isset(MONITOR))) {
 	if (how & Z_ASYNC) {
-	    settrap(SIGINT, NULL);
-	    settrap(SIGQUIT, NULL);
+	    settrap(SIGINT, NULL, 0);
+	    settrap(SIGQUIT, NULL, 0);
 	    if (isatty(0)) {
 		close(0);
 		if (open("/dev/null", O_RDWR | O_NOCTTY)) {
@@ -3340,13 +3340,12 @@ execfuncdef(Estate state, UNUSED(int do_exec))
 	/* is this shell function a signal trap? */
 	if (!strncmp(s, "TRAP", 4) &&
 	    (signum = getsignum(s + 4)) != -1) {
-	    if (settrap(signum, shf->funcdef)) {
+	    if (settrap(signum, NULL, ZSIG_FUNC)) {
 		freeeprog(shf->funcdef);
 		zfree(shf, sizeof(*shf));
 		state->pc = end;
 		return 1;
 	    }
-	    sigtrapped[signum] |= ZSIG_FUNC;
 
 	    /*
 	     * Remove the old node explicitly in case it has
diff --git a/Src/hashtable.c b/Src/hashtable.c
index b06800b41..470978574 100644
--- a/Src/hashtable.c
+++ b/Src/hashtable.c
@@ -819,7 +819,6 @@ disableshfuncnode(HashNode hn, UNUSED(int flags))
     if (!strncmp(hn->nam, "TRAP", 4)) {
 	int signum = getsignum(hn->nam + 4);
 	sigtrapped[signum] &= ~ZSIG_FUNC;
-	sigfuncs[signum] = NULL;
 	unsettrap(signum);
     }
 }
@@ -838,8 +837,7 @@ enableshfuncnode(HashNode hn, UNUSED(int flags))
     if (!strncmp(shf->nam, "TRAP", 4)) {
 	int signum = getsignum(shf->nam + 4);
 	if (signum != -1) {
-	    settrap(signum, shf->funcdef);
-	    sigtrapped[signum] |= ZSIG_FUNC;
+	    settrap(signum, NULL, ZSIG_FUNC);
 	}
     }
 }
diff --git a/Src/parse.c b/Src/parse.c
index d0f108333..9a4a95bec 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -2105,6 +2105,16 @@ yyerror(int noerr)
 	errflag = 1;
 }
 
+/*
+ * Duplicate a programme list, on the heap if heap is 1, else
+ * in permanent storage.
+ *
+ * Be careful in case p is the Eprog for a function which will
+ * later be autoloaded.  The shf element of the returned Eprog
+ * must be set appropriately by the caller.  (Normally we create
+ * the Eprog in this case by using mkautofn.)
+ */
+
 /**/
 mod_export Eprog
 dupeprog(Eprog p, int heap)
diff --git a/Src/signals.c b/Src/signals.c
index 53520aa4a..d1e6f22fe 100644
--- a/Src/signals.c
+++ b/Src/signals.c
@@ -36,10 +36,19 @@
 /**/
 mod_export int sigtrapped[VSIGCOUNT];
 
-/* trap functions for each signal */
+/*
+ * Trap programme lists for each signal.
+ *
+ * If (sigtrapped[sig] & ZSIG_FUNC) is set, this isn't used.
+ * The corresponding shell function is used instead.
+ *
+ * Otherwise, if sigtrapped[sig] is not zero, this is NULL when a signal
+ * is to be ignored, and if not NULL contains the programme list to be
+ * eval'd.
+ */
 
 /**/
-mod_export Eprog sigfuncs[VSIGCOUNT];
+mod_export Eprog siglists[VSIGCOUNT];
 
 /* Total count of trapped signals */
 
@@ -682,7 +691,7 @@ static int dontsavetrap;
 
 /*
  * Save the current trap by copying it.  This does nothing to
- * the existing value of sigtrapped or sigfuncs.
+ * the existing value of sigtrapped or siglists.
  */
 
 static void
@@ -704,15 +713,18 @@ dosavetrap(int sig, int level)
 	    newshf->nam = ztrdup(shf->nam);
 	    newshf->flags = shf->flags;
 	    newshf->funcdef = dupeprog(shf->funcdef, 0);
+	    if (shf->flags & PM_UNDEFINED)
+		newshf->funcdef->shf = newshf;
 	}
 #ifdef DEBUG
 	else dputs("BUG: no function present with function trap flag set.");
 #endif
+	DPUTS(siglists[sig], "BUG: function signal has eval list, too.");
 	st->list = newshf;
     } else if (sigtrapped[sig]) {
-	st->list = sigfuncs[sig] ? dupeprog(sigfuncs[sig], 0) : NULL;
+	st->list = siglists[sig] ? dupeprog(siglists[sig], 0) : NULL;
     } else {
-	DPUTS(sigfuncs[sig], "BUG: sigfuncs not null for untrapped signal");
+	DPUTS(siglists[sig], "BUG: siglists not null for untrapped signal");
 	st->list = NULL;
     }
     if (!savetraps)
@@ -723,9 +735,24 @@ dosavetrap(int sig, int level)
     zinsertlinknode(savetraps, (LinkNode)savetraps, st);
 }
 
+
+/*
+ * Set a trap:  note this does not handle manipulation of
+ * the function table for TRAPNAL functions.
+ *
+ * sig is the signal number.
+ *
+ * l is the list to be eval'd for a trap defined with the "trap"
+ * builtin and should be NULL for a function trap.
+ *
+ * flags includes any additional flags to be or'd into sigtrapped[sig],
+ * in particular ZSIG_FUNC; the basic flags will be assigned within
+ * settrap.
+ */
+
 /**/
 mod_export int
-settrap(int sig, Eprog l)
+settrap(int sig, Eprog l, int flags)
 {
     if (sig == -1)
         return 1;
@@ -741,8 +768,10 @@ settrap(int sig, Eprog l)
     queue_signals();
     unsettrap(sig);
 
-    sigfuncs[sig] = l;
-    if (empty_eprog(l)) {
+    DPUTS((flags & ZSIG_FUNC) && l,
+	  "BUG: trap function has passed eval list, too");
+    siglists[sig] = l;
+    if (!(flags & ZSIG_FUNC) && empty_eprog(l)) {
 	sigtrapped[sig] = ZSIG_IGNORED;
         if (sig && sig <= SIGCOUNT &&
 #ifdef SIGWINCH
@@ -765,7 +794,7 @@ settrap(int sig, Eprog l)
      * sigtrapped[sig] is zero or not, i.e. a test without a mask
      * works just the same.
      */
-    sigtrapped[sig] |= (locallevel << ZSIG_SHIFT);
+    sigtrapped[sig] |= (locallevel << ZSIG_SHIFT) | flags;
     unqueue_signals();
     return 0;
 }
@@ -829,7 +858,7 @@ removetrap(int sig)
     /*
      * At this point we free the appropriate structs.  If we don't
      * want that to happen then either the function should already have been
-     * removed from shfunctab, or the entry in sigfuncs should have been set
+     * removed from shfunctab, or the entry in siglists should have been set
      * to NULL.  This is no longer necessary for saving traps as that
      * copies the structures, so here we are remove the originals.
      * That causes a little inefficiency, but a good deal more reliability.
@@ -841,15 +870,14 @@ removetrap(int sig)
 	 * As in dosavetrap(), don't call removeshfuncnode() because
 	 * that calls back into unsettrap();
 	 */
-	sigfuncs[sig] = NULL;
 	if (node)
 	    removehashnode(shfunctab, node->nam);
 	unqueue_signals();
 
 	return node;
-    } else if (sigfuncs[sig]) {
-	freeeprog(sigfuncs[sig]);
-	sigfuncs[sig] = NULL;
+    } else if (siglists[sig]) {
+	freeeprog(siglists[sig]);
+	siglists[sig] = NULL;
     }
     unqueue_signals();
 
@@ -894,9 +922,9 @@ endtrapscope(void)
 	if (exittr & ZSIG_FUNC) {
 	    exitfn = removehashnode(shfunctab, "TRAPEXIT");
 	} else {
-	    exitfn = sigfuncs[SIGEXIT];
+	    exitfn = siglists[SIGEXIT];
+	    siglists[SIGEXIT] = NULL;
 	}
-	sigfuncs[SIGEXIT] = NULL;
 	if (sigtrapped[SIGEXIT] & ZSIG_TRAPPED)
 	    nsigtrapped--;
 	sigtrapped[SIGEXIT] = 0;
@@ -911,11 +939,12 @@ endtrapscope(void)
 	    remnode(savetraps, ln);
 
 	    if (st->flags && (st->list != NULL)) {
-		Eprog prog = (st->flags & ZSIG_FUNC) ?
-		    ((Shfunc) st->list)->funcdef : (Eprog) st->list;
 		/* prevent settrap from saving this */
 		dontsavetrap++;
-		settrap(sig, prog);
+		if (st->flags & ZSIG_FUNC)
+		    settrap(sig, NULL, ZSIG_FUNC);
+		else
+		    settrap(sig, (Eprog) st->list, 0);
 		dontsavetrap--;
 		/*
 		 * counting of nsigtrapped should presumably be handled
@@ -946,7 +975,7 @@ endtrapscope(void)
 }
 
 /* Execute a trap function for a given signal, possibly
- * with non-standard sigtrapped & sigfuncs values
+ * with non-standard sigtrapped & siglists values
  */
 
 /* Are we already executing a trap? */
@@ -1097,9 +1126,24 @@ dotrapargs(int sig, int *sigtr, void *sigfn)
 void
 dotrap(int sig)
 {
+    Eprog funcprog;
+
+    if (sigtrapped[sig] & ZSIG_FUNC) {
+	HashNode hn = gettrapnode(sig, 0);
+	if (hn)
+	    funcprog = ((Shfunc)hn)->funcdef;
+	else {
+#ifdef DEBUG
+	    dputs("BUG: running function trap which has escaped.");
+#endif
+	    funcprog = NULL;
+	}
+    } else
+	funcprog = siglists[sig];
+
     /* Copied from dotrapargs(). */
-    if ((sigtrapped[sig] & ZSIG_IGNORED) || !sigfuncs[sig] || errflag)
+    if ((sigtrapped[sig] & ZSIG_IGNORED) || !funcprog || errflag)
 	return;
 
-    dotrapargs(sig, sigtrapped+sig, sigfuncs[sig]);
+    dotrapargs(sig, sigtrapped+sig, funcprog);
 }
diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst
index 3ef399efd..f75c47c4d 100644
--- a/Test/C03traps.ztst
+++ b/Test/C03traps.ztst
@@ -183,17 +183,21 @@
   }
   testunset
   f
-1: more sophisticated error trapping
+  print status $?
+  unfunction TRAPZERR
+0: more sophisticated error trapping
 >f
 >ERR-or!
 >f
 >t
+>f
 >t
 >f
 >ERR-or!
 >testunset
 >f
 >ERR-or!
+>status 1
 
   f() {
     setopt localtraps
@@ -216,6 +220,32 @@
   fn
 1: ksh-style EXIT traps preserve return value
 
-  inner() { trap 'return 3' EXIT; return 2: }
+  inner() { trap 'return 3' EXIT; return 2; }
   outer() { inner; return 1; }
+  outer
 3: ksh-style EXIT traps can force return status of enclosing function
+
+# Autoloaded traps are horrid, but unfortunately people expect
+# them to work if we support them.
+  echo "print Running exit trap" >TRAPEXIT
+  $ZTST_testdir/../Src/zsh -fc '
+    fpath=(. $fpath)
+    autoload TRAPEXIT
+    print "Exiting, attempt 1"
+    exit
+    print "What?"
+  '
+  $ZTST_testdir/../Src/zsh -fc '
+    fpath=(. $fpath)
+    autoload TRAPEXIT;
+    fn() { print Some function }
+    fn
+    print "Exiting, attempt 2"
+    exit
+  '
+0: autoloaded TRAPEXIT (exit status > 128 indicates an old bug is back)
+>Exiting, attempt 1
+>Running exit trap
+>Some function
+>Exiting, attempt 2
+>Running exit trap