From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19759 invoked by alias); 2 Oct 2014 15:06:04 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 33325 Received: (qmail 21458 invoked from network); 2 Oct 2014 15:06:01 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_HI, SPF_HELO_PASS autolearn=ham version=3.3.2 X-AuditID: cbfec7f5-b7f776d000003e54-a2-542d677aa215 Date: Thu, 02 Oct 2014 15:55:53 +0100 From: Peter Stephenson To: Zsh hackers list Subject: Re: PATCH: functions with redirections Message-id: <20141002155553.310f566b@pwslap01u.europe.root.pri> In-reply-to: <20141002093509.789ff912@pwslap01u.europe.root.pri> References: <20140929205236.2eb5e622@pws-pc.ntlworld.com> <20141001201705.23ee198d@pws-pc.ntlworld.com> <141001210741.ZM6790@torch.brasslantern.com> <20141002093509.789ff912@pwslap01u.europe.root.pri> Organization: Samsung Cambridge Solution Centre X-Mailer: Claws Mail 3.7.9 (GTK+ 2.22.0; i386-redhat-linux-gnu) MIME-version: 1.0 Content-type: text/plain; charset=US-ASCII Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFupjluLIzCtJLcpLzFFi42I5/e/4Zd2qdN0Qg2kXlSwONj9kcmD0WHXw A1MAYxSXTUpqTmZZapG+XQJXRtuMWcwFl40q7tyZydzAeF+ti5GTQ0LAROL2iWfsELaYxIV7 69m6GLk4hASWMkq8ePKTEcJZziTxZOVuRpAqFgFVid97V4N1sAkYSkzdNBssLiKgJbHj5Ekm EFtYQF/iz8aTYHFeAXuJtU3bwOKcAg4Su260Qg29wyTx6MhyNpAEP1DD1b+fmCDOsJeYeeUM VLOgxI/J91hAbGagBZu3NbFC2PISm9e8ZZ7AKDALSdksJGWzkJQtYGRexSiaWppcUJyUnmuk V5yYW1yal66XnJ+7iREShl93MC49ZnWIUYCDUYmH90CTTogQa2JZcWXuIUYJDmYlEV69KN0Q Id6UxMqq1KL8+KLSnNTiQ4xMHJxSDYzsJs8ivq2Y+/KLo3u2j9VMe//a9It/dnxpn/irOyc+ XPHXDqmsTQ9Z/bm0Wi/cu/m8zJLF6IYfn1e/w4F3YQ8Ddob0tjzlDn7Re9svMzT1r75y7O/u hCcbpcUkDO5t+WKb2Dmbs/XEqTOWn8NNZz7jXBIgvrpzx9Lg3/ov2mOb+wNyZ0ZrOyuxFGck GmoxFxUnAgDhtQK9IQIAAA== On Thu, 02 Oct 2014 09:35:09 +0100 Peter Stephenson wrote: > > torch% cat /tmp/foo > > foo() { echo foo } >&3 > > torch% zcompile -k /tmp/foo > > torch% autoload foo > > torch% FPATH=/tmp foo > > foo > > > > Oops, the redirection didn't get applied. > > Not sure how that could happen --- there must be some kludge to run the > function the first time. That's correct: we use a very simple execution environment to execute a function that's just been autoloaded. This isn't actually related to dumps --- we have the same problem with a normal ksh autoload. The fix is to promote the point where we actually load (but don't execute) the function. We do this for all autoloads to find out if there are redirections. Having done that we are in a position to extract the redirections as usual. The normal zsh case wouldn't have redirections at that point because the syntax doesn't allow it. The kludged zsh case with an explicit call to the function (the one I originally tested) works because in that case all the normal function definition code is executed; it's not a special case as far as redirection code is concerned. We don't currently set errflag when failing to load a function (unless it happens deeper than I've looked), so we still don't. There'a at least some sign this is deliberate (warnings rather than errors). Moving the load up might have some very minor side effect on some error case where previously we wouldn't have defined the function but now do. pws diff --git a/Src/exec.c b/Src/exec.c index 10f71da..a5452e5 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -2789,13 +2789,6 @@ execcmd(Estate state, int input, int output, int how, int last1) errflag = 1; } - if (errflag) { - lastval = 1; - if (oautocont >= 0) - opts[AUTOCONTINUE] = oautocont; - return; - } - if (type == WC_FUNCDEF) { /* * The first word of a function definition is a list of @@ -2809,8 +2802,24 @@ execcmd(Estate state, int input, int output, int how, int last1) /* Nonymous, don't do redirections here */ redir = NULL; } - } else if (is_shfunc) { - Shfunc shf = (Shfunc)hn; + } else if (is_shfunc || type == WC_AUTOFN) { + Shfunc shf; + if (is_shfunc) + shf = (Shfunc)hn; + else { + shf = loadautofn(state->prog->shf, 1, 0); + if (shf) + state->prog->shf = shf; + else { + /* + * This doesn't set errflag, so just return now. + */ + lastval = 1; + if (oautocont >= 0) + opts[AUTOCONTINUE] = oautocont; + return; + } + } /* * A function definition may have a list of additional * redirections to apply, so retrieve it. @@ -2832,6 +2841,13 @@ execcmd(Estate state, int input, int output, int how, int last1) } } + if (errflag) { + lastval = 1; + if (oautocont >= 0) + opts[AUTOCONTINUE] = oautocont; + return; + } + if (type == WC_SIMPLE && !nullexec) { char *s; char trycd = (isset(AUTOCD) && isset(SHINSTDIN) && @@ -3306,10 +3322,18 @@ execcmd(Estate state, int input, int output, int how, int last1) redir_prog = NULL; lastval = execfuncdef(state, redir_prog); - } else if (type >= WC_CURSH) { + } + else if (type >= WC_CURSH) { if (last1 == 1) do_exec = 1; - lastval = (execfuncs[type - WC_CURSH])(state, do_exec); + if (type == WC_AUTOFN) { + /* + * We pre-loaded this to get any redirs. + * So we execuate a simplified function here. + */ + lastval = execautofn_basic(state, do_exec); + } else + lastval = (execfuncs[type - WC_CURSH])(state, do_exec); } else if (is_builtin || is_shfunc) { LinkList restorelist = 0, removelist = 0; /* builtin or shell function */ @@ -4540,21 +4564,28 @@ execshfunc(Shfunc shf, LinkList args) deletefilelist(last_file_list, 0); } -/* Function to execute the special type of command that represents an * - * autoloaded shell function. The command structure tells us which * - * function it is. This function is actually called as part of the * - * execution of the autoloaded function itself, so when the function * - * has been autoloaded, its list is just run with no frills. */ +/* + * Function to execute the special type of command that represents an + * autoloaded shell function. The command structure tells us which + * function it is. This function is actually called as part of the + * execution of the autoloaded function itself, so when the function + * has been autoloaded, its list is just run with no frills. + * + * There are two cases because if we are doing all-singing, all-dancing + * non-simple code we load the shell function early in execcmd() (the + * action also present in the non-basic version) to check if + * there are redirections that need to be handled at that point. + * Then we call execautofn_basic() to do the rest. + */ /**/ static int -execautofn(Estate state, UNUSED(int do_exec)) +execautofn_basic(Estate state, UNUSED(int do_exec)) { Shfunc shf; char *oldscriptname, *oldscriptfilename; - if (!(shf = loadautofn(state->prog->shf, 1, 0))) - return 1; + shf = state->prog->shf; /* * Probably we didn't know the filename where this function was @@ -4574,6 +4605,19 @@ execautofn(Estate state, UNUSED(int do_exec)) } /**/ +static int +execautofn(Estate state, UNUSED(int do_exec)) +{ + Shfunc shf; + + if (!(shf = loadautofn(state->prog->shf, 1, 0))) + return 1; + + state->prog->shf = shf; + return execautofn_basic(state, 0); +} + +/**/ Shfunc loadautofn(Shfunc shf, int fksh, int autol) { diff --git a/Test/A05execution.ztst b/Test/A05execution.ztst index df6d7bc..8d256ff 100644 --- a/Test/A05execution.ztst +++ b/Test/A05execution.ztst @@ -216,3 +216,16 @@ F:This similar test was triggering a reproducible failure with pipestatus. >done F:This test checks for a file descriptor leak that could cause the left F:side of a pipe to block on write after the right side has exited + + print "autoload_redir() { print Autoloaded ksh style; } >autoload.log" >autoload_redir + autoload -Uk autoload_redir + autoload_redir + print No output yet + cat autoload.log + functions autoload_redir +0: +>No output yet +>Autoloaded ksh style +>autoload_redir () { +> print Autoloaded ksh style +>} > autoload.log