Re: PATCH: pattern incremental search

[Bart Schaefer <schaefer@brasslantern.com>]

[-- Attachment #1: Type: text/plain, Size: 2199 bytes --]

                /* patmatchlen returns metafied length, as we need */
                On Thu, Mar 31, 2022 at 8:49 PM Bart Schaefer
<schaefer@brasslantern.com> wrote:
>
> The following appears to fix the crash problem.  I don't know what
> else the rest of the patch in workers/49781 accomplishes.

Hmm, sorry ... that fixes the crash when there is no match on the
search, but there is still a crash when the search does begin to match
something.

I note this commentary difference from the MULTIBYTE to not-so case,
which I hope is correct:

-                /* patmatchlen returns unmetafied length in this case */
+                /* patmatchlen returns metafied length, as we need */

Also in the MULTIBYTE branch, we have declared two "char *mpos" in
nested scopes, and I can't immediately tell whether that's a bug
waiting to be surfaced or if the not-MULTIBYTE branch just needs to
declare the outer mpos.  This has a potential effect because there's a
call to get_match_ret(&imd, t-s, mpos-s); that is in the outer (loop)
scope in the MULTIBYTE branch but the inner ("if (pattrylen(...))")
scope in the not-MULTIBYTE branch.  I would expect one of those
placements must be wrong?

Those differences aside, the hunk from workers/49781 that prevents the
crash in the successful-match case is this one:

@@ -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);

(plus the matching close-brace, of course).  SUB_LIST is explained as
/* no substitution, return list of matches */
so I'm somewhat concerned that there remains a different way to enter
this code that is still going to crash it, but I don't know how to
force that path.

Anyway, a shorter but equivalent (requires no re-indentation) fix is
included in the patch below.  I have not attempted to resolve the
"mpos" question.  I did pull in a couple of what appear to be
optimizations (early loop break) from the MULTIBYTE code.

[-- Attachment #2: glob-pattern-search.txt --]
[-- Type: text/plain, Size: 2399 bytes --]

diff --git a/Src/Zle/zle_refresh.c b/Src/Zle/zle_refresh.c
index d9d9503e2..c8c6f78c6 100644
--- a/Src/Zle/zle_refresh.c
+++ b/Src/Zle/zle_refresh.c
@@ -255,7 +255,9 @@ int cost;
 #endif
 
 static const REFRESH_ELEMENT zr_cr = { ZWC('\r'), 0 };
+#ifdef MULTIBYTE_support
 static const REFRESH_ELEMENT zr_dt = { ZWC('.'), 0 };
+#endif
 static const REFRESH_ELEMENT zr_nl = { ZWC('\n'), 0 };
 static const REFRESH_ELEMENT zr_sp = { ZWC(' '), 0 };
 static const REFRESH_ELEMENT zr_zr = { ZWC('\0'), 0 };
diff --git a/Src/glob.c b/Src/glob.c
index 349862531..d4ffc2274 100644
--- a/Src/glob.c
+++ b/Src/glob.c
@@ -3286,6 +3286,7 @@ igetmatch(char **sp, Patprog p, int fl, int n, char *replstr,
 	return 1;
     }
     if (matched) {
+        /* Default is to match at the start; see comment in MULTIBYTE above */
 	switch (fl & (SUB_END|SUB_LONG|SUB_SUBSTR)) {
 	case 0:
 	case SUB_LONG:
@@ -3355,7 +3356,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;
 	    }
@@ -3405,6 +3406,8 @@ igetmatch(char **sp, Patprog p, int fl, int n, char *replstr,
 			 * which is already marked for replacement.
 			 */
 			matched = 1;
+			if (t == send)
+			    break;
 			while (t < mpos) {
 			    ioff++;
 			    umlen--;
@@ -3412,8 +3415,10 @@ igetmatch(char **sp, Patprog p, int fl, int n, char *replstr,
 			}
 			break;
 		    }
+		    if (t == send)
+			break;
 		}
-	    } while (matched);
+	    } while (matched && t < send);
 	    /*
 	     * check if we can match a blank string, if so do it
 	     * at the start.  Goodness knows if this is a good idea
@@ -3485,6 +3490,8 @@ 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)
+	    return 1;
 	i = 0;			/* start of last chunk we got from *sp */
 	for (nd = firstnode(imd.repllist); nd; incnode(nd)) {
 	    rd = (Repldata) getdata(nd);
@@ -3514,7 +3521,7 @@ igetmatch(char **sp, Patprog p, int fl, int n, char *replstr,
     imd.replstr = NULL;
     imd.repllist = NULL;
     *sp = get_match_ret(&imd, 0, 0);
-    return 1;
+    return (fl & SUB_RETFAIL) ? 0 : 1;
 }
 
 /**/