11026: fix for unfunction of trap with localtraps bug

5 files changed, 49 insertions, 16 deletions

diff --git a/ChangeLog b/ChangeLog
index 5c023c022..210ae35cc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2000-04-30  Peter Stephenson  <pws@pwstephenson.fsnet.co.uk>
+
+	* 11026: Src/builtin.c, Src/hashtable.c, Src/signals.c,
+	Test/08traps.ztst: fix unfunction problem for localtraps by
+	always copying traps to be saved, hence decoupling saved and actual
+	traps.
+
 2000-04-30  Bart Schaefer  <schaefer@zsh.org>
 
 	* 11024: Completion/Commands/_complete_debug, Doc/Zsh/compsys.yo:
diff --git a/Src/builtin.c b/Src/builtin.c
index 6d9fcd493..79648561e 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -2592,14 +2592,12 @@ bin_unhash(char *name, char **argv, char *ops, int func)
 
     /* Take arguments literally -- do not glob */
     for (; *argv; argv++) {
-	int ne = noerrs;
 	if ((hn = ht->removenode(ht, *argv))) {
 	    ht->freenode(hn);
 	} else {
 	    zwarnnam(name, "no such hash table element: %s", *argv, 0);
-	    returnval |= !noerrs;
+	    returnval = 1;
 	}
-	noerrs = ne;
     }
     return returnval;
 }
diff --git a/Src/hashtable.c b/Src/hashtable.c
index 3595c2721..19f23086b 100644
--- a/Src/hashtable.c
+++ b/Src/hashtable.c
@@ -787,8 +787,9 @@ static HashNode
 removeshfuncnode(HashTable ht, char *nam)
 {
     HashNode hn;
+    int signum;
 
-    if (!strncmp(nam, "TRAP", 4))
+    if (!strncmp(nam, "TRAP", 4) && (signum = getsignum(nam +4)) != -1)
 	hn = removetrap(getsignum(nam + 4));
     else
 	hn = removehashnode(shfunctab, nam);
diff --git a/Src/signals.c b/Src/signals.c
index 891ea1143..e93030002 100644
--- a/Src/signals.c
+++ b/Src/signals.c
@@ -644,7 +644,8 @@ struct savetrap {
 static LinkList savetraps;
 
 /*
- * Save the current trap and unset it.
+ * Save the current trap by copying it.  This does nothing to
+ * the existing value of sigtrapped or sigfuncs.
  */
 
 static void
@@ -660,15 +661,18 @@ dosavetrap(int sig, int level)
 	 * the new one yet.
 	 */
 	char func[20];
+	Shfunc shf, newshf = NULL;
 	sprintf(func, "TRAP%s", sigs[sig]);
-	/* We call removehashnode() directly because otherwise
-	 * removeshfuncnode() would be called which in turn would
-	 * call us again so that we would end up with a NULL pointer
-	 * instead of the list for the trap. */
-	st->list = removehashnode(shfunctab, func);
+	if ((shf = (Shfunc)shfunctab->getnode2(shfunctab, func))) {
+	    /* Copy the node for saving */
+	    newshf = (Shfunc) zalloc(sizeof(*newshf));
+	    newshf->nam = ztrdup(shf->nam);
+	    newshf->flags = shf->flags;
+	    newshf->funcdef = dupeprog(shf->funcdef, 0);
+	}
+	st->list = newshf;
     } else {
-	st->list = sigfuncs[sig];
-	sigfuncs[sig] = NULL;
+	st->list = sigfuncs[sig] ? dupeprog(sigfuncs[sig], 0) : NULL;
     }
     noerrs = !!st->list;
     if (!savetraps)
@@ -774,10 +778,11 @@ removetrap(int sig)
 
     /*
      * At this point we free the appropriate structs.  If we don't
-     * want that to happen (e.g. we are saving the trap), then
-     * either the function should already have been removed from shfunctab,
-     * or the entry in sigfuncs should have been set to NULL, and then
-     * we're laughing, in a sort of vague virtual sense.
+     * want that to happen then either the function should already have been
+     * removed from shfunctab, or the entry in sigfuncs 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.
      */
     if (trapped & ZSIG_FUNC) {
 	char func[20];
diff --git a/Test/08traps.ztst b/Test/08traps.ztst
index 10eb48ba9..5ede57fdb 100644
--- a/Test/08traps.ztst
+++ b/Test/08traps.ztst
@@ -126,3 +126,25 @@
   sleep 2
 0: Nested `trap ... TERM', triggered on outer loop
 >TERM1
+
+  TRAPZERR() { print error activated; }
+  fn() { print start of fn; false; print end of fn; }
+  fn
+  fn() {
+    setopt localoptions localtraps
+    unfunction TRAPZERR
+    print start of fn
+    false
+    print end of fn
+  }
+  fn
+  unfunction TRAPZERR
+  print finish
+0: basic localtraps handling
+>start of fn
+>error activated
+>end of fn
+>start of fn
+>end of fn
+>finish
+