PATCH: Re: Seg fault in matcher-list matching

PATCH: Re: Seg fault in matcher-list matching

[Sven Wischnowsky]

Bart Schaefer wrote:

> I did this:
> 
> zagzig[41] /u/s/l/z/z4/s/zsh
>                      ^cursor over the 4, press TAB
> 
> The path this was intended to match was /usr/src/local/zsh/zsh-2.4/src/zsh.
> I had first tried TAB at the end of the line and gotten a feep, in case
> that matters.

I couldn't get it to seg-fault, but there was something broken. Matching 
of the suffix, for example, and that both in C and shell code.


Oliver Kiddle wrote:

> ...
> 
> It's too late too look at this in any more detail but I've just found that
> I get a seg fault when I do this:
> diff  .c<tab>
>      ^cursor here.

I couldn't reproduce this (neither before nor after this patch -- your 
match specs would have helped), but this looks suspiciously similar to 
the one above... could you try with this patch?

Bye
 Sven

Index: Completion/Core/_path_files
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Core/_path_files,v
retrieving revision 1.11
diff -u -r1.11 _path_files
--- Completion/Core/_path_files	2000/05/02 08:18:54	1.11
+++ Completion/Core/_path_files	2000/05/15 09:25:35
@@ -6,7 +6,7 @@
 local linepath realpath donepath prepath testpath exppath skips skipped
 local tmp1 tmp2 tmp3 tmp4 i orig eorig pre suf tpre tsuf opre osuf cpre
 local pats haspats ignore pfxsfx remt sopt gopt opt sdirs ignpar
-local nm=$compstate[nmatches] menu matcher mopts sort match
+local nm=$compstate[nmatches] menu matcher mopts sort match mid
 
 typeset -U prepaths exppaths
 
@@ -529,6 +529,7 @@
       cpre="${cpre}${tpre%%/*}/"
       tpre="${tpre#*/}"
     elif [[ "$tsuf" = */* ]]; then
+      mid="$testpath"
       cpre="${cpre}${tpre}/"
       tpre="${tsuf#*/}"
       tsuf=
@@ -539,17 +540,32 @@
   done
 
   if [[ -z "$tmp4" ]]; then
-    if [[ "$osuf" = */* ]]; then
-      PREFIX="${opre}${osuf}"
-      SUFFIX=
-    else
+    if [[ "$mid" = */ ]]; then
       PREFIX="${opre}"
       SUFFIX="${osuf}"
+
+      tmp4="${testpath#${mid}}"
+      tmp3="${mid%/*/}"
+      tmp2="${${mid%/}##*/}"
+      compquote tmp4 tmp3 tmp2 tmp1
+      for i in "$tmp1[@]"; do
+        compadd -Qf "$mopts[@]" -p "$linepath$tmp3/" -s "/$tmp4$i" \
+                -W "$prepath$realpath${mid%/*/}/" \
+	        "$pfxsfx[@]" -M "r:|/=* r:|=*" - "$tmp2"
+      done
+    else
+      if [[ "$osuf" = */* ]]; then
+        PREFIX="${opre}${osuf}"
+        SUFFIX=
+      else
+        PREFIX="${opre}"
+        SUFFIX="${osuf}"
+      fi
+      tmp4="$testpath"
+      compquote tmp4 tmp1
+      compadd -Qf "$mopts[@]" -p "$linepath$tmp4" -W "$prepath$realpath$testpath" \
+	      "$pfxsfx[@]" -M "r:|/=* r:|=*" - "$tmp1[@]"
     fi
-    tmp4="$testpath"
-    compquote tmp4 tmp1
-    compadd -Qf "$mopts[@]" -p "$linepath$tmp4" -W "$prepath$realpath$testpath" \
-	    "$pfxsfx[@]" -M "r:|/=* r:|=*" - "$tmp1[@]"
   fi
 done
 
Index: Src/Zle/compmatch.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/compmatch.c,v
retrieving revision 1.12
diff -u -r1.12 compmatch.c
--- Src/Zle/compmatch.c	2000/05/12 11:52:30	1.12
+++ Src/Zle/compmatch.c	2000/05/15 09:25:37
@@ -482,9 +482,10 @@
 	 */
 
 	bslash = 0;
-	if (test && (l[ind] == w[ind] ||
-		     (bslash = (lw > 1 && w[ind] == '\\' &&
-				(ind ? (w[0] == l[0]) : (w[1] == l[0])))))) {
+	if (test && !sfx &&
+	    (l[ind] == w[ind] ||
+	     (bslash = (lw > 1 && w[ind] == '\\' &&
+			(ind ? (w[0] == l[0]) : (w[1] == l[0])))))) {
 	    /* No matcher could be used, but the strings have the same
 	     * character here, skip over it. */
 	    l += add; w += (bslash ? (add + add ) : add);
@@ -562,10 +563,8 @@
 
 		    /* Fine, now we call ourselves recursively to find the
 		     * string matched by the `*'. */
-		    if (sfx) {
-			savl = l[-(llen + zoff)];
+		    if (sfx && (savl = l[-(llen + zoff)]))
 			l[-(llen + zoff)] = '\0';
-		    }
 		    for (t = 0, tp = w, ct = 0, ict = lw - alen + 1;
 			 ict;
 			 tp += add, ct++, ict--) {
@@ -585,11 +584,12 @@
 				!match_parts(l + aoff , tp - moff, alen, part))
 				break;
 			    if (sfx) {
-				savw = tp[-zoff];
-				tp[-zoff] = '\0';
+				if ((savw = tp[-zoff]))
+				    tp[-zoff] = '\0';
 				t = match_str(l - ll, w - lw,
 					      NULL, 0, NULL, 1, 2, part);
-				tp[-zoff] = savw;
+				if (savw)
+				    tp[-zoff] = savw;
 			    } else
 				t = match_str(l + llen + moff, tp + moff,
 					      NULL, 0, NULL, 0, 1, part);
@@ -598,7 +598,7 @@
 			}
 		    }
 		    ict = ct;
-		    if (sfx)
+		    if (sfx && savl)
 			l[-(llen + zoff)] = savl;
 
 		    /* Have we found a position in w where the rest of l
@@ -794,9 +794,10 @@
 	/* Same code as at the beginning, used in top-level calls. */
 
 	bslash = 0;
-	if (!test && (l[ind] == w[ind] ||
-		      (bslash = (lw > 1 && w[ind] == '\\' &&
-				 (ind ? (w[0] == l[0]) : (w[1] == l[0])))))) {
+	if ((!test || sfx) &&
+	    (l[ind] == w[ind] ||
+	     (bslash = (lw > 1 && w[ind] == '\\' &&
+			(ind ? (w[0] == l[0]) : (w[1] == l[0])))))) {
 	    /* No matcher could be used, but the strings have the same
 	     * character here, skip over it. */
 	    l += add; w += (bslash ? (add + add ) : add);
@@ -811,6 +812,8 @@
 	    lm = NULL;
 	    he = 0;
 	} else {
+	    if (!lw)
+		break;
 	    /* No matcher and different characters: l does not match w. */
 	    if (test)
 		return 0;
@@ -873,10 +876,15 @@
     char lsav = l[n], wsav = w[n];
     int ret;
 
-    l[n] = w[n] = '\0';
+    if (lsav)
+	l[n] = '\0';
+    if (wsav)
+	w[n] = '\0';
     ret = match_str(l, w, NULL, 0, NULL, 0, 1, part);
-    l[n] = lsav;
-    w[n] = wsav;
+    if (lsav)
+	l[n] = lsav;
+    if (wsav)
+	w[n] = wsav;
 
     return ret;
 }

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: PATCH: Re: Seg fault in matcher-list matching

[Bart Schaefer]

On May 15, 11:26am, Sven Wischnowsky wrote:
} Subject: PATCH: Re: Seg fault in matcher-list matching
}
} 
} Bart Schaefer wrote:
} 
} > I did this:
} > 
} > zagzig[41] /u/s/l/z/z4/s/zsh
} >                      ^cursor over the 4, press TAB
} > 
} > The path this was intended to match was /usr/src/local/zsh/zsh-2.4/src/zsh.
} > I had first tried TAB at the end of the line and gotten a feep, in case
} > that matters.
} 
} I couldn't get it to seg-fault, but there was something broken. Matching 
} of the suffix, for example, and that both in C and shell code.

It doesn't dump for me any more, but I'm still nervous about line 1767 of
compcore.c:

#1  0x80bd798 in addmatches (dat=0xbfffa854, argv=0xbfffa8d8)
    at ../../../zsh-3.1.6/Src/Zle/compcore.c:1768
1768                if ((ml = match_str(lsuf, s, &bsl, 0, NULL, 1, 0, 1)) >= 0) {
(gdb) l 
1763                    else
1764                        *argv = NULL;
1765                    bcp = lpl;
1766                }
1767                s = dat->psuf ? dat->psuf : "";
1768                if ((ml = match_str(lsuf, s, &bsl, 0, NULL, 1, 0, 1)) >= 0) {
1769                    if (matchsubs) {
1770                        Cline tmp = get_cline(NULL, 0, NULL, 0, NULL, 0, CLF_SUF);
1771
1772                        tmp->suffix = matchsubs;

The reported core dump was caused because match_str() wrote a '\0' byte into
the string pointed to by its second argument [`s' above, `w' in match_str()]
which is being passed as a string constant when dat->psuf == 0.  Is that a
potential bug, still?

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

seg fault fixed and Makefile change

[Oliver Kiddle]

Sven Wischnowsky wrote:
> 
> I couldn't reproduce this (neither before nor after this patch -- your
> match specs would have helped), but this looks suspiciously similar to
> the one above... could you try with this patch?

That's fixed it: it now works fine, thanks.

This small patch to the Makefile causes it to install the functions
before the man pages. This is because the man page installation fails
if yodl isn't installed meaning that I have to do a separate
make install.fns. Of course this is only an issue if someone compiles
from CVS because the proper releases include pre-built man pages.

Completion for netscape remote commands doesn't seem to work properly
anymore:
netscape -remote <tab>
Basically, it is a bit too eager to put the \( suffix in. This did
actually work before but I've always felt that this part of _netscape
(which I am to blame for) is a bit of a nasty hack. Basically, the
'(' suffix needs to be quoted unless it already is and that was the
way I found which worked. 

Oliver

Index: Makefile.in
===================================================================
RCS file: /cvsroot/zsh/zsh/Makefile.in,v
retrieving revision 1.3
diff -u -r1.3 Makefile.in
--- Makefile.in 2000/04/06 11:47:46     1.3
+++ Makefile.in 2000/05/15 11:57:53
@@ -60,8 +60,8 @@
        $(MAKE) install STRIPFLAGS="-s"

 # install/uninstall most things
-install: install.bin install.modules install.man install.fns
-uninstall: uninstall.bin uninstall.modules uninstall.man uninstall.fns
+install: install.bin install.modules install.fns install.man
+uninstall: uninstall.bin uninstall.modules uninstall.fns uninstall.man

 # install/uninstall just the binary
 install.bin uninstall.bin:

Re: PATCH: Re: Seg fault in matcher-list matching

[Tanaka Akira]

In article <200005150926.LAA17371@beta.informatik.hu-berlin.de>,
  Sven Wischnowsky <wischnow@informatik.hu-berlin.de> writes:

> I couldn't get it to seg-fault, but there was something broken. Matching 
> of the suffix, for example, and that both in C and shell code.

I found another problem about matching control.

Z(4):akr@serein% Src/zsh -f
serein% bindkey -e; autoload -U compinit; compinit -D; compdef _tst tst
serein% _tst () { compadd -M 'r:|[:.]=* r:|=*' a.b:0 c.d:1 }
serein% tst :<TAB>

This inserts nothing now.

But 3.1.7-pre-2 inserts `.' as follows (and place the cursor on the
`.').

% tst .:
-- 
Tanaka Akira

Re: seg fault fixed and Makefile change

[Sven Wischnowsky]

Oliver Kiddle wrote:

> Sven Wischnowsky wrote:
> > 
> > I couldn't reproduce this (neither before nor after this patch -- your
> > match specs would have helped), but this looks suspiciously similar to
> > the one above... could you try with this patch?
> 
> That's fixed it: it now works fine, thanks.

Uff ;-)

> ...
> 
> Completion for netscape remote commands doesn't seem to work properly
> anymore:
> netscape -remote <tab>
> Basically, it is a bit too eager to put the \( suffix in. This did
> actually work before but I've always felt that this part of _netscape
> (which I am to blame for) is a bit of a nasty hack. Basically, the
> '(' suffix needs to be quoted unless it already is and that was the
> way I found which worked. 

Hm. What exactly doesn't seem to work properly for you anymore? The
only ugliness I can see is that you get the `\(' if the string is not
in quotes but when it is. Why not use:

  compadd -s '(' -S '' - ...

or

  local suf='('
  compquote suf
  compadd -qS "$s" - ...

and if the `compset -q' is really needed, put it after the compquote.

I don't really know all the things that are possible with -remote,
though, so these suggestions may not be usable(?).

Bye
 Sven


--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de