Re: Wordcode functions with empty bodies

[Peter Stephenson <pws@xxxxxxxxxxxxxxxxxxxxxxxx>]

"Bart Schaefer" wrote:
> However, either Clint's 11839 or my 11857 is still needed to prevent the
> crash reported in 11837, so there's still something fishy going on with
> saving and restoring the EXIT trap.  Or else the lack of 11857 _is_ the
> fishy thing that's going on, but I don't follow how the ZSIG_FUNC bit is
> set when there is no function.

I found another *two* bugs (that's five today).  Apart from the bug which
was causing this --- premature setting of sigtrapped[SIGEXIT], causing that
to be set when resetting the old trap so that there as a flag but no
function --- there was another one:

% TRAPEXIT() { true; }
% accept-line() { zle .accept-line; }
% zle -N accept-line
% trap
TRAPEXIT () {
        true
}
% trap
# with the first bug, printed
BUG: I did not find any trap functions!
# with the second bug printed nothing

--- because dosavetrap() was saving an extra empty TRAPEXIT at locallevel
0, which got restored as no trap, the reason being TRAPEXIT is handled
specially to save an empty trap even if LOCALTRAPS is not set because it
gets saved and restored for every single function scope.  However, it
shouldn't get saved when *leaving* the function scope, which was what was
happening here.

Anybody else is welcome to rewrite this from scratch, taking account of all
the special cases.

I've added some extra DPUTS's, but they still don't catch all possible
problems.

Index: Src/signals.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/signals.c,v
retrieving revision 1.9
diff -u -r1.9 signals.c
--- Src/signals.c	2000/06/14 15:14:49	1.9
+++ Src/signals.c	2000/06/14 21:51:18
@@ -642,6 +642,7 @@
 };
 
 static LinkList savetraps;
+static int dontsavetrap;
 
 /*
  * Save the current trap by copying it.  This does nothing to
@@ -670,6 +671,10 @@
 	    newshf->flags = shf->flags;
 	    newshf->funcdef = dupeprog(shf->funcdef, 0);
 	}
+#ifdef DEBUG
+	else dputs("BUG: no function present with function trap flag set.");
+#endif
+	    
 	st->list = newshf;
     } else if (sigtrapped[sig]) {
 	st->list = sigfuncs[sig] ? dupeprog(sigfuncs[sig], 0) : NULL;
@@ -754,7 +759,8 @@
      * one, to aid in removing this one.  However, if there's
      * already one at the current locallevel we just overwrite it.
      */
-    if ((isset(LOCALTRAPS) || sig == SIGEXIT) && locallevel &&
+    if (!dontsavetrap && (isset(LOCALTRAPS) || sig == SIGEXIT) &&
+	locallevel &&
 	(!trapped || locallevel > (sigtrapped[sig] >> ZSIG_SHIFT)))
 	dosavetrap(sig, locallevel);
 
@@ -854,21 +860,19 @@
 
 	    remnode(savetraps, ln);
 
-	    if (sigtrapped[sig])
-		unsettrap(sig);
-	    sigtrapped[sig] = st->flags;
 	    if (st->flags && (st->list != NULL)) {
 		Eprog prog = (st->flags & ZSIG_FUNC) ?
 		    ((Shfunc) st->list)->funcdef : (Eprog) st->list;
 		/* prevent settrap from saving this */
-		int oldlt = opts[LOCALTRAPS];
-		opts[LOCALTRAPS] = 0;
+		dontsavetrap++;
 		settrap(sig, prog);
-		opts[LOCALTRAPS] = oldlt;
+		dontsavetrap--;
 		if ((sigtrapped[sig] = st->flags) & ZSIG_FUNC)
 		    shfunctab->addnode(shfunctab, ((Shfunc)st->list)->nam,
 				       (Shfunc) st->list);
-	    }
+	    } else if (sigtrapped[sig])
+		unsettrap(sig);
+
 	    zfree(st, sizeof(*st));
 	}
     }
@@ -881,6 +885,8 @@
 	else
 	    freeeprog(exitfn);
     }
+    DPUTS(!locallevel && savetraps && firstnode(savetraps),
+	  "BUG: still saved traps outside all function scope");
 }
 
 /* Execute a trap function for a given signal, possibly

-- 
Peter Stephenson <pws@xxxxxxxxxxxxxxxxxxxxxxxx>
Work: pws@xxxxxxxxxxxxxxxxxxxxxxxxx
Web: http://www.pwstephenson.fsnet.co.uk