* PATCH: Re: localtraps causing core dump in 3.1.6+ [not found] ` <1000429044220.ZM30544@candle.brasslantern.com> @ 2000-04-29 6:23 ` Bart Schaefer 2000-04-30 14:29 ` Peter Stephenson 0 siblings, 1 reply; 3+ messages in thread From: Bart Schaefer @ 2000-04-29 6:23 UTC (permalink / raw) To: zsh-workers On Apr 29, 4:42am, Bart Schaefer wrote: } Subject: Re: localtraps causing core dump in 3.1.6+ } } } I recently started doing `setopt localtraps && unset -f TRAPZERR' } } because of some changes that took place for 3.1.7-pre-1. } } } } However that led me to try the above, which dumps core } } on both 3.1.7-pre-1 and 3.1.6 when changing directory. } } There is a bug in unsetting TRAPxxx (as opposed to resetting them to } something else) when localtraps is set. You might try `trap - ZERR' or } `disable -f TRAPZERR' instead for now. This is a bit ugly. Both unsettrap() and removeshfuncnode() want to call removenode(). removeshfuncnode() returns the hashnode, and a warning is generated if it's NULL, so it has to get the node before unsettrap() removes it. But unsettrap() subsequently calls freenode(), so it doesn't work to grab the node with gethashnode2() before calling unsettrap(). The standard trick is `sigtrapped[sig] &= ~ZSIG_FUNC;' before calling unsettrap(), but if that's done then dosavetrap() doesn't save the shell function body and localtraps is foiled. In the current state, dosavetrap() thinks there's a function and tries to save the body, but it can't because removeshfuncnode() has already removed it from the table. Thus the core dump when endtrapscope() tries to restore the nonexistent body. If we let dosavetrap() do its work, then *that* removes the node to save it, so again we can't pass it back up through removeshfuncnode() to stop the error message. Which is just as well, because if it did make it up to bin_unhash(), it'd get freed, when the whole point is to save it. Gaah. If anyone can think of a better fix for this than diddling with `noerrs', don't be shy. By the way, the more I see of the "compiler-like error messages" from 8779, the more I hate them. Please let's back out that change, or at least put the space back in after each colon. Oh, yeah: One side-effect of this patch is that `unfunction TRAPxxx' has the same effect as `trap - xxx', which is symmetrical with `trap - xxx' having the effect of `unfunction TRAPxxx'. The "no such ..." warning is suppressed when such a trap is removed, but that can be changed by moving `noerrs = !!st->list;' up four lines to above `} else {'. Index: Src/builtin.c =================================================================== @@ -2592,12 +2592,14 @@ /* 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 = 1; + returnval |= !noerrs; } + noerrs = ne; } return returnval; } Index: Src/hashtable.c =================================================================== @@ -788,12 +788,12 @@ { HashNode hn; - if ((hn = removehashnode(shfunctab, nam))) { - if (!strncmp(hn->nam, "TRAP", 4)) - unsettrap(getsignum(hn->nam + 4)); - return hn; - } else - return NULL; + if (!strncmp(nam, "TRAP", 4)) + hn = removetrap(getsignum(nam + 4)); + else + hn = removehashnode(shfunctab, nam); + + return hn; } /* Disable an entry in the shell function hash table. * @@ -822,11 +822,10 @@ enableshfuncnode(HashNode hn, int flags) { Shfunc shf = (Shfunc) hn; - int signum; shf->flags &= ~DISABLED; if (!strncmp(shf->nam, "TRAP", 4)) { - signum = getsignum(shf->nam + 4); + int signum = getsignum(shf->nam + 4); if (signum != -1) { settrap(signum, shf->funcdef); sigtrapped[signum] |= ZSIG_FUNC; Index: Src/signals.c =================================================================== @@ -670,6 +670,7 @@ st->list = sigfuncs[sig]; sigfuncs[sig] = NULL; } + noerrs = !!st->list; if (!savetraps) savetraps = znewlinklist(); /* @@ -726,11 +727,22 @@ void unsettrap(int sig) { + int ne = noerrs; + HashNode hn = removetrap(sig); + noerrs = ne; + if (hn) + shfunctab->freenode(hn); +} + +/**/ +HashNode +removetrap(int sig) +{ int trapped; if (sig == -1 || (jobbing && (sig == SIGTTOU || sig == SIGTSTP || sig == SIGTTIN))) - return; + return NULL; trapped = sigtrapped[sig]; /* @@ -743,7 +755,7 @@ dosavetrap(sig, locallevel); if (!trapped) - return; + return NULL; sigtrapped[sig] = 0; if (sig == SIGINT && interact) { @@ -769,19 +781,19 @@ */ if (trapped & ZSIG_FUNC) { char func[20]; - HashNode hn; sprintf(func, "TRAP%s", sigs[sig]); /* * As in dosavetrap(), don't call removeshfuncnode() because * that calls back into unsettrap(); */ - if ((hn = removehashnode(shfunctab, func))) - shfunctab->freenode(hn); + return removehashnode(shfunctab, func); } else if (sigfuncs[sig]) { freeeprog(sigfuncs[sig]); sigfuncs[sig] = NULL; } + + return NULL; } /**/ -- Bart Schaefer Brass Lantern Enterprises http://www.well.com/user/barts http://www.brasslantern.com ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: PATCH: Re: localtraps causing core dump in 3.1.6+ 2000-04-29 6:23 ` PATCH: Re: localtraps causing core dump in 3.1.6+ Bart Schaefer @ 2000-04-30 14:29 ` Peter Stephenson 2000-04-30 17:03 ` Bart Schaefer 0 siblings, 1 reply; 3+ messages in thread From: Peter Stephenson @ 2000-04-30 14:29 UTC (permalink / raw) To: Zsh hackers list "Bart Schaefer" wrote: > On Apr 29, 4:42am, Bart Schaefer wrote: > } Subject: Re: localtraps causing core dump in 3.1.6+ > } > } } I recently started doing `setopt localtraps && unset -f TRAPZERR' > } } because of some changes that took place for 3.1.7-pre-1. > } } > } } However that led me to try the above, which dumps core > } } on both 3.1.7-pre-1 and 3.1.6 when changing directory. > > This is a bit ugly. The following is less ugly, although a bit more inefficient, so I've backed out Bart's noerr changes. dosavetrap() now always copies the trap its saving. In fact, we always call it when we're unsetting a trap, so in most cases (this bug is the exception) we don't need to do that. However, it's much more logical and fixes the bug neatly, since we are free to return the function node without worrying where it's come from. Since the changes in the code structure, copying a chunk of shell code is now very much more efficient (thanks, Sven). I've merged it round Bart's changes, which were neater than mine in some places, and added a minimal localtraps test. It probably should be pointed out that `trap - ZERR' is a better thing to use in general, since it's more standard and picks up both sorts of trap. Index: Src/builtin.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v retrieving revision 1.9 diff -u -r1.9 builtin.c --- Src/builtin.c 2000/04/29 06:33:48 1.9 +++ Src/builtin.c 2000/04/30 14:23:02 @@ -2592,14 +2592,12 @@ /* 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; } Index: Src/hashtable.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/hashtable.c,v retrieving revision 1.4 diff -u -r1.4 hashtable.c --- Src/hashtable.c 2000/04/29 06:33:48 1.4 +++ Src/hashtable.c 2000/04/30 14:23:04 @@ -787,8 +787,9 @@ 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); Index: Src/signals.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/signals.c,v retrieving revision 1.3 diff -u -r1.3 signals.c --- Src/signals.c 2000/04/29 06:33:48 1.3 +++ Src/signals.c 2000/04/30 14:23:06 @@ -644,7 +644,8 @@ 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 @@ * 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 @@ /* * 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]; Index: Test/08traps.ztst =================================================================== RCS file: /cvsroot/zsh/zsh/Test/08traps.ztst,v retrieving revision 1.1.1.1 diff -u -r1.1.1.1 08traps.ztst --- Test/08traps.ztst 2000/01/07 22:31:15 1.1.1.1 +++ Test/08traps.ztst 2000/04/30 14:23:06 @@ -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 + -- Peter Stephenson <pws@pwstephenson.fsnet.co.uk> Work: pws@CambridgeSiliconRadio.com Web: http://www.pwstephenson.fsnet.co.uk ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: PATCH: Re: localtraps causing core dump in 3.1.6+ 2000-04-30 14:29 ` Peter Stephenson @ 2000-04-30 17:03 ` Bart Schaefer 0 siblings, 0 replies; 3+ messages in thread From: Bart Schaefer @ 2000-04-30 17:03 UTC (permalink / raw) To: Zsh hackers list On Apr 30, 3:29pm, Peter Stephenson wrote: } Subject: Re: PATCH: Re: localtraps causing core dump in 3.1.6+ } } dosavetrap() now always copies the trap its saving. } } I've merged it round Bart's changes, which were neater than mine in some } places, and added a minimal localtraps test. You missed a few tidbits. } - if (!strncmp(nam, "TRAP", 4)) } + if (!strncmp(nam, "TRAP", 4) && (signum = getsignum(nam +4)) != -1) } hn = removetrap(getsignum(nam + 4)); --- zsh-forge/current/Src/hashtable.c Sun Apr 30 09:56:30 2000 +++ Src/hashtable.c Sun Apr 30 09:58:25 2000 @@ -789,8 +789,8 @@ HashNode hn; int signum; - if (!strncmp(nam, "TRAP", 4) && (signum = getsignum(nam +4)) != -1) - hn = removetrap(getsignum(nam + 4)); + if (!strncmp(nam, "TRAP", 4) && (signum = getsignum(nam + 4)) != -1) + hn = removetrap(signum); else hn = removehashnode(shfunctab, nam); --- zsh-forge/current/Src/signals.c Sun Apr 30 09:56:30 2000 +++ Src/signals.c Sun Apr 30 10:01:28 2000 @@ -674,7 +674,6 @@ } else { st->list = sigfuncs[sig] ? dupeprog(sigfuncs[sig], 0) : NULL; } - noerrs = !!st->list; if (!savetraps) savetraps = znewlinklist(); /* @@ -731,9 +730,7 @@ void unsettrap(int sig) { - int ne = noerrs; HashNode hn = removetrap(sig); - noerrs = ne; if (hn) shfunctab->freenode(hn); } -- Bart Schaefer Brass Lantern Enterprises http://www.well.com/user/barts http://www.brasslantern.com ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2000-04-30 17:03 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20000428161546.A8208@tdc134.comm.mot.com> [not found] ` <1000429044220.ZM30544@candle.brasslantern.com> 2000-04-29 6:23 ` PATCH: Re: localtraps causing core dump in 3.1.6+ Bart Schaefer 2000-04-30 14:29 ` Peter Stephenson 2000-04-30 17:03 ` Bart Schaefer
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).