* [BUG] Strange auto-load behaviour when function name contains hyphen @ 2017-12-14 6:46 ` dana 2017-12-14 10:01 ` Peter Stephenson 0 siblings, 1 reply; 6+ messages in thread From: dana @ 2017-12-14 6:46 UTC (permalink / raw) To: zsh-workers Hey again. :/ The documentation states that function files intended for zsh-style auto-loading are allowed to contain a 'simple definition' (`foo() { ... }`) if that's the only thing in the file. This seems to work fine... except when the function name contains a hyphen. Illustration: % fpath=( . $fpath ) % print -r 'foo_bar() print foo_bar' > foo_bar % autoload -Uz foo_bar % foo_bar foo_bar % print -r 'foo-bar() print foo-bar' > foo-bar % autoload -Uz foo-bar % foo-bar # No output — function is only defined now % foo-bar # Second call actually executes the function foo-bar In the foo-bar case, stripkshdef() doesn't strip the definition out because the Eprog->strs value for the function contains weird characters (and thus fails the strcmp() against the expected name). Specifically, it seems the strs value contains \x9b in place of the hyphens. Not sure what the significance of that is; that's about as far as i could follow it, unfortunately. dana ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BUG] Strange auto-load behaviour when function name contains hyphen 2017-12-14 6:46 ` [BUG] Strange auto-load behaviour when function name contains hyphen dana @ 2017-12-14 10:01 ` Peter Stephenson 2017-12-14 10:28 ` Peter Stephenson 0 siblings, 1 reply; 6+ messages in thread From: Peter Stephenson @ 2017-12-14 10:01 UTC (permalink / raw) To: zsh-workers On Thu, 14 Dec 2017 00:46:29 -0600 dana <dana@dana.is> wrote: > The documentation states that function files intended for zsh-style > auto-loading are allowed to contain a 'simple definition' (`foo() { > ... }`) if that's the only thing in the file. This seems to work > fine... except when the function name contains a hyphen. A while back we had to turn "-" into a token to fix a compatibility issue with pattern matching in POSIX shells. This has caused problems to come out of the wardrobe ever since. ("Woodwork" implies a level of solid material without the appropriate magical connotations.) This has got line offsets including the function line numbering change as it currently stands but I won't commit that. pws diff --git a/Src/exec.c b/Src/exec.c index fc6d02d..5149349 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -5932,6 +5934,7 @@ stripkshdef(Eprog prog, char *name) { Wordcode pc; wordcode code; + char *ptr1, *ptr2; if (!prog) return NULL; @@ -5942,8 +5945,23 @@ stripkshdef(Eprog prog, char *name) return prog; pc++; code = *pc++; - if (wc_code(code) != WC_FUNCDEF || - *pc != 1 || strcmp(name, ecrawstr(prog, pc + 1, NULL))) + if (wc_code(code) != WC_FUNCDEF || *pc != 1) + return prog; + + /* + * See if name of function requested (name) is same as + * name of function in word code. name may still have "-" + * tokenised. + */ + ptr1 = name; + ptr2 = ecrawstr(prog, pc + 1, NULL); + while (*ptr1 && *ptr2) { + if (*ptr1 != *ptr2 && *ptr1 != Dash && *ptr1 != '-') + break; + ptr1++; + ptr2++; + } + if (*ptr1 || *ptr2) return prog; { ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BUG] Strange auto-load behaviour when function name contains hyphen 2017-12-14 10:01 ` Peter Stephenson @ 2017-12-14 10:28 ` Peter Stephenson 2017-12-14 17:21 ` dana 0 siblings, 1 reply; 6+ messages in thread From: Peter Stephenson @ 2017-12-14 10:28 UTC (permalink / raw) To: zsh-workers On Thu, 14 Dec 2017 10:01:42 +0000 Peter Stephenson <p.stephenson@samsung.com> wrote: > + if (*ptr1 != *ptr2 && *ptr1 != Dash && *ptr1 != '-') Bart noticed the error here. I stared at for ages without noticing. pws diff --git a/Src/exec.c b/Src/exec.c index fc6d02d..eda1015 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -5669,11 +5669,13 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) funcsave->fstack.caller = funcstack ? funcstack->name : dupstring(funcsave->argv0 ? funcsave->argv0 : argzero); funcsave->fstack.lineno = lineno; + funcsave->fstack.flineno = shfunc->lineno; + if (funcstack) + funcsave->fstack.flineno += funcstack->flineno; funcsave->fstack.prev = funcstack; funcsave->fstack.tp = FS_FUNC; funcstack = &funcsave->fstack; - funcsave->fstack.flineno = shfunc->lineno; funcsave->fstack.filename = getshfuncfile(shfunc); prog = shfunc->funcdef; @@ -5932,6 +5934,7 @@ stripkshdef(Eprog prog, char *name) { Wordcode pc; wordcode code; + char *ptr1, *ptr2; if (!prog) return NULL; @@ -5942,8 +5945,23 @@ stripkshdef(Eprog prog, char *name) return prog; pc++; code = *pc++; - if (wc_code(code) != WC_FUNCDEF || - *pc != 1 || strcmp(name, ecrawstr(prog, pc + 1, NULL))) + if (wc_code(code) != WC_FUNCDEF || *pc != 1) + return prog; + + /* + * See if name of function requested (name) is same as + * name of function in word code. name may still have "-" + * tokenised. + */ + ptr1 = name; + ptr2 = ecrawstr(prog, pc + 1, NULL); + while (*ptr1 && *ptr2) { + if (*ptr1 != *ptr2 && *ptr1 != Dash && *ptr2 != '-') + break; + ptr1++; + ptr2++; + } + if (*ptr1 || *ptr2) return prog; { ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BUG] Strange auto-load behaviour when function name contains hyphen 2017-12-14 10:28 ` Peter Stephenson @ 2017-12-14 17:21 ` dana 2017-12-15 2:33 ` dana 2017-12-15 9:09 ` Peter Stephenson 0 siblings, 2 replies; 6+ messages in thread From: dana @ 2017-12-14 17:21 UTC (permalink / raw) To: Peter Stephenson; +Cc: zsh-workers On 14 Dec 2017, at 04:28, Peter Stephenson <p.stephenson@samsung.com> wrote: >+ /* >+ * See if name of function requested (name) is same as >+ * name of function in word code. name may still have "-" >+ * tokenised. >+ */ >+ ptr1 = name; >+ ptr2 = ecrawstr(prog, pc + 1, NULL); >+ while (*ptr1 && *ptr2) { >+ if (*ptr1 != *ptr2 && *ptr1 != Dash && *ptr2 != '-') I think there's another error here. name isn't the one that might be tokenised (or it's not in my case anyway) — it's the other one. (And just swapping the last two tests would make it so that it accepts a definition with a different name than expected.) The patch seems to work for me if i change the condition to this: if (*ptr1 != *ptr2 && !(*ptr1 == '-' && *ptr2 == Dash)) Thanks! dana ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BUG] Strange auto-load behaviour when function name contains hyphen 2017-12-14 17:21 ` dana @ 2017-12-15 2:33 ` dana 2017-12-15 9:09 ` Peter Stephenson 1 sibling, 0 replies; 6+ messages in thread From: dana @ 2017-12-15 2:33 UTC (permalink / raw) To: Peter Stephenson; +Cc: zsh-workers On 14 Dec 2017, at 11:21, dana <dana@dana.is> wrote: >The patch seems to work for me if i change the condition to this: Understanding a bit more about the token thing, i went and tested some of the others defined in zsh.h. The same problem occurs if you have a function name containing almost any of them. Also, if your function name is quoted or escaped, the tokens for those meta-characters are present and have to be dealt with for the comparison too. I'm not sure why someone would use an asterisk or whatever in a function name, but i can't find anything in the documentation that says you shouldn't (and i'm not aware of any issues doing so aside from this). Should it perform a more comprehensive check? Something like the following? Possibly it could be optimised to avoid going through the whole dance when it doesn't need to (considering it's an edge case)... dana diff --git a/Src/exec.c b/Src/exec.c index fc6d02dc3..4c096fd12 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -5932,6 +5932,7 @@ stripkshdef(Eprog prog, char *name) { Wordcode pc; wordcode code; + char *pname, *litpname; if (!prog) return NULL; @@ -5942,10 +5943,25 @@ stripkshdef(Eprog prog, char *name) return prog; pc++; code = *pc++; - if (wc_code(code) != WC_FUNCDEF || - *pc != 1 || strcmp(name, ecrawstr(prog, pc + 1, NULL))) + if (wc_code(code) != WC_FUNCDEF || *pc != 1) return prog; + /* + * Detokenise and literalise the name in the word code (it may be quoted) + * and make sure it matches the expected name + */ + pname = ecrawstr(prog, pc + 1, NULL); + litpname = ztrdup(pname); + parse_subst_string(litpname); + remnulargs(litpname); + untokenize(litpname); + + if (strcmp(name, litpname)) { + free(litpname); + return prog; + } + free(litpname); + { Eprog ret; Wordcode end = pc + WC_FUNCDEF_SKIP(code); diff --git a/Test/C04funcdef.ztst b/Test/C04funcdef.ztst index 0c00a0477..cd29f658c 100644 --- a/Test/C04funcdef.ztst +++ b/Test/C04funcdef.ztst @@ -515,6 +515,22 @@ 0:autoload with absolute path not cancelled by bare autoload >I have been loaded by explicit path. + ( + fpath=(.) + print -n '[' + for f in ${(s<>):-'x "#$^*$=|`<>?~,-!'}; do + # Test with both backslashes and single-quotes + print -r -- "${(q)f}_1() print -nr -- ${(q)f}" > ${f}_1 + print -r -- "${(qq)f}_2() print -nr -- ${(qq)f}" > ${f}_2 + autoload -Uz -- ${f}_1 ${f}_2 + ${f}_1 + ${f}_2 + done + print ']' + ) +0:autoload with weird function names and ksh-style definitions +>[xx ""##$$^^**$$==||``<<>>??~~,,--!!] + ( FUNCNEST=0 fn() { true; } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BUG] Strange auto-load behaviour when function name contains hyphen 2017-12-14 17:21 ` dana 2017-12-15 2:33 ` dana @ 2017-12-15 9:09 ` Peter Stephenson 1 sibling, 0 replies; 6+ messages in thread From: Peter Stephenson @ 2017-12-15 9:09 UTC (permalink / raw) To: zsh-workers On Thu, 14 Dec 2017 11:21:21 -0600 dana <dana@dana.is> wrote: > On 14 Dec 2017, at 04:28, Peter Stephenson <p.stephenson@samsung.com> wrote: > >+ /* > >+ * See if name of function requested (name) is same as > >+ * name of function in word code. name may still have "-" > >+ * tokenised. > >+ */ > >+ ptr1 = name; > >+ ptr2 = ecrawstr(prog, pc + 1, NULL); > >+ while (*ptr1 && *ptr2) { > >+ if (*ptr1 != *ptr2 && *ptr1 != Dash && *ptr2 != '-') > > I think there's another error here. name isn't the one that might be tokenised > (or it's not in my case anyway) — it's the other one. That suggests another bug since I don't think function names in the word code ought to be tokenised, but quite possibly that hasn't been an issue up to now. I haven't gone looking for that --- I've just handled both cases (and added a test for the obvious case). > Understanding a bit more about the token thing, i went and tested some of the > others defined in zsh.h. The same problem occurs if you have a function name > containing almost any of them. Dash is a bit of a special case, since in normal zsh syntax it's actually not a pattern character. Things like * and ? will cause immediate expansion, so any function name using them always has to be quoted. There's not a lot of point in supporting ths, though, as you say, it's not explicitly excluded at the moment. Quotes are stripped from the word on the command line before the comparision. I think that's all that sanity really requires. pws ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-12-15 9:09 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20171214064805epcas2p196a185a6c3c5e66640833d8dfa15d593@epcas2p1.samsung.com> 2017-12-14 6:46 ` [BUG] Strange auto-load behaviour when function name contains hyphen dana 2017-12-14 10:01 ` Peter Stephenson 2017-12-14 10:28 ` Peter Stephenson 2017-12-14 17:21 ` dana 2017-12-15 2:33 ` dana 2017-12-15 9:09 ` Peter Stephenson
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).