Re: PATCH: pattern incremental search

[Madhu <enometh@meer.net>]

[-- Attachment #1: Type: Text/Plain, Size: 1916 bytes --]

* Peter Stephenson <1514882387.357077.1640002234148@mail2.virginmedia.com> :
Wrote on Mon, 20 Dec 2021 12:10:34 +0000 (GMT):

> On 12/20/21, Mikael Magnusson <mikachu@gmail.com> wrote:
>> On 4/26/08, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
>>> - if (replstr) { + if (replstr || (fl & SUB_LIST)) {
>> Someone in the irc channel reported a crash on this strlen when
>> doing history-incremental-pattern-search-backward with any search,
>> and they can reproduce it with the latest git version too, they
>> posted this backtrace:
>
> That extra test doesn't look like it makes any sense --- I think it
> may just be in completely the wrong place and shouldn't be in
> get_match_ret() at all since it's similar to some checks in other
> places where we allow zero-length (but not NULL) strings for some edge
> cases in some variants of matching.  We should probably just remove it
> and see what happens.
>
> pws
>
> diff --git a/Src/glob.c b/Src/glob.c index bee890caf..375671cea 100644
> --- a/Src/glob.c +++ b/Src/glob.c @@ -2549,7 +2549,7 @@
> get_match_ret(Imatchdata imd, int b, int e) e += add;
>      /* Everything now refers to metafied lengths. */ - if (replstr ||
> (fl & SUB_LIST)) { + if (replstr) { if (fl & SUB_DOSUBST) { replstr =
> dupstring(replstr); singsub(&replstr);

This doesn't fix the segfault: which just gets postponed.  Besides
this breaks incremental-pattern-search, which just stops working and
doesn't match anything in the history. Also, the segfault only occurs
when zsh is built without multibyte.

To hit the segfault, in a --disable-multibyte build
$ zsh -f
$ bindkey ^R history-incremental-pattern-search-backward
C-r .

Please consider the attached patch which 1) reverts the above fix, and
2) modifies the non-multibyte version of igetmatch to match the
multibyte version at some points. (Disclaimer. this is submitted with
no understanding of what the code does :)


[-- Attachment #2: glob.c-fix-segfault-on-non-multibyte-history-inc.patch --]
[-- Type: Text/X-Patch, Size: 2210 bytes --]

From 81e59a30fa593a8bfd10ad8a28a7475803d5f839 Mon Sep 17 00:00:00 2001
From: Madhu <enometh@net.meer>
Date: Mon, 28 Feb 2022 22:04:09 +0530
Subject: [PATCH] Src/glob.c: fix segfault on non-multibyte
 history-incremental-pattern-search-backward

* Src/glob.c: (get_match_ret): revert the fix in
7f240e6aa9f5596a129474ba6294875dfe7ae264. With this patch ^R stops
working altogether.  (igetmatch): cargo cult port code from the ifdef
MULTIBYTE_SUPPORT version to the ifndef MULTIBYTE_VERSION. This seems
to fix the segfault.
---
 Src/glob.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/Src/glob.c b/Src/glob.c
index 375671c..7e2d810 100644
--- a/Src/glob.c
+++ b/Src/glob.c
@@ -2549,7 +2549,7 @@ get_match_ret(Imatchdata imd, int b, int e)
     e += add;
 
     /* Everything now refers to metafied lengths. */
-    if (replstr) {
+    if (replstr || (fl & SUB_LIST)) {
 	if (fl & SUB_DOSUBST) {
 	    replstr = dupstring(replstr);
 	    singsub(&replstr);
@@ -3351,7 +3351,7 @@ igetmatch(char **sp, Patprog p, int fl, int n, char *replstr,
 	    /* longest or smallest at start with substrings */
 	    t = s;
 	    if (fl & SUB_GLOBAL) {
-		imd.repllist = newlinklist();
+		imd.repllist = (fl & SUB_LIST) ? znewlinklist() : newlinklist();
 		if (repllistp)
 		    *repllistp = imd.repllist;
 	    }
@@ -3481,6 +3481,7 @@ igetmatch(char **sp, Patprog p, int fl, int n, char *replstr,
 	 * Results from get_match_ret in repllist are all metafied.
 	 */
 	s = *sp;
+	if (!(fl & SUB_LIST)) {
 	i = 0;			/* start of last chunk we got from *sp */
 	for (nd = firstnode(imd.repllist); nd; incnode(nd)) {
 	    rd = (Repldata) getdata(nd);
@@ -3503,16 +3504,22 @@ igetmatch(char **sp, Patprog p, int fl, int n, char *replstr,
 	memcpy(t, s + i, l - i);
 	start[lleft] = '\0';
 	*sp = (char *)start;
+	}
 	return 1;
     }
 
+    if (fl & SUB_LIST) {	/* safety: don't think this can happen */
+	return 0;
+    }
+
     /* munge the whole string: no match, so no replstr */
     imd.replstr = NULL;
     imd.repllist = NULL;
     *sp = get_match_ret(&imd, 0, 0);
-    return 1;
+    return (fl & SUB_RETFAIL) ? 0 : 1;
 }
 
+
 /**/
 #endif /* MULTIBYTE_SUPPORT */
 
-- 
2.35.1.dirty