zsh-workers
 help / color / mirror / code / Atom feed
From: Mikael Magnusson <mikachu@gmail.com>
To: Bart Schaefer <schaefer@brasslantern.com>
Cc: Zsh hackers list <zsh-workers@zsh.org>
Subject: Re: PATCH: [RFC] crash with weird completer
Date: Thu, 10 Mar 2022 13:27:18 +0100	[thread overview]
Message-ID: <CAHYJk3RBj4YnkM2WcRb8VH_X8-V4ArhTkZBvpjo7VN6VnTYd6w@mail.gmail.com> (raw)
In-Reply-To: <CAH+w=7YoiRdzE9ewFRij-pRE4idpXGU=C7+b_TLNMba07uLzPw@mail.gmail.com>

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


      reply	other threads:[~2022-03-10 12:27 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-08 19:32 Mikael Magnusson
2022-03-10  5:47 ` Bart Schaefer
2022-03-10 12:27   ` Mikael Magnusson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHYJk3RBj4YnkM2WcRb8VH_X8-V4ArhTkZBvpjo7VN6VnTYd6w@mail.gmail.com \
    --to=mikachu@gmail.com \
    --cc=schaefer@brasslantern.com \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).