From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from euclid.skiles.gatech.edu (list@euclid.skiles.gatech.edu [130.207.146.50]) by melb.werple.net.au (8.7.5/8.7.3/2) with ESMTP id XAA24068 for ; Mon, 8 Jul 1996 23:02:58 +1000 (EST) Received: (from list@localhost) by euclid.skiles.gatech.edu (8.7.3/8.7.3) id IAA05725; Mon, 8 Jul 1996 08:51:05 -0400 (EDT) Resent-Date: Mon, 8 Jul 1996 08:51:05 -0400 (EDT) Message-Id: <199607081249.OAA09298@hydra.ifh.de> To: zsh-workers@math.gatech.edu (Zsh hackers list) Subject: Re: Bug Report: Env Vars and shell functions In-reply-to: "schaefer@candle.brasslantern.com"'s message of "Mon, 08 Jul 1996 01:58:50 MET." <960708015853.ZM8327@candle.brasslantern.com> Date: Mon, 08 Jul 1996 14:49:18 +0200 From: Peter Stephenson Resent-Message-ID: <"Ea2YQ1.0.NP1.qGGun"@euclid> Resent-From: zsh-workers@math.gatech.edu X-Mailing-List: archive/latest/1573 X-Loop: zsh-workers@math.gatech.edu Precedence: list Resent-Sender: zsh-workers-request@math.gatech.edu schaefer@candle.brasslantern.com wrote: > On Jul 8, 1:49pm, Peter Bray wrote: > } Subject: Bug Report: Env Vars and shell functions > } > } Can others reproduce this bug in zsh-3.0-pre2, where a command > } line environment variable is ignored in other functions called by the > } original functions. > > The local environment seems to get lost on the second and succeeding > passes around the 'for' loop. > > zagzig[52] bar() { echo $1 $BAR } > zagzig[53] foo() { bar } > zagzig[54] BAR=foo foo > foo > zagzig[55] foo() { for x in 1 2 3 ; do bar $x ; done } > zagzig[56] foo > 1 > 2 > 3 > zagzig[57] BAR=foo foo > 1 foo > 2 > 3 The culprit is the save_params()/restore_params() mechanism called from execcmd() to handle temporary settings during builtins, which uses static variables. Only the original author of that can say why it wasn't scoped within execcmd() and therefore if I'm missing something with the following patch. I honestly can't see any problem: I've made sure everything gets restored, including the only abnormal return. However, that can hardly have been the problem being worked around, since in fact the same abnormal return would before have potentially left dreck hanging on removelist/restorelist. I can't see any new scope for leakage. *** Src/exec.c.savpar Mon Jul 8 09:29:41 1996 --- Src/exec.c Mon Jul 8 14:37:10 1996 *************** *** 1561,1571 **** lastval = (func[type - CURSH]) (cmd); } else if (is_builtin || is_shfunc) { /* builtin or shell function */ if (!forked && !assign) { PERMALLOC { ! save_params(cmd); } LASTALLOC; } --- 1561,1572 ---- lastval = (func[type - CURSH]) (cmd); } else if (is_builtin || is_shfunc) { + LinkList restorelist = 0, removelist = 0; /* builtin or shell function */ if (!forked && !assign) { PERMALLOC { ! save_params(cmd, &restorelist, &removelist); } LASTALLOC; } *************** *** 1575,1580 **** --- 1576,1582 ---- */ addvars(cmd->vars, is_shfunc); if (errflag) { + restore_params(restorelist, removelist); lastval = 1; return; } *************** *** 1632,1639 **** exit(lastval); } ! if (!forked && !assign) ! restore_params(); } else { if (cmd->flags & CFLAG_EXEC) { --- 1634,1640 ---- exit(lastval); } ! restore_params(restorelist, removelist); } else { if (cmd->flags & CFLAG_EXEC) { *************** *** 1672,1693 **** fixfds(save); } - /* Link list used to save parameters during * - * execution of shfunc or builtin. */ - static LinkList restorelist; - static LinkList removelist; - - /* Setup the link lists use to save parameters * - * for shfuncs or builtins. */ - - /**/ - void - init_save_params(void) - { - restorelist = newlinklist(); - removelist = newlinklist(); - } - /* Arrange to have variables restored. * * As yet special parameters won't work, nor will * * parameter names that need substituting. * --- 1673,1678 ---- *************** *** 1695,1716 **** /**/ void ! save_params(Cmd cmd) { Param pm; LinkNode node; char *s; for (node = firstnode(cmd->vars); node; incnode(node)) { s = ((Varasg) getdata(node))->name; if ((pm = (Param) paramtab->getnode(paramtab, s))) { if (!(pm->flags & PM_SPECIAL)) { paramtab->removenode(paramtab, s); ! addlinknode(removelist, s); ! addlinknode(restorelist, pm); } } else { ! addlinknode(removelist, s); } } } --- 1680,1704 ---- /**/ void ! save_params(Cmd cmd, LinkList *restore_p, LinkList *remove_p) { Param pm; LinkNode node; char *s; + *restore_p = newlinklist(); + *remove_p = newlinklist(); + for (node = firstnode(cmd->vars); node; incnode(node)) { s = ((Varasg) getdata(node))->name; if ((pm = (Param) paramtab->getnode(paramtab, s))) { if (!(pm->flags & PM_SPECIAL)) { paramtab->removenode(paramtab, s); ! addlinknode(*remove_p, s); ! addlinknode(*restore_p, pm); } } else { ! addlinknode(*remove_p, s); } } } *************** *** 1719,1736 **** /**/ void ! restore_params(void) { Param pm; char *s; ! /* remove temporary parameters */ ! while ((s = (char *) getlinknode(removelist))) ! unsetparam(s); ! ! /* restore saved parameters */ ! while ((pm = (Param) getlinknode(restorelist))) ! paramtab->addnode(paramtab, pm->nam, pm); } /* restore fds after redirecting a builtin */ --- 1707,1730 ---- /**/ void ! restore_params(LinkList restorelist, LinkList removelist) { Param pm; char *s; ! if (removelist) { ! /* remove temporary parameters */ ! while ((s = (char *) getlinknode(removelist))) ! unsetparam(s); ! freelinklist(removelist, 0); ! } ! ! if (restorelist) { ! /* restore saved parameters */ ! while ((pm = (Param) getlinknode(restorelist))) ! paramtab->addnode(paramtab, pm->nam, pm); ! freelinklist(restorelist, 0); ! } } /* restore fds after redirecting a builtin */ *** Src/init.c.savpar Mon Jul 8 09:30:24 1996 --- Src/init.c Mon Jul 8 14:30:47 1996 *************** *** 597,603 **** initkeybindings(); /* initialize key bindings */ compctlsetup(); - init_save_params(); /* init saving params during shfunc or builtin */ #ifdef HAVE_GETRLIMIT for (i = 0; i != RLIM_NLIMITS; i++) --- 597,602 ---- -- Peter Stephenson Tel: +49 33762 77366 WWW: http://www.ifh.de/~pws/ Fax: +49 33762 77330 Deutches Electronen-Synchrotron --- Institut fuer Hochenergiephysik Zeuthen DESY-IfH, 15735 Zeuthen, Germany.