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

[Mikael Magnusson <mikachu@xxxxxxxxx>]

On Thu, May 7, 2026 at 1:44 AM Oliver Kiddle <opk@xxxxxxx> 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;