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

[Oliver Kiddle <opk@xxxxxxx>]

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?

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;