expr(1) fails on negative arguments (with fix)
Dave Steffens
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
Tony L. Hansen
< 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
g...@sun.uucp
> < 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
g...@sun.com (or g...@sun.arpa)
Guy Harris
> > < 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.
Unfortunately, they did NOT rip out the "match" operator, which is a prefix
synonym for the infix ":" operator. As such,
expr match : "[a-z]atch"
gets a "syntax error" message, while
expr hatch : "[a-z]atch"
doesn't. The person who deleted them must have been confused, and figured
"match" had to stay in because its infix equivalent was still there.
Of course, even if you rip "match" out,
expr : : "[a-z]atch"
still gets a "syntax error", so "expr" can't do mattern matching on
arbitrary strings. Be warned.