* PATCH: [RFC] crash with weird completer @ 2022-03-08 19:32 Mikael Magnusson 2022-03-10 5:47 ` Bart Schaefer 0 siblings, 1 reply; 3+ messages in thread From: Mikael Magnusson @ 2022-03-08 19:32 UTC (permalink / raw) To: zsh-workers minimal reproducer .zshrc: zstyle ':completion:*' completer _oldlist _complete setopt nolistambiguous autoload compinit; compinit compdef _foo foo;_foo() { compadd -Q -- stash@{{0,1}} } type "foo " and press tab twice, and it should crash. The patch prevents the crash but I have no further arguments that it's correct. Also without 'setopt nolistambiguous' we just get an additional { inserted for every tab press which also doesn't seem useful (not fixed by patch). (Apologies if I've sent this before, looking through local commits that could potentially be useful to send.) --- Src/Zle/compresult.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Src/Zle/compresult.c b/Src/Zle/compresult.c index 8b5955819a..12585e3564 100644 --- a/Src/Zle/compresult.c +++ b/Src/Zle/compresult.c @@ -608,7 +608,7 @@ instmatch(Cmatch m, int *scs) r += l; ocs = zlemetacs; /* Re-insert the brace beginnings, if any. */ - if (brbeg) { + if (brbeg && m->brpl) { int pcs = zlemetacs; l = 0; -- 2.15.1 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: PATCH: [RFC] crash with weird completer 2022-03-08 19:32 PATCH: [RFC] crash with weird completer Mikael Magnusson @ 2022-03-10 5:47 ` Bart Schaefer 2022-03-10 12:27 ` Mikael Magnusson 0 siblings, 1 reply; 3+ messages in thread From: Bart Schaefer @ 2022-03-10 5:47 UTC (permalink / raw) To: Mikael Magnusson; +Cc: Zsh hackers list On Tue, Mar 8, 2022 at 11:33 AM Mikael Magnusson <mikachu@gmail.com> wrote: > > type "foo " and press tab twice, and it should crash. Alternate patch below. This differs in that it initializes the "lastprebr" array, which step is skipped by Mikael's patch. > The patch prevents the crash but I have no further arguments that it's > correct. Ditto. > Also without 'setopt nolistambiguous' we just get an additional > { inserted for every tab press which also doesn't seem useful (not fixed > by patch). This is unrelated and I think connected to something mentioned elsewhere; I can't find the reference at the moment, but IIRC the gist is that because you're completing after an open brace, it's removed from the prefix in expectation of completing a brace expansion, so the suffix to be added then appears to begin with a brace, which is added after the actual prefix on the line, resulting in doubled (etc.) braces. You can work around this by quoting the braces that should be treated literally: compdef _foo foo;_foo() { compadd -Q -- stash@\\{{0,1}\\} } The patch: diff --git a/Src/Zle/compresult.c b/Src/Zle/compresult.c index 8b5955819..a1f2ec322 100644 --- a/Src/Zle/compresult.c +++ b/Src/Zle/compresult.c @@ -614,7 +614,7 @@ instmatch(Cmatch m, int *scs) l = 0; for (bp = brbeg, brpos = m->brpl, bradd = (m->pre ? strlen(m->pre) : 0); - bp; bp = bp->next, brpos++) { + bp && brpos; bp = bp->next, brpos++) { zlemetacs = a + *brpos + bradd; pcs = zlemetacs; l = strlen(bp->str); ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: PATCH: [RFC] crash with weird completer 2022-03-10 5:47 ` Bart Schaefer @ 2022-03-10 12:27 ` Mikael Magnusson 0 siblings, 0 replies; 3+ messages in thread From: Mikael Magnusson @ 2022-03-10 12:27 UTC (permalink / raw) To: Bart Schaefer; +Cc: Zsh hackers list On 3/10/22, Bart Schaefer <schaefer@brasslantern.com> wrote: > On Tue, Mar 8, 2022 at 11:33 AM Mikael Magnusson <mikachu@gmail.com> wrote: >> >> type "foo " and press tab twice, and it should crash. > > Alternate patch below. This differs in that it initializes the > "lastprebr" array, which step is skipped by Mikael's patch. > >> The patch prevents the crash but I have no further arguments that it's >> correct. > > Ditto. > > You can work around this by quoting the braces that should be treated > literally: > compdef _foo foo;_foo() { compadd -Q -- stash@\\{{0,1}\\} } I don't remember the thought process that led to the test case, but in that case you could just drop the -Q too, right? > The patch: Hmm, I guess this is safer in the case that somehow m->brpl is set but a subsequent brpos is NULL. It's probably (definitely) not worth bothering about skipping NULL entries to process other ones (eg, continue instead of break when !brpos) I was staring at the original diff in gmail trying to figure out what I was looking at, and this should be equivalent to your patch but spare the eyes of future generations slightly (this is directed at the original code, not your patch), diff --git a/Src/Zle/compresult.c b/Src/Zle/compresult.c index 8b5955819a..0fed297b56 100644 --- a/Src/Zle/compresult.c +++ b/Src/Zle/compresult.c @@ -612,9 +612,10 @@ instmatch(Cmatch m, int *scs) int pcs = zlemetacs; l = 0; - for (bp = brbeg, brpos = m->brpl, - bradd = (m->pre ? strlen(m->pre) : 0); - bp; bp = bp->next, brpos++) { + bradd = (m->pre ? strlen(m->pre) : 0); + for (bp = brbeg, brpos = m->brpl; + bp && brpos; + bp = bp->next, brpos++) { zlemetacs = a + *brpos + bradd; pcs = zlemetacs; l = strlen(bp->str); Eg, bradd is not involved with the loop condition at all, so don't set it in the for-initializer, and put each foo;bar;baz; on separate lines. -- Mikael Magnusson ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-03-10 12:27 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-08 19:32 PATCH: [RFC] crash with weird completer Mikael Magnusson 2022-03-10 5:47 ` Bart Schaefer 2022-03-10 12:27 ` Mikael Magnusson
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).