Bugs with exclusion using file paths.

Bugs with exclusion using file paths.

[Peter Stephenson]

Fixes for the bugs I mentioned earlier.  For globbing, a top level ~
is handled specially, leading to these two.  Getting exclusion to work
properly is extremely complicated, so I can't promise this is the last of
it.  Maybe someone out there knows better algorithms.


1. Some idiot messed up the exclusion of absolute paths when he was optimizing
the pattern matching code.

5:32% print /*
/bin /boot /data /dev /etc /home /lib /lost+found /mnt /opt /proc /root /sbin /tmp /usr /var /windows
5:32% print /*~/b*
/*~/b*

The problem is that / is treated as an end of path segment, which it isn't
in the exclusion part, so it first parses /*~ as a string, then finds
garbage at the end.


2. print **/*~(.)# dumped core

This is because the pattern is handled in two parts.  The path before the
last slash is tried first, then the final bit is matched against whatever's
in the directory.  However, the exclusion is supposed to exclude the entire
pattern, treating / as an ordinary character.  Hence the pre-path has to be
prepended for the exclusion matching only.  I forgot to do this with the
pointer to the start of the pattern, so pointer arithmetic was off.  I
didn't fix up the variable holding the total length of the pattern, either.
This is all a recipe for disaster, and I love to know how else to do it.

We need some real globbing tests to pick up this sort of thing; pattern
matching in tests is insensitive to it.


Index: Src/pattern.c
===================================================================
RCS file: /home/pws/CVSROOT/projects/zsh/Src/pattern.c,v
retrieving revision 1.6
diff -u -r1.6 pattern.c
--- Src/pattern.c	2000/02/23 18:29:27	1.6
+++ Src/pattern.c	2000/03/26 22:05:05
@@ -486,7 +486,8 @@
 
     while (*patparse == Bar ||
 	   (isset(EXTENDEDGLOB) && *patparse == Tilde &&
-	    !memchr(patendseg, patparse[1], patendseglen))) {
+	    (patparse[1] == '/' ||
+	     !memchr(patendseg, patparse[1], patendseglen)))) {
 	int tilde = *patparse++ == Tilde;
 	long gfnode = 0, newbr;
 
@@ -634,7 +635,7 @@
 
     starter = chain = 0;
     while (!memchr(patendseg, *patparse, patendseglen) ||
-	   (*patparse == Tilde &&
+	   (*patparse == Tilde && patparse[1] != '/' &&
 	    memchr(patendseg, patparse[1], patendseglen))) {
 	if (isset(EXTENDEDGLOB) &&
 	    ((!isset(SHGLOB) &&
@@ -811,6 +812,7 @@
 	 */
 	if (kshchar || (memchr(patendstr, *patparse, patendstrlen) &&
 			(*patparse != Tilde ||
+			 patparse[1] == '/' ||
 			 !memchr(patendseg, patparse[1], patendseglen))))
 	    break;
 
@@ -1754,7 +1756,8 @@
 			while ((ret = patmatch(P_OPERAND(scan)))) {
 			    unsigned char *syncpt;
 			    char savchar, *testptr;
-			    int savforce = forceerrs;
+			    char *savpatinstart = patinstart;
+			    int savforce = forceerrs, savpatinlen = patinlen;
 			    forceerrs = -1;
 			    savglobdots = globdots;
 			    matchederrs = errsfound;
@@ -1799,7 +1802,8 @@
 					zalloc(pathpos + patinlen);
 				    strcpy(buf, pathbuf);
 				    strcpy(buf + pathpos, patinput);
-				    patinput = buf;
+				    patinput = patinstart = buf;
+				    patinlen += pathpos;
 				}
 				if (patmatch(opnd)) {
 				    ret = 0;
@@ -1810,8 +1814,11 @@
 				     */
 				    parsfound = savparsfound;
 				}
-				if (buf)
+				if (buf) {
 				    zfree(buf, pathpos + patinlen);
+				    patinstart = savpatinstart;
+				    patinlen = savpatinlen;
+				}
 				if (!ret)
 				    break;
 				next = PATNEXT(next);

-- 
Peter Stephenson <pws@pwstephenson.fsnet.co.uk>

Re: Bugs with exclusion using file paths.

[Bart Schaefer]

On Mar 26, 11:17pm, Peter Stephenson wrote:
} Subject: Bugs with exclusion using file paths.
}
} 1. Some idiot messed up the exclusion of absolute paths
} 2. print **/*~(.)# dumped core
} 
} We need some real globbing tests to pick up this sort of thing; pattern
} matching in tests is insensitive to it.

Here's a patch to 11glob.ztst .. but like Sven I get piles of BUG: output,
so some of the other tests fail as well, and on top of that the **/~(.)#
test still dumps core for me even after applying 10284.  And even before
applying 10284, I *sometimes* got success, and other times got core dumps,
from the /*~/* test.

Index: Test/11glob.ztst
===================================================================
@@ -3,6 +3,13 @@
 %prep
   globtest () { $ZTST_testdir/../Src/zsh -f $ZTST_srcdir/../Misc/$1 }
 
+  regress_absolute_path_and_core_dump() {
+    local absolute_srcdir=$(cd $ZTST_srcdir/.. && pwd -P) || return 1
+    setopt localoptions extendedglob nonomatch
+    print $absolute_srcdir/*~/*
+    print $absolute_srcdir/**/*~(.)#
+  }
+
 %test
 
   globtest globtests
@@ -234,3 +241,6 @@
 >0:  [[ FOO = @(bar|(#i)foo) ]]
 >0:  [[ Modules = (#i)*m* ]]
 >0 tests failed.
+
+  ( regress_absolute_path_and_core_dump )
+0:exclusions regression test

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

Re: Bugs with exclusion using file paths.

[Bart Schaefer]

On Mar 28,  9:48pm, Peter Stephenson wrote:
> Subject: Re: Bugs with exclusion using file paths.
> 
> Let me know if there is still a problem after this.

I now get no core dumps, no BUG:s, and the same results from both dynamic
and static links (which I'm not sure I was getting before, strangely).

A somewhat more complete patch for 11glob.ztst seems to be in order.  This
goes on top of my last patch.  I guess it turns out to be a multios test
as well; hmm.

This patch also fixes a problem I inadvertently found with the test
for completion, which is that if TERM is set to a value which causes ZLE
to be disabled, the test hangs forever.  I chose to force TERM=vt100,
but if anyone can think of a safer choice ...

Index: Test/11glob.ztst
===================================================================
@@ -1,13 +1,20 @@
 # Tests for globbing
 
 %prep
+  mkdir glob.tmp
+  mkdir glob.tmp/dir1
+  mkdir glob.tmp/dir2
+  : >glob.tmp/{,{dir1,dir2}/}{a,b,c}
+
   globtest () { $ZTST_testdir/../Src/zsh -f $ZTST_srcdir/../Misc/$1 }
 
   regress_absolute_path_and_core_dump() {
-    local absolute_srcdir=$(cd $ZTST_srcdir/.. && pwd -P) || return 1
-    setopt localoptions extendedglob nonomatch
-    print $absolute_srcdir/**/*~/*
-    print $absolute_srcdir/**/*~(.)#
+    local absolute_dir=$(cd glob.tmp && pwd -P)
+    [[ -n $absolute_dir ]] || return 1
+    setopt localoptions extendedglob nullglob
+    print $absolute_dir/**/*~/*
+    setopt nonullglob nomatch
+    print glob.tmp/**/*~(.)#
   }
 
 %test
@@ -244,3 +251,5 @@
 
   ( regress_absolute_path_and_core_dump )
 0:exclusions regression test
+>
+>glob.tmp/a glob.tmp/b glob.tmp/c glob.tmp/dir1 glob.tmp/dir1/a glob.tmp/dir1/b glob.tmp/dir1/c glob.tmp/dir2 glob.tmp/dir2/a glob.tmp/dir2/b glob.tmp/dir2/c
Index: Test/53completion.ztst
===================================================================
@@ -3,7 +3,8 @@
 %prep
   zmodload -i zsh/zpty
 
-  export ZTST_testdir ZTST_srcdir
+  TERM=vt100
+  export ZTST_testdir ZTST_srcdir TERM
   comptest () { $ZTST_testdir/../Src/zsh -f $ZTST_srcdir/comptest -z $ZTST_testdir/../Src/zsh -d $ZTST_testdir/compdump.tmp "$@" }
 
   mkdir comp.tmp

Re: Bugs with exclusion using file paths.

[Sven Wischnowsky]

Peter Stephenson wrote:

> ...
> @@ -1810,8 +1814,11 @@
>  				     */
>  				    parsfound = savparsfound;
>  				}
> -				if (buf)
> +				if (buf) {
>  				    zfree(buf, pathpos + patinlen);
> +				    patinstart = savpatinstart;
> +				    patinlen = savpatinlen;
> +				}
>  				if (!ret)
>  				    break;
>  				next = PATNEXT(next);

With something like **/_*~*~ I now get lots of:

  BUG: attempt to free more than allocated.

warnings. I haven't grokked all this pattern.c stuff, so I can't send
a patch (I don't even know if the problem is just with the second arg
to zfree() or if this shows a real bug).

Bye
 Sven


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

Re: Bugs with exclusion using file paths.

[Peter Stephenson]

Sven Wischnowsky wrote:
> > +				if (buf) {
> >  				    zfree(buf, pathpos + patinlen);
> > +				    patinstart = savpatinstart;
> > +				    patinlen = savpatinlen;
> 
> With something like **/_*~*~ I now get lots of:
> 
>   BUG: attempt to free more than allocated.
> 
> warnings.

Very stupid, sorry; use the original pattern length, not the temporrary
one.  For boring technical reasons due to the way my front-end to configure
is run, I didn't have Zsh memory allocation on, or I would presumably have
picked it up.

Let me know if there is still a problem after this.  I tried **/*~(.)# in
my home directory, with zsh-mem turned on, which ought to have tickled any
bug of this kind.

Index: Src/pattern.c
===================================================================
RCS file: /home/pws/CVSROOT/projects/zsh/Src/pattern.c,v
retrieving revision 1.7
diff -u -r1.7 pattern.c
--- Src/pattern.c	2000/03/27 21:40:57	1.7
+++ Src/pattern.c	2000/03/28 20:16:50
@@ -1815,9 +1815,9 @@
 				    parsfound = savparsfound;
 				}
 				if (buf) {
-				    zfree(buf, pathpos + patinlen);
 				    patinstart = savpatinstart;
 				    patinlen = savpatinlen;
+				    zfree(buf, pathpos + patinlen);
 				}
 				if (!ret)
 				    break;

-- 
Peter Stephenson <pws@pwstephenson.fsnet.co.uk>
Work: pws@CambridgeSiliconRadio.com
Web: http://www.pwstephenson.fsnet.co.uk