Bug in zsh-3.1.5

Bug in zsh-3.1.5

[Martin Birgmeier]

Hi -

First, thanks for a great program!

Now, the bug report: I am using zsh-3.1.5 on FreeBSD 2.2.7. Since zsh-3.1.3, there
is a bug in case statement processing. Relevant output goes here:

$ echo echo "${HOSTTYPE:-$OSTYPE}:${TERM}:${TTY}"       <--- informational
freebsd2.2.7:xterm:/dev/ttyp0

$ case a in                                             <--- works
a* | b* )
echo yes
;;
* )
echo no
;;
esac
yes

$ case "${HOSTTYPE:-$OSTYPE}:${TERM}:${TTY}" in         <--- works
freebsd*:xterm:* )
echo yes
;;
* )
echo no
;;
case> esac
yes

$ case "${HOSTTYPE:-$OSTYPE}:${TERM}:${TTY}" in        <--- DOES NOT WORK
freebsd*:xterm:* | freebsd*:xterms:* )
echo yes
;;
* )
echo no
;;
esac
no

It seems that the second choice of the first case label controls the truth
value of the expression!

$ case "${HOSTTYPE:-$OSTYPE}:${TERM}:${TTY}" in       <--- WORKS!
( freebsd*:xterm:* | freebsd*:xterms:* )
echo yes
;;
* )
echo no
;;
esac
yes

The last example shows that the problem must have something to do with
a new allowed syntax for case labels (leading parenthesis). Also, the
first example shows that the case statement is working if patterns are
quite simple.

I hope you can reproduce and fix this problem. I did not include
the output of "reporter" since I am quite confident that you don't
need it. If I am wrong, I'll mail it to you, just let me know.

Please note that I am not on the mailing list.

Best regards,

Martin

-- 
Martin Birgmeier

Vienna                            e-mail: Martin.Birgmeier@IEEE.org
Austria                                   Martin.Birgmeier@aon.at

Re: Bug in zsh-3.1.5

[Bart Schaefer]

On Nov 7, 10:20am, Martin Birgmeier wrote:
} Subject: Bug in zsh-3.1.5
}
} $ echo echo "${HOSTTYPE:-$OSTYPE}:${TERM}:${TTY}"       <--- informational
} freebsd2.2.7:xterm:/dev/ttyp0
} 
} $ case "${HOSTTYPE:-$OSTYPE}:${TERM}:${TTY}" in        <--- DOES NOT WORK
} freebsd*:xterm:* | freebsd*:xterms:* )
} echo yes
} ;;
} * )
} echo no
} ;;
} esac
} no
}
} It seems that the second choice of the first case label controls the truth
} value of the expression!

Actually, it's the first alternative in that label that is at fault, and
it only incidentally has to do with the new grouping syntax.

The pattern match fails in matchonce() after executing the code at about
line 2497.  The comment on line 2496 says:
	    /* optimisation when next pattern is not a closure */
The current pattern at this point is the "*" just to the left of the "|"
and the next pattern is the empty string.  The text being matched is
"/dev/ttyp0", the "freebsd2.2.7:xterm:" part already having correctly
matched.

Up at around line 2217, in doesmatch(), this condition failed:

	    if (STARP(c) && c->next &&
		!c->next->left && (looka = *c->next->str) &&
		!itok(looka)) {

because *(c->next->str) is 0 -- which means that even though STARP(c) is
true, it didn't consume "/dev/ttyp0" before calling matchonce().

Now, arguably the empty string ought to match anywhere, so one possible
fix is to change the condition above to:

	    if (STARP(c) && c->next && !c->next->left &&
		((looka = *c->next->str) && !itok(looka) || !looka)) {

However, I'm concerned that the real problem is that the pattern was not
parsed correctly, i.e. the trailing empty string should never have been
there in the first place; so I'm reluctant to recommend that change.  I
don't have time to re-learn the pattern-parsing code just now. :-/

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

PATCH: Re: Bug in zsh-3.1.5 `case' pattern matching

[Peter Stephenson]

I replied to this on Sunday but it looks like it didn't get to the
list, so I'm assuming it didn't get anywhere.

Martin Birgmeier wrote:
> $ echo echo "${HOSTTYPE:-$OSTYPE}:${TERM}:${TTY}"       <--- informational
> freebsd2.2.7:xterm:/dev/ttyp0
> 
> $ case "${HOSTTYPE:-$OSTYPE}:${TERM}:${TTY}" in        <--- DOES NOT WORK
> freebsd*:xterm:* | freebsd*:xterms:* )
> echo yes
> ;;
> * )
> echo no
> ;;
> esac
> no

The problem was that the pattern matcher didn't know that the `|' was
the end of the pattern, because the alternative after it was
completely separate.  The reason is that `|'s are usually buried
inside parentheses, so aren't the end of a pattern.  That was so in
the old `case' code, which just stuck an open parenthesis in front.
This makes a difference because it needs to know whether the final
part of the pattern, here `*', is forced to reach the end of the test
string.   So Bart's suggestion that there was a parse error is correct.

Luckily the same problem already turned up with `~', the exclusion
pattern, so fixing it with `|' is easy.

*** Src/glob.c.bar	Sun Nov  8 16:03:29 1998
--- Src/glob.c	Sun Nov  8 16:01:28 1998
***************
*** 594,600 ****
  		 pptr[1] && pptr[1] != Outpar && pptr[1] != Bar) ||
  		*pptr == Outpar) {
  		if (*pptr == '/' || !*pptr ||
! 		    (isset(EXTENDEDGLOB) && *pptr == Tilde &&
  		     (gflag & GF_TOPLEV)))
  		    c->stat |= C_LAST;
  		return c;
--- 594,601 ----
  		 pptr[1] && pptr[1] != Outpar && pptr[1] != Bar) ||
  		*pptr == Outpar) {
  		if (*pptr == '/' || !*pptr ||
! 		    ((*pptr == Bar ||
! 		      (isset(EXTENDEDGLOB) && *pptr == Tilde)) &&
  		     (gflag & GF_TOPLEV)))
  		    c->stat |= C_LAST;
  		return c;
***************
*** 746,752 ****
      }
      /* mark if last pattern component in path component or pattern */
      if (*pptr == '/' || !*pptr ||
! 	(isset(EXTENDEDGLOB) && *pptr == Tilde && (gflag & GF_TOPLEV)))
  	c->stat |= C_LAST;
      c->str = dupstrpfx(cstr, pptr - cstr);
      return c;
--- 747,754 ----
      }
      /* mark if last pattern component in path component or pattern */
      if (*pptr == '/' || !*pptr ||
! 	((*pptr == Bar ||
! 	 (isset(EXTENDEDGLOB) && *pptr == Tilde)) && (gflag & GF_TOPLEV)))
  	c->stat |= C_LAST;
      c->str = dupstrpfx(cstr, pptr - cstr);
      return c;

-- 
Peter Stephenson <pws@ibmth.df.unipi.it>       Tel: +39 050 844536
WWW:  http://www.ifh.de/~pws/
Dipartimento di Fisica, Via Buonarotti 2, 56100 Pisa, Italy