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;