Re: PATCH 1/5: Fix reading outside string in termquery.c

Mikael Magnusson <[email protected]>
Newsgroups gmane.comp.shells.zsh.devel
Message-ID <CAHYJk3QFpPf6+NGxahCjMQPanw71mV1_8sshK9PS8wHpOakHwg@mail.gmail.com>
On Thu, May 7, 2026 at 1:44 AM Oliver Kiddle <[email protected]> wrote:
>
> Mikael Magnusson wrote:
> > +             (strlen(*e + negate) >= editext[i].class &&
>
> Having reviewed the condition again more closely, I'd propose the following
> instead. It'd be good if you could retest with valgrind because I wasn't
> able to reproduce the error. What options are you using with valgrind?

I retested and confirmed this patch also makes the warning go away. I
wasn't using any special options to trigger the warning, but I used
these to hide other junk: -q --trace-children=no --leak-check=no

And later when I jury-rigged valgrind into the test Makefile, I did this:
valgrind --leak-check=no --log-fd=13 --trace-children=yes
--trace-children-skip=/bin/\*,/usr/bin/\* $(dir_top)/Src/zsh +Z -f
$(sdir)/$$ZTST_handler; then \
with the 13 just being the fd I got from exec {myfd}>valgrind.log in
the shell where I was going to run make check. Running the tests this
way takes ~30 minutes on my old computer, and it does cause a number
of the tests to fail with mysterious errors, but it gave me the other
four warnings.

--
Mikael Magnusson

> Also, I needed to check what I wrote in the documentation and noticed a
> line where an extra comma would add clarity.
>
> Oliver
>
> diff --git a/Doc/Zsh/zle.yo b/Doc/Zsh/zle.yo
> index ead52065a..4d44ecf21 100644
> --- a/Doc/Zsh/zle.yo
> +++ b/Doc/Zsh/zle.yo
> @@ -2636,7 +2636,7 @@ as by adding `tt(-cursor)' to disable cursor shape and color changing.
>
>  When ZLE starts, it will add entries for features that were auto-detected.  This
>  auto-detection uses extensions itself, all named with a `tt(query)' prefix.  As
> -this happens when ZLE starts disabling them needs to be done early in the
> +this happens when ZLE starts, disabling them needs to be done early in the
>  startup files.  A value of `tt(-query)' will disable all terminal queries on
>  startup, including those that query terminal properties such as colors rather
>  than detecting features.  Populating the array with the status of auto-detected
> diff --git a/Src/Zle/termquery.c b/Src/Zle/termquery.c
> index ef6f013b2..4d034a7fe 100644
> --- a/Src/Zle/termquery.c
> +++ b/Src/Zle/termquery.c
> @@ -707,11 +707,11 @@ collate_seq(int sindex, int dir)
>             int negate = (**e == '-');
>             if (negate != enabled)
>                 continue;
> -           if ((editext[i].class &&
> -                !strncmp(*e + negate, editext[i].key, editext[i].class) &&
> -               !*(*e + negate + editext[i].class)) ||
> +           if ((!editext[i].class ||
> +                !strncmp(*e + negate, editext[i].key, editext[i].class)) &&
> +               ((editext[i].class && !*(*e + negate + editext[i].class)) ||
>                 !strcmp(*e + negate + editext[i].class,
> -                    editext[i].key + editext[i].class))
> +                    editext[i].key + editext[i].class)))
>             {
>                  enabled = !negate;
>                 break;
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.