From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17649 invoked by alias); 13 Jan 2017 18:04:39 -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: 40353 Received: (qmail 24453 invoked from network); 13 Jan 2017 18:04:38 -0000 X-Qmail-Scanner-Diagnostics: from mailout4.w1.samsung.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.99.2/21882. spamassassin: 3.4.1. Clear:RC:0(210.118.77.14):SA:0(-8.2/5.0):. Processed in 1.9088 secs); 13 Jan 2017 18:04:38 -0000 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-8.2 required=5.0 tests=RCVD_IN_DNSWL_HI, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=unavailable autolearn_force=no version=3.4.1 X-Envelope-From: p.stephenson@samsung.com X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: none (ns1.primenet.com.au: domain at samsung.com does not designate permitted sender hosts) X-AuditID: cbfec7f2-f790f6d000002555-83-587916ab774e Date: Fri, 13 Jan 2017 18:04:23 +0000 From: Peter Stephenson To: Zsh hackers list Subject: Re: PATH: autoload with explicit path Message-id: <20170113180423.12225591@pwslap01u.europe.root.pri> In-reply-to: <20170111205122.2c47e89f@ntlworld.com> 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+NgFnrLIsWRmVeSWpSXmKPExsWy7djPc7qrxSojDKZMVLI42PyQyYHRY9XB D0wBjFFcNimpOZllqUX6dglcGW8OXWYvOONfcWLFI6YGxlO2XYycHBICJhJ9C98zQthiEhfu rWfrYuTiEBJYyiixu3caK4TTyyRxs/MRM0xH08d7zBCJZYwS9/+egGqZxiSx+N9rJgjnDKPE k8mzWCCcs4wSm569ZQfpZxFQldjxFyTBycEmYCgxddNssO0iAloSO06eZAKxhQX0JH58/QBW wytgL/Hj6X42EJtTwFji0LW5YHF+AX2Jq38/MUHcZC8x88oZRoh6QYkfk++B1TAL6Ehs2/aY HcKWl9i85i3Y3RICzewSrw7MBvqOA8iRldh0AOo3F4ltdw6xQNjCEq+Ob2GHsGUkLk/uhor3 A33W7QsxZwajxOkzO9ggEtYSfbcvMkIs45OYtG06M8R8XomONiGIEg+Jpa+vs0GEHSVe7rSd wKg4C8nVs5BcPQvJ1QsYmVcxiqSWFuempxYb6xUn5haX5qXrJefnbmIEJoLT/45/2sH49YTV IUYBDkYlHt4Jt8sjhFgTy4orcw8xSnAwK4nwlrJVRgjxpiRWVqUW5ccXleakFh9ilOZgURLn 3bPgSriQQHpiSWp2ampBahFMlomDU6qBUf8gs7z5B/Fw1urtruXP0j4tY2rel/zS6oTjQ614 NQYTd25TXQZZ7aMaHF5M/9hTivav4ol/p6v/kT81aDvzPs2ZqxRd9rz9URP71U/h0Okfvlnz j3/9yN7yqMVUWFNXLf3Ckae7tv1w01t/+bVeYnnq6qPejMuYk646HtfdfcNCXarcyO6xEktx RqKhFnNRcSIAzDPKkAADAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrOIsWRmVeSWpSXmKPExsVy+t/xy7pLxCojDFY2MVocbH7I5MDoserg B6YAxig3m4zUxJTUIoXUvOT8lMy8dFul0BA3XQslhbzE3FRbpQhd35AgJYWyxJxSIM/IAA04 OAe4Byvp2yW4Zbw5dJm94Ix/xYkVj5gaGE/ZdjFyckgImEg0fbzHDGGLSVy4t56ti5GLQ0hg CaPE/A8/WCCcGUwS+7Z3skI45xgl5j3/zw7hnGWUWNS/FqyfRUBVYsffWSwgNpuAocTUTbMZ QWwRAS2JHSdPMoHYwgJ6Ej++fgCr4RWwl/jxdD8biM0pYCxx6NpcqHWfmCRmbH3ADpLgF9CX uPr3ExPEgfYSM6+cYYRoFpT4Mfke2CBmoAWbtzWxQtjyEpvXvAU7SEhAXeLG3d3sExiFZyFp mYWkZRaSlgWMzKsYRVJLi3PTc4uN9IoTc4tL89L1kvNzNzECI2nbsZ9bdjB2vQs+xCjAwajE wzvhdnmEEGtiWXFl7iFGCQ5mJRHeUrbKCCHelMTKqtSi/Pii0pzU4kOMpsCQmcgsJZqcD4zy vJJ4QxNDc0tDI2MLC3MjIyVx3qkfroQLCaQnlqRmp6YWpBbB9DFxcEo1MBa/OcGybkPcpnfq P7TaWZf+jPPpjGa3ll10qvHXtbnLbB32yZzqYblcxbLGK1uW64bSQZ72Zx+zX6k+nq9os6m2 pKxzVsIKA3Of+4cWHIn32DhnuoCJ/af7H1d8ed1zJOvTkr0eHa0p7oeX8P4J83o6W7qiaip3 jM5a0/oGa2G/zZzXIyufnVdiKc5INNRiLipOBADfyRtjugIAAA== X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20170113180426eucas1p21db3bf083c7c8cbf6bf16c4caa9df1ac X-Msg-Generator: CA X-Sender-IP: 182.198.249.180 X-Local-Sender: =?UTF-8?B?UGV0ZXIgU3RlcGhlbnNvbhtTQ1NDLURhdGEgUGxhbmUb?= =?UTF-8?B?7IK87ISx7KCE7J6QG1ByaW5jaXBhbCBFbmdpbmVlciwgU29mdHdhcmU=?= X-Global-Sender: =?UTF-8?B?UGV0ZXIgU3RlcGhlbnNvbhtTQ1NDLURhdGEgUGxhbmUbU2Ft?= =?UTF-8?B?c3VuZyBFbGVjdHJvbmljcxtQcmluY2lwYWwgRW5naW5lZXIsIFNvZnR3YXJl?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDA1Q0QwNTAwNTg=?= CMS-TYPE: 201P X-HopCount: 7 X-CMS-RootMailID: 20161212160617epcas2p16960e3d95c694147035f760090e6011b X-RootMTR: 20161212160617epcas2p16960e3d95c694147035f760090e6011b References: <20161211221844.5e51affe@ntlworld.com> <161212080550.ZM935@torch.brasslantern.com> <20161212163124.6654f077@pwslap01u.europe.root.pri> <20170110193102.7725620a@ntlworld.com> <20170111114232.04eedd08@pwslap01u.europe.root.pri> <20170111205122.2c47e89f@ntlworld.com> On Wed, 11 Jan 2017 20:51:22 +0000 Peter Stephenson wrote: > With a bit of work this can be expanded to paths for already loaded > functions, and possibly elsewhere though I think the pickings are > a lot smaller. This does the former of that --- it was less work than I expected. The upshot of this is that with the new absolute path scheme, storing the filename at the point of loading the function is a no-op since we've already got the components and keep them. With a traditional fpath-style load, we use the new directory cache at the point where we load the file from the path, so this now also avoids an extra directory allocation (and, unlike the similar mechanism used for the command hash, avoids the brittle association with the original path variable). There is a heap allocation when we enter the function to put the name on the function stack (see getshfuncfile()), but there always was --- the only difference is a little extra calculation for the three components. So I think the penalty is trivial to non-existent. The only significant extra requirement is use of the heap to get the filename in "whence -v" --- that's a bit of laziness, we could just print out the components using the same logic as getshfuncfile(). Might fix that later. Note that the PM_LOADDIR flag still represents exactly what it did, it's just that the period of possible validity now lasts throughout the function [nearly said product] life cycle. pws diff --git a/Src/exec.c b/Src/exec.c index 7a5d2bd..6c82643 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -5124,12 +5124,12 @@ execautofn_basic(Estate state, UNUSED(int do_exec)) * defined yet. */ if (funcstack && !funcstack->filename) - funcstack->filename = dupstring(shf->filename); + funcstack->filename = getshfuncfile(shf); oldscriptname = scriptname; oldscriptfilename = scriptfilename; scriptname = dupstring(shf->node.nam); - scriptfilename = dupstring(shf->filename); + scriptfilename = getshfuncfile(shf); execode(shf->funcdef, 1, 0, "loadautofunc"); scriptname = oldscriptname; scriptfilename = oldscriptfilename; @@ -5150,13 +5150,47 @@ execautofn(Estate state, UNUSED(int do_exec)) return execautofn_basic(state, 0); } +/* + * Helper function to install the source file name of a shell function + * just autoloaded. + * + * We attempt to do this efficiently as the typical case is the + * directory part is a well-known directory, which is cached, and + * the non-directory part is the same as the node name. + */ + +/**/ +static void +loadautofnsetfile(Shfunc shf, char *fdir) +{ + /* + * If shf->filename is already the load directory --- + * keep it as we can still use it to get the load file. + * This makes autoload with an absolute path particularly efficient. + */ + if (!(shf->node.flags & PM_LOADDIR) || + strcmp(shf->filename, fdir) != 0) { + /* Old directory name not useful... */ + dircache_set(&shf->filename, NULL); + if (fdir) { + /* ...can still cache directory */ + shf->node.flags |= PM_LOADDIR; + dircache_set(&shf->filename, fdir); + } else { + /* ...no separate directory part to cache, for some reason. */ + shf->node.flags &= ~PM_LOADDIR; + shf->filename = ztrdup(shf->node.nam); + } + } +} + /**/ Shfunc loadautofn(Shfunc shf, int fksh, int autol, int current_fpath) { int noalias = noaliases, ksh = 1; Eprog prog; - char *fname; + char *fdir; /* Directory path where func found */ pushheap(); @@ -5167,13 +5201,13 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath) char *spec_path[2]; spec_path[0] = dupstring(shf->filename); spec_path[1] = NULL; - prog = getfpfunc(shf->node.nam, &ksh, &fname, spec_path, 0); + prog = getfpfunc(shf->node.nam, &ksh, &fdir, spec_path, 0); if (prog == &dummy_eprog && (current_fpath || (shf->node.flags & PM_CUR_FPATH))) - prog = getfpfunc(shf->node.nam, &ksh, &fname, NULL, 0); + prog = getfpfunc(shf->node.nam, &ksh, &fdir, NULL, 0); } else - prog = getfpfunc(shf->node.nam, &ksh, &fname, NULL, 0); + prog = getfpfunc(shf->node.nam, &ksh, &fdir, NULL, 0); noaliases = noalias; if (ksh == 1) { @@ -5192,7 +5226,6 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath) return NULL; } if (!prog) { - zsfree(fname); popheap(); return NULL; } @@ -5205,10 +5238,8 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath) shf->funcdef = prog; else shf->funcdef = dupeprog(prog, 0); - shf->node.flags &= ~(PM_UNDEFINED|PM_LOADDIR); - dircache_set(&shf->filename, NULL); - /* Full filename, don't use dircache */ - shf->filename = fname; + shf->node.flags &= ~PM_UNDEFINED; + loadautofnsetfile(shf, fdir); } else { VARARR(char, n, strlen(shf->node.nam) + 1); strcpy(n, shf->node.nam); @@ -5220,7 +5251,6 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath) zwarn("%s: function not defined by file", n); locallevel++; popheap(); - zsfree(fname); return NULL; } } @@ -5230,10 +5260,8 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath) shf->funcdef = stripkshdef(prog, shf->node.nam); else shf->funcdef = dupeprog(stripkshdef(prog, shf->node.nam), 0); - shf->node.flags &= ~(PM_UNDEFINED|PM_LOADDIR); - dircache_set(&shf->filename, NULL); - /* Full filename, don't use dircache */ - shf->filename = fname; + shf->node.flags &= ~PM_UNDEFINED; + loadautofnsetfile(shf, fdir); } popheap(); @@ -5453,7 +5481,7 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) funcstack = &fstack; fstack.flineno = shfunc->lineno; - fstack.filename = dupstring(shfunc->filename); + fstack.filename = getshfuncfile(shfunc); prog = shfunc->funcdef; if (prog->flags & EF_RUN) { @@ -5623,16 +5651,17 @@ runshfunc(Eprog prog, FuncWrap wrap, char *name) * Search fpath for an undefined function. Finds the file, and returns the * list of its contents. * - * If test is 0, load the function; *fname is set to zalloc'ed location. + * If test is 0, load the function. * * If test_only is 1, don't load function, just test for it: - * - Non-null return means function was found - * - *fname points to path at which found (not duplicated) + * Non-null return means function was found + * + * *fdir points to path at which found (as passed in, not duplicated) */ /**/ Eprog -getfpfunc(char *s, int *ksh, char **fname, char **alt_path, int test_only) +getfpfunc(char *s, int *ksh, char **fdir, char **alt_path, int test_only) { char **pp, buf[PATH_MAX+1]; off_t len; @@ -5650,8 +5679,8 @@ getfpfunc(char *s, int *ksh, char **fname, char **alt_path, int test_only) else strcpy(buf, s); if ((r = try_dump_file(*pp, s, buf, ksh, test_only))) { - if (fname) - *fname = test_only ? *pp : ztrdup(buf); + if (fdir) + *fdir = *pp; return r; } unmetafy(buf, NULL); @@ -5661,7 +5690,8 @@ getfpfunc(char *s, int *ksh, char **fname, char **alt_path, int test_only) (len = lseek(fd, 0, 2)) != -1) { if (test_only) { close(fd); - *fname = *pp; + if (fdir) + *fdir = *pp; return &dummy_eprog; } d = (char *) zalloc(len + 1); @@ -5677,8 +5707,8 @@ getfpfunc(char *s, int *ksh, char **fname, char **alt_path, int test_only) r = parse_string(d, 1); scriptname = oldscriptname; - if (fname) - *fname = ztrdup(buf); + if (fdir) + *fdir = *pp; zfree(d, len + 1); diff --git a/Src/hashtable.c b/Src/hashtable.c index a3a38f7..0fc87f3 100644 --- a/Src/hashtable.c +++ b/Src/hashtable.c @@ -926,10 +926,12 @@ printshfuncnode(HashNode hn, int printflags) (f->node.flags & PM_UNDEFINED) ? " is an autoload shell function" : " is a shell function"); - if (f->filename && (printflags & PRINT_WHENCE_VERBOSE) && - strcmp(f->filename, f->node.nam) != 0) { - printf(" from "); - quotedzputs(f->filename, stdout); + if (printflags & PRINT_WHENCE_VERBOSE) { + char *fname = getshfuncfile(f); + if (fname && strcmp(fname, f->node.nam) != 0) { + printf(" from "); + quotedzputs(fname, stdout); + } } putchar('\n'); return; @@ -959,7 +961,7 @@ printshfuncnode(HashNode hn, int printflags) zputs("builtin autoload -X", stdout); for (fl=0;fopt[fl];fl++) if (f->node.flags & flgs[fl]) putchar(fopt[fl]); - if (f->filename) { + if (f->filename && (f->node.flags & PM_LOADDIR)) { putchar(' '); zputs(f->filename, stdout); } @@ -1041,6 +1043,24 @@ printshfuncexpand(HashNode hn, int printflags, int expand) text_expand_tabs = save_expand; } +/* + * Get a heap-duplicated name of the shell function, for + * use in tracing. + */ + +/**/ +mod_export char * +getshfuncfile(Shfunc shf) +{ + if (shf->node.flags & PM_LOADDIR) { + return zhtricat(shf->filename, "/", shf->node.nam); + } else if (shf->filename) { + return dupstring(shf->filename); + } else { + return NULL; + } +} + /**************************************/ /* Reserved Word Hash Table Functions */ /**************************************/ diff --git a/Src/signals.c b/Src/signals.c index 2de5743..a717677 100644 --- a/Src/signals.c +++ b/Src/signals.c @@ -811,8 +811,11 @@ dosavetrap(int sig, int level) newshf->node.nam = ztrdup(shf->node.nam); newshf->node.flags = shf->node.flags; newshf->funcdef = dupeprog(shf->funcdef, 0); - /* Assume this is a real filename, don't use dircache */ - newshf->filename = ztrdup(shf->filename); + if (shf->node.flags & PM_LOADDIR) { + dircache_set(&newshf->filename, shf->filename); + } else { + newshf->filename = ztrdup(shf->filename); + } if (shf->sticky) { newshf->sticky = sticky_emulation_dup(shf->sticky, 0); } else