completion matching problem with 4.3.8

completion matching problem with 4.3.8

[Oliver Kiddle]

There seems to be a problem with matching control.

touch PacketDefinitionSAXHandler.h PacketListBox.cpp PacketListBox.h PacketsDialog.h
zsh -f
autoload -U compinit; compinit
zstyle ':completion:*' matcher-list 'm:{a-z}={A-Z}'
: P<tab>
  --> Completes to 'Packets' instead of just 'Packet'

Oliver

Re: completion matching problem with 4.3.8

[Peter Stephenson]

On Thu, 30 Oct 2008 12:28:50 +0100
Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> There seems to be a problem with matching control.
> 
> touch PacketDefinitionSAXHandler.h PacketListBox.cpp PacketListBox.h PacketsDialog.h
> zsh -f
> autoload -U compinit; compinit
> zstyle ':completion:*' matcher-list 'm:{a-z}={A-Z}'
> : P<tab>
>   --> Completes to 'Packets' instead of just 'Packet'

Doesn't appear to be happening for me with the latest latest code.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070

Re: completion matching problem with 4.3.8

[Oliver Kiddle]

Peter wrote:
> Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> > There seems to be a problem with matching control.
> Doesn't appear to be happening for me with the latest latest code.

I've just checked and the problem doesn't occur on Linux. It does,
however, occur on at least Solaris 10 5/08 sparc using the latest zsh
from cvs, compiled with Sun Studio 12. Do you still have access to a
Solaris system?

I'll try to verify whether the problem occurs with a gcc build and on
OpenSolaris/x86, Solaris 8 etc.

Oliver

Re: completion matching problem with 4.3.8

[Peter Stephenson]

On Thu, 30 Oct 2008 14:13:32 +0100
Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> Peter wrote:
> > Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> > > There seems to be a problem with matching control.
> > Doesn't appear to be happening for me with the latest latest code.
> 
> I've just checked and the problem doesn't occur on Linux. It does,
> however, occur on at least Solaris 10 5/08 sparc using the latest zsh
> from cvs, compiled with Sun Studio 12. Do you still have access to a
> Solaris system?
> 
> I'll try to verify whether the problem occurs with a gcc build and on
> OpenSolaris/x86, Solaris 8 etc.

I couldn't see a problem with Solaris 8 and gcc, which is what I've got.

It would be useful to know if it showed up in non-file completions such as

_tst() {
  compadd -M 'm:{a-z}={A-Z}' -- \
  PacketDefinitionSAXHandler.h PacketListBox.cpp \
  PacketListBox.h PacketsDialog.h;
}

since the code to add matchers for file completion is separate and I've
changed it recently.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070

Re: completion matching problem with 4.3.8

[Oliver Kiddle]

Peter wrote:
> I couldn't see a problem with Solaris 8 and gcc, which is what I've got.

I've tried building with Sun Studio 12 on Linux and there's no problem
there. My gcc build is failing on configure but I'll let you know.
 
> It would be useful to know if it showed up in non-file completions such as

Yes, problem also occurs with that function. It also occurs with the
following:

zsh -f
compctl -M 'm:{a-z}={A-Z}' -k '(PacketDefinitionSAXHandler.h PacketListBox.cpp PacketListBox.h PacketsDialog.h)' tst

Oliver

Re: completion matching problem with 4.3.8

[Oliver Kiddle]

I wrote:
> My gcc build is failing on configure but I'll let you know.

Turns out it was finding libgdbm in /opt/csw but needed the rpath
setting up.

Anyway, the gcc built zsh also has the problem so it isn't compiler
related. I tried running the minimal zsh -f, compctl test on Linux
through valgrind and I get the following message. Maybe valgrind has
found the source of the problem:

==647== Conditional jump or move depends on uninitialised value(s)
==647==    at 0x66E0ADD: pattern_match_restrict (compmatch.c:1276)
==647==    by 0x66E1781: bld_line (compmatch.c:1811)
==647==    by 0x66E26D4: join_sub (compmatch.c:2123)
==647==    by 0x66E2E96: join_psfx (compmatch.c:2362)
==647==    by 0x66E42C4: join_clines (compmatch.c:2805)
==647==    by 0x66DAF04: add_match_data (compcore.c:2879)
==647==    by 0x68FA4BE: addmatch (compctl.c:2020)
==647==    by 0x6900018: makecomplistflags (compctl.c:3629)
==647==    by 0x68FBBCF: makecomplistlist (compctl.c:2595)
==647==    by 0x68FBAA6: makecomplistor (compctl.c:2559)
==647==    by 0x68FBA30: makecomplistcc (compctl.c:2536)
==647==    by 0x68FB864: makecomplistcmd (compctl.c:2492)

Valgrind also finds some leaked memory from parse_class (complete.c:467)
with the same test.

Oliver

Re: completion matching problem with 4.3.8

[Peter Stephenson]

On Thu, 30 Oct 2008 16:43:40 +0100
Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> Anyway, the gcc built zsh also has the problem so it isn't compiler
> related. I tried running the minimal zsh -f, compctl test on Linux
> through valgrind and I get the following message. Maybe valgrind has
> found the source of the problem:
>...
> Valgrind also finds some leaked memory from parse_class (complete.c:467)
> with the same test.

This should keep valgrind happy.  I can't see any way these changes can
be wrong, but I've said that before.

Unfortunately, when I do this, the original problem (showing too many
characters in an ambiguous completion) now crops up.  This will take
*massively* more time to fix than the hour and a half I've spent so far.

Index: Src/Zle/complete.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/complete.c,v
retrieving revision 1.42
diff -u -r1.42 complete.c
--- Src/Zle/complete.c	23 Sep 2008 19:33:26 -0000	1.42
+++ Src/Zle/complete.c	30 Oct 2008 20:14:21 -0000
@@ -464,7 +464,7 @@
      * As the input string is metafied, but shouldn't contain shell
      * tokens, we can just add our own tokens willy nilly.
      */
-    optr = p->u.str = zalloc((optr-iptr) + 1);
+    optr = p->u.str = zhalloc((optr-iptr) + 1);
 
     while (firsttime || *iptr != endchar) {
 	int ch;
Index: Src/Zle/compmatch.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/compmatch.c,v
retrieving revision 1.59
diff -u -r1.59 compmatch.c
--- Src/Zle/compmatch.c	12 Oct 2008 19:28:10 -0000	1.59
+++ Src/Zle/compmatch.c	30 Oct 2008 20:14:22 -0000
@@ -1687,6 +1687,7 @@
     llen = mp->llen;
     rl = 0;
 
+    *line = '\0';
     if (sfx)
     {
 	/*


-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: completion matching problem with 4.3.8

[Peter Stephenson]

On Thu, 30 Oct 2008 12:28:50 +0100
Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> There seems to be a problem with matching control.
> 
> touch PacketDefinitionSAXHandler.h PacketListBox.cpp PacketListBox.h PacketsDialog.h
> zsh -f
> autoload -U compinit; compinit
> zstyle ':completion:*' matcher-list 'm:{a-z}={A-Z}'
> : P<tab>
>   --> Completes to 'Packets' instead of just 'Packet'

(Well, took an hour and three quarter or so.)

This *might* be the fix to the original bug.

The valgrind error in pattern_match_restrict() about uninitialised
memory was because at that point the memory is deliberately
uninitialised---we're building up the matched line.  We should therefore
be testing for the case where we're doing that (prestrict != NULL).  If
we get to the end and prestrict is not NULL, we haven't succeeded in
building the line because we haven't fulfilled all the restrictions, so
we should return 0, which is the fix for the bug above, perhaps.

I deliberately left in some debug tests that I added while tracking this
down.  You get file and line number; anything else is superflous here.

Someone should try some hairy matching just to make sure.

Index: Src/Zle/compmatch.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/compmatch.c,v
retrieving revision 1.60
diff -u -r1.60 compmatch.c
--- Src/Zle/compmatch.c	30 Oct 2008 20:29:56 -0000	1.60
+++ Src/Zle/compmatch.c	30 Oct 2008 21:54:59 -0000
@@ -156,6 +156,7 @@
     r->next = NULL;
     r->line = l; r->llen = ll;
     r->word = w; r->wlen = wl;
+    DPUTS(wl > 0 && !*w, "Bad word");
     r->orig = o; r->olen = ol;
     r->slen = 0;
     r->flags = fl;
@@ -416,6 +417,7 @@
     } else {
 	lp->line = l; lp->llen = wl;
 	lp->word = w; lp->wlen = wl;
+	DPUTS(wl > 0 && !*w, "Bad word");
 	lp->orig = o; lp->olen = ol;
     }
     if (o || ol)
@@ -1242,7 +1244,7 @@
 
 /*
  * Check if the given pattern matches the given string.
- *  p and  s are either anchor or line pattern and string;
+ * p and  s are either anchor or line pattern and string;
  * wp and ws are word (candidate) pattern and string
  *
  * If only one pattern is given, we just check if characters match.
@@ -1273,7 +1275,7 @@
     int wc, wind;
     int len = 0, wlen, mt, wmt;
 
-    while (p && wp && *s && *ws) {
+    while (p && wp && (prestrict || *s) && *ws) {
 	/* First test the word character */
 	if (*ws == Meta) {
 	    wc = STOUC(ws[1]) ^ 32;
@@ -1387,7 +1389,7 @@
 	wp = wp->next;
     }
 
-    while (p && *s) {
+    while (p && (prestrict || *s)) {
 	if (prestrict) {
 	    /*
 	     * As above, but with even less info to go on.
@@ -1438,6 +1440,11 @@
 	    s += len;
     }
 
+    if (prestrict) {
+	/* Restriction with nothing to match */
+	return 0;
+    }
+
     while (wp && *ws) {
 	/* No funny business when we only have the word pattern. */
 	if (*ws == Meta) {
@@ -2057,6 +2064,7 @@
     } else if (md->len != md->olen) {
 	r->wlen = md->len;
 	r->word = md->str - (sfx ? md->len : 0);
+	DPUTS(r->wlen > 0 && !*r->word, "Bad word");
     }
     return r;
 }


-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: completion matching problem with 4.3.8

[Oliver Kiddle]

Peter Stephenson wrote:
> (Well, took an hour and three quarter or so.)
> 
> This *might* be the fix to the original bug.

Thanks Peter. I've rebuilt on Solaris 10 now and it is working again.

Oliver