[net.bugs.4bsd] expr

das@cirl.UUCP (Dave Steffens) (06/09/86)

Subject: expr(1) fails on negative arguments (with fix)

Index: bin/expr.y

Description:
	expr(1) fails to handle negative arguments correctly.
	In particular, arithmetic operators will not accept
	negative arguments.  Additionally,  comparison operators
	use string comparison instead of arithmetic comparison
	if the second argument is negative.

	Additional fixes are included to eliminate the "illegal pointer
	integer combination" warnings from the 4.2bsd C compiler.

	I have cross-posted because these bugs exist in the v7 version
	and probably in various 2.9bsd and SYSV versions as well.

	CAVEAT: There may be a **good reason** why expr was **not**
	made to handle negative arguments.  The author of expr **did**
	include code to permit the first argument of comparison operators
	to be negative, presumably to handle the case where a sub-expression
	returned a negative result, e.g. expr 0 - 1 \< 0 and the like.
	Therefore, this posting may describe a fix which will **break
	something useful**.  If anyone knows of an example, PLEASE POST IT.

	Note to v7 (2.9bsd?) users:  While comparing v7 and 4.2bsd sources,
	I discovered that the 4.2bsd version contains additional bug fixes
	not present in the v7 source I have.  I presume these fixes shouldn't
	be divulged to those who don't have a 4.2bsd license so I haven't
	included them in my net.bugs.v7 posting.  Anyone care to comment?

Repeat-By:
	Arithmetic operator fails to handle a negative argument:
		expr 0 + -1
		non-numeric argument

	String comparison instead of arithmetic comparison:
		expr -2 \> -1
		1
	** because "-2" is lexicographically greater than "-1" **

Fix:
	In the following fixes, the line numbers shown refer to
	the 4.2bsd source for expr.y whose sccsid is
	"@(#)expr.y	4.3 (Berkeley) 6/30/83".

***** To fix the arithmetic operators:
116c123
< 	if(!(ematch(r1, "[0-9]*$") && ematch(r2, "[0-9]*$")))
---
> 	if(!(ematch(r1, "-*[0-9]*$") && ematch(r2, "-*[0-9]*$")))

***** To fix the comparison operators:
97c104
< 	if(ematch(r1, "-*[0-9]*$") && ematch(r2, "[0-9]*$"))
---
> 	if(ematch(r1, "-*[0-9]*$") && ematch(r2, "-*[0-9]*$"))

***** To fix the compiler warnings:
2a3,9
> %union {
> 	char *s;
> }
> 
> %type  <s> expr
> %token <s> A_STRING
> 
4c11
< %token A_STRING SUBSTR LENGTH INDEX NOARG MATCH
---
> %token SUBSTR LENGTH INDEX NOARG MATCH
90c97
< 	yylval = p;
---
> 	yylval.s = p;

-- 
{harvard,mit-eddie,think}!eplunix!earvax!das	David Allan Steffens
243 Charles St., Boston, MA 02114		Eaton-Peabody Laboratory
(617) 523-7900 x2748				Mass. Eye & Ear Infirmary

ccb@decvax.UUCP (Charles C. Bennett) (06/11/86)

[]
Nice fix. Try "expr -5 * ---6". The problem cannot be correctly fixed w/out
going all out and implementing UMINUS is the yacc grammar. Any takers?

decvax!ccb
Chas Bennett
=============================================================================
2 kinda people: those who say "There are 2 kinds of people" and those who
don't  -- Sam Clemens.

hansen@pegasus.UUCP (Tony L. Hansen) (06/13/86)

<	expr(1) fails to handle negative arguments correctly.
<	In particular, arithmetic operators will not accept
<	negative arguments.  Additionally,  comparison operators
<	use string comparison instead of arithmetic comparison
<	if the second argument is negative.

Like so many other things, this bug was fixed in System V. My UNIX System
Vr2 machine gives the following answers:

<		expr 0 + -1
<		non-numeric argument
	-1
<		expr -2 \> -1
<		1
	0

< Nice fix. Try "expr -5 * ---6". The problem cannot be correctly fixed w/out
< going all out and implementing UMINUS is the yacc grammar. Any takers?

<		expr -5 \* ---6
	expr: non-numeric argument
<		expr -5 \* -6
	30

Sorry, but I haven't looked at the expr code to see what the real fix was.

					Tony Hansen
					ihnp4!pegasus!hansen

guy@sun.UUCP (06/15/86)

> < Nice fix. Try "expr -5 * ---6". The problem cannot be correctly fixed
> < w/out going all out and implementing UMINUS is the yacc grammar. Any
> < takers?

> (fixed in S5) Sorry, but I haven't looked at the expr code to see what
> the real fix was.

It may have been to rewrite the whole thing in C.  It had been so rewritten
as of S3, although the S3 version still maintained the old "index",
"substr", and "length" operators from V7, which were undocumented and caused
confusion if those names appeared as operands.  S5 ripped them out.
-- 
	Guy Harris
	{ihnp4, decvax, seismo, decwrl, ...}!sun!guy
	guy@sun.com (or guy@sun.arpa)