Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 01 Jul 2011 21:51:17 +0200
From:      Stefan Esser <se@freebsd.org>
To:        freebsd-standards@freebsd.org
Cc:        Garrett Wollman <wollman@csail.mit.edu>
Subject:   New patches for expr (was: Re: Fwd: [RFC] Consistent numeric range for "expr" on all architectures)
Message-ID:  <4E0E2535.1060204@freebsd.org>
In-Reply-To: <20110701092909.F1164@besplex.bde.org>
References:  <4E09AF8E.5010509@freebsd.org> <4E0B860E.50504@freebsd.org> <20110630164653.GA82980@zim.MIT.EDU> <4E0CACFB.8040906@freebsd.org> <19980.47094.184089.398324@khavrinen.csail.mit.edu> <20110701092909.F1164@besplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------060600010302060201060806
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

On 01.07.2011 01:58, Bruce Evans wrote:
[...]
> The part about recognizing different syntaxes is an extension and
> requires explicit permission.  The part about "using" a rank larger
> than that of signed long requires no explicit permission, since it
> only affects the result if there is overflow, but then the behaviour
> is undefined.  So any utility can do the latter.

I have attached a new patch which does the following for overflow:

1) Numeric range is extended to intmax_t
2) Overflow is caught in all arithmetic operations
3) Integers not used in arithmetic operations are treated like strings

> Found another bug in FreeBSD expr: it accepts leading whitespace in
> integers (since strto*() does), but the above doesn't allow this.
> Leading whitespace might even be useful for turning numbers into
> strings (it is sort of the reverse of turning strings into numbers
> in awk and even in expr by adding 0 to them).  The FreeBSD man page
> of course gives null detail about this requirement of the syntax.

4) Leading whitespace in numbers is only accepted in compat mode
   ("expr -e" or EXPR_COMPAT defined in the process environment or
   check_utility_compat("expr") == true).

<<<<<< Aside: Is check_utility_compat() still used at all?

It provides compatibility with FreeBSD-4 if the environment variable
_COMPAT_FreeBSD_4 exists and contains "expr" as member of a comma
separated list, or if the following symlink exists in /etc:

	compat-FreeBSD-4-util -> expr

In fact, the symlink is also parsed as comma separated list, but expr is
the only program in FreeBSD that calls check_utility_compat() ...

Since expr is the only user of check_utility_compat() and the same
effect can be had by specifying EXPR_COMPAT, the check_utility_compat
function should be considered deprecated and only be kept in libc for
backwards compatibility of old binaries, IMHO. This function adds one
unnecessary call of getenv() and one of readlink() per invocation of
expr and adds complexity to the man-page for expr.

>>>>>>>

> Found another bug of commission in the man page:
> 
> %      Arithmetic operations are performed using signed integer math. If the -e
> %      flag is specified, arithmetic uses the C intmax_t data type (the largest
> %      integral type available), and expr will detect arithmetic overflow and
> %      return an error indication.  If a numeric operand is specified which is
> %      so large as to overflow conversion to an integer, it is parsed as a
> %      string instead.  If -e is not specified, arithmetic operations and pars-
> %      ing of integer arguments will overflow silently according to the rules of
> %      the C standard, using the long data type.
> 
> The rule of the C standard is that overflow gives undefined behaviour, not
> silence.

I have prepared a number of patches, which I attach to this message. All
patches are relative to the current SVN version of expr.y (i.e. do *not*
try to apply the higher numbered ones on a patched source file).

The first file (expr.y.diff1) contains a "diff -w -u3" of the functional
changes that I suggest. It is only meant to show the actual code changes
(applying this diff results in mis-indented code).

The second file (expr.y.diff2) has been created with the same command
and contains some refactoring of functions. It is functionally identical
to the first version.

The third file (expr.y.diff3) contains the diff with whitespace and
style changes that I suggest as final version. I plan to commit the
changes in sequence (with correct indentation of each step, of course),
in order to keep the change history separate for each intended purpose.

The final version is some 40 lines (plus some 25 lines of stripped
comments and whitespace) shorter than the current version (607 vs. 674
lines of code) and it complies with style(9), AFAICT.

I also include a SHAR of regression tests meant to be committed to
/usr/src/tools/regression/bin/expr. All tests pass with the current
patched version of expr (while the currently shipped version dumps core
in one test case).

Please let me know, whether there are any objections to the
functionality and the patches. If there are no strong objections, then I
intend to commit the changes on July 6th.

I plan to commit all three versions in short sequence (not by applying
the patches, as explained above they are created with "-w" for a minimal
diff and therefore lead to wrong indentation).

I also plan to commit the regression tests. Suggestions for further
tests are welcome. I know that tests for comparison operators are mostly
missing and plan to add a few (especially string vs. decimal), but
perhaps you want to suggest special boundary cases for the arithmetic tests.

There have been comments regarding the wording of the man page and I
want to commit an updated man page with the source file. But before
finalizing the man page, I want to have finalized the functionality to
be described in the man page ...

Best regards, STefan

--------------060600010302060201060806
Content-Type: text/plain;
 name="expr-test.sh"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="expr-test.sh"

# This is a shell archive.  Save it in a file, remove anything before
# this line, and then unpack it by entering "sh file".  Note, it may
# create directories; files and directories will be owned by you and
# have default permissions.
#
# This archive contains:
#
#	expr/Makefile
#	expr/regress.sh
#	expr/regress.t
#
echo x - expr/Makefile
sed 's/^X//' >expr/Makefile << 'f9600fcce6c6ed7d48045c253fb4f463'
X# $FreeBSD: head/tools/regression/bin/test/Makefile 215022 2010-11-08 23:15:10Z jilles $
X
Xall:
X	sh regress.sh
f9600fcce6c6ed7d48045c253fb4f463
echo x - expr/regress.sh
sed 's/^X//' >expr/regress.sh << '78ebeeef8ff008d708ef3dfc31f2d4b1'
X#!/bin/sh
X
X#-
X# Copyright (c) 2011 Stefan Esser <se@freebsd.org>
X# All rights reserved. 
X#
X# Redistribution and use in source and binary forms, with or without
X# modification, are permitted provided that the following conditions
X# are met:
X# 1. Redistributions of source code must retain the above copyright
X#    notice, this list of conditions and the following disclaimer.
X# 2. Redistributions in binary form must reproduce the above copyright
X#    notice, this list of conditions and the following disclaimer in the
X#    documentation and/or other materials provided with the distribution.
X#
X# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
X# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
X# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
X# ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
X# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
X# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
X# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
X# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
X# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
X# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
X# SUCH DAMAGE.
X
X#
X# expr.sh - check if test(1) or builtin test works
X#
X# $FreeBSD:$
X
X# force a specified test program, e.g. `env test=/bin/expr sh regress.sh'
X: ${test=expr}
X
Xt ()
X{
X	# $1 -> exit code
X	# $2 -> result written to stdout
X	# $3 -> $test expression
X
X	count=$((count+1))
X	# check for syntax errors
X	output="`eval $test $3 2>/dev/null`"
X	ret=$?
X	if [ "$ret" -ne "$1" ]; then
X		printf "not ok %s - (got $ret, expected $1)\n" "$count $3"
X	elif [ "$output" != "$2" ]; then
X		printf "not ok %s - (unexpected output)\n" "$count $3"
X	else
X		printf "ok %s\n" "$count $3"
X	fi
X}
X
Xcount=0
Xecho "1..55"
X
Xt 0 xyzzy '\( xyzzy \)'
Xt 2 '' 'xyzzy + 0'
X
Xt 0 000100000 '000100000'
Xt 0 00010000000000000000000 '00010000000000000000000'
X
Xt 0 1 '0 + 1'
Xt 1 0 '0 + 0'
Xt 0 2 '1 + 1'
Xt 0 -2 '-- -1 + -1'
Xt 1 0 '-- -1 + 1'
Xt 2 '' '-'			# syntax error
Xt 2 '' '-1'			# syntax error
Xt 2 '' '-1 + 1'			# syntax error
Xt 1 0 '-- -1 + 1'
Xt 2 '' '" 	  1" + 1'	# syntax error
Xt 0 2 '-e " 	  1" + 1'	# allow leading white space with -e
Xt 0 1 '"" + 1'			# empty string is interpreted as 0
X
Xt 0 9223372036854775807 '9223372036854775807 + 0'
Xt 2 '' '9223372036854775807 + 1'
Xt 0 -9223372036854775808 '-- -9223372036854775807 - 1'
Xt 0 -9223372036854775808 '-- -9223372036854775808 \* 1'
Xt 2 '' '-- -9223372036854775808 \* -1'
Xt 0 9223372036854775807 '-- -9223372036854775807 \* -1'
Xt 0 10000000000000000000 '10000000000000000000'
Xt 2 '' '10000000000000000000 + 0'
Xt 0 100001 '000100000 + 1'
Xt 0 9223372036854775807 '4611686018427387904 - 1 - -4611686018427387904'
Xt 0 -9223372036854775808 '-- -4611686018427387904 + -4611686018427387904'
Xt 0 -9223372036854775808 '-- -9223372036854775808 / 1'
Xt 2 '' '-- -9223372036854775808 / -1'
Xt 0 -9223372036854775807 '9223372036854775807 / -1'
Xt 0 9223372036854775806 '4611686018427387903 \* 2'
Xt 2 '' '4611686018427387904 \* 2'
Xt 0 -9223372036854775806 '4611686018427387903 \* -2'
Xt 0 -9223372036854775808 '4611686018427387904 \* -2'
Xt 2 '' '4611686018427387905 \* -2'
Xt 0 -7 '-- -9223372036854775807 % 10'
Xt 0 -7 '-- -9223372036854775807 % -10'
Xt 0 7 '9223372036854775807 % 10'
Xt 0 7 '9223372036854775807 % -10'
Xt 0 64 '\( 2 \* 2 \) \* \( 2 \* 2 \) \* 2 \* 2'
Xt 0 10 '5 \* 7 - 5 \* 5'
Xt 0 50 '5 \* \( 7 - 5 \) \* 5'
X
Xt 0 abc '123abc123 : ".*\(a[^0-9]*\)"'
Xt 0 124 '123abc123 : ".*a[^0-9]*\(.*\)" + 1'
Xt 0 1 '123abc123 : ".*a[^0-9]*\(.*\)" = 123'
Xt 0 7 '123abc123 : ".*a[^0-9]*" + 1'
Xt 1 0 '123Abc123 : ".*a[^0-9]*"'
Xt 0 1 '123Abc123 : ".*a[^0-9]*" + 1'
Xt 0 1 '123Abc123 : ".*a[^0-9]*" = 0'
X
Xt 1 0 '2 + 0 "<" 1'
Xt 0 1 '0 "<" 1 + 2'
Xt 0 3 '\( 0 "<" 1 \) + 2'
X
XEXPR_COMPAT=1; export EXPR_COMPAT
X
Xt 0 -2 '-1 + -1'
Xt 1 0 '-1 + 1'
Xt 0 2 '" 1" + 1'
78ebeeef8ff008d708ef3dfc31f2d4b1
echo x - expr/regress.t
sed 's/^X//' >expr/regress.t << '85ccbaa69b7bc1caaef6199f47c0ae78'
X#!/bin/sh
X# $FreeBSD: head/tools/regression/bin/test/regress.t 215022 2010-11-08 23:15:10Z jilles $
X
Xcd `dirname $0`
X
Xsh regress.sh
85ccbaa69b7bc1caaef6199f47c0ae78
exit


--------------060600010302060201060806
Content-Type: text/plain;
 name="expr.y.diff1"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="expr.y.diff1"

--- expr.y.ORIG	2011-07-01 20:22:29.119607269 +0200
+++ expr.y.NEW1	2011-07-01 20:27:35.146459819 +0200
@@ -42,13 +42,15 @@
 
 struct val *result;
 
+void		assert_to_integer(struct val *);
 int		chk_div(intmax_t, intmax_t);
 int		chk_minus(intmax_t, intmax_t, intmax_t);
 int		chk_plus(intmax_t, intmax_t, intmax_t);
 int		chk_times(intmax_t, intmax_t, intmax_t);
 void		free_value(struct val *);
-int		is_zero_or_null(struct val *);
+int		is_integer(const char *);
 int		isstring(struct val *);
+int		is_zero_or_null(struct val *);
 struct val	*make_integer(intmax_t);
 struct val	*make_str(const char *);
 struct val	*op_and(struct val *, struct val *);
@@ -65,7 +67,7 @@
 struct val	*op_plus(struct val *, struct val *);
 struct val	*op_rem(struct val *, struct val *);
 struct val	*op_times(struct val *, struct val *);
-intmax_t	to_integer(struct val *);
+int		to_integer(struct val *);
 void		to_string(struct val *);
 int		yyerror(const char *);
 int		yylex(void);
@@ -134,37 +136,16 @@
 make_str(const char *s)
 {
 	struct val *vp;
-	char *ep;
 
 	vp = (struct val *) malloc (sizeof (*vp));
 	if (vp == NULL || ((vp->u.s = strdup (s)) == NULL)) {
 		errx(ERR_EXIT, "malloc() failed");
 	}
 
-	/*
-	 * Previously we tried to scan the string to see if it ``looked like''
-	 * an integer (erroneously, as it happened).  Let strtoimax() do the
-	 * dirty work.  We could cache the value, except that we are using
-	 * a union and need to preserve the original string form until we
-	 * are certain that it is not needed.
-	 *
-	 * IEEE Std.1003.1-2001 says:
-	 * /integer/ An argument consisting only of an (optional) unary minus  
-	 *	     followed by digits.          
-	 *
-	 * This means that arguments which consist of digits followed by
-	 * non-digits MUST NOT be considered integers.  strtoimax() will
-	 * figure this out for us.
-	 */
-	if (eflag)
-		(void)strtoimax(s, &ep, 10);
+	if (is_integer(s))
+		vp->type = numeric_string;
 	else
-		(void)strtol(s, &ep, 10);
-
-	if (*ep != '\0')
 		vp->type = string;
-	else	
-		vp->type = numeric_string;
 
 	return vp;
 }
@@ -178,31 +159,33 @@
 }
 
 
-intmax_t
+int
 to_integer(struct val *vp)
 {
 	intmax_t i;
 
-	if (vp->type == integer)
-		return 1;
-
-	if (vp->type == string)
-		return 0;
-
-	/* vp->type == numeric_string, make it numeric */
+	/* we can only convert numeric_string to integer, here */
+	if (vp->type == numeric_string) {
 	errno = 0;
-	if (eflag) {
 		i  = strtoimax(vp->u.s, (char **)NULL, 10);
-		if (errno == ERANGE)
-			err(ERR_EXIT, NULL);
-	} else {
-		i = strtol(vp->u.s, (char **)NULL, 10);
-	}
-
+		/* just keep as numeric_string, if the conversion fails */
+		if (errno != ERANGE) {
 	free (vp->u.s);
 	vp->u.i = i;
 	vp->type = integer;
-	return 1;
+		}
+	}
+	return (vp->type == integer);
+}
+
+
+void
+assert_to_integer(struct val *vp)
+{
+	if (vp->type == string)
+		errx(ERR_EXIT, "not a decimal number: '%s'", vp->u.s);
+	if (!to_integer(vp))
+		errx(ERR_EXIT, "operand too large: '%s'", vp->u.s);
 }
 
 void
@@ -230,6 +213,20 @@
 
 
 int
+is_integer(const char *s)
+{
+	if (eflag)
+		while (isspace((unsigned char)*s))
+			s++;
+	if (*s == '-')
+		s++;
+	while (isdigit((unsigned char)*s))
+		s++;
+	return (*s == '\0');
+}
+
+
+int
 isstring(struct val *vp)
 {
 	/* only TRUE if this string is not a valid integer */
@@ -350,8 +347,8 @@
 		to_string (b);	
 		r = make_integer ((intmax_t)(strcoll (a->u.s, b->u.s) == 0));
 	} else {
-		(void)to_integer(a);
-		(void)to_integer(b);
+		assert_to_integer(a);
+		assert_to_integer(b);
 		r = make_integer ((intmax_t)(a->u.i == b->u.i));
 	}
 
@@ -370,8 +367,8 @@
 		to_string (b);
 		r = make_integer ((intmax_t)(strcoll (a->u.s, b->u.s) > 0));
 	} else {
-		(void)to_integer(a);
-		(void)to_integer(b);
+		assert_to_integer(a);
+		assert_to_integer(b);
 		r = make_integer ((intmax_t)(a->u.i > b->u.i));
 	}
 
@@ -390,8 +387,8 @@
 		to_string (b);
 		r = make_integer ((intmax_t)(strcoll (a->u.s, b->u.s) < 0));
 	} else {
-		(void)to_integer(a);
-		(void)to_integer(b);
+		assert_to_integer(a);
+		assert_to_integer(b);
 		r = make_integer ((intmax_t)(a->u.i < b->u.i));
 	}
 
@@ -410,8 +407,8 @@
 		to_string (b);
 		r = make_integer ((intmax_t)(strcoll (a->u.s, b->u.s) >= 0));
 	} else {
-		(void)to_integer(a);
-		(void)to_integer(b);
+		assert_to_integer(a);
+		assert_to_integer(b);
 		r = make_integer ((intmax_t)(a->u.i >= b->u.i));
 	}
 
@@ -430,8 +427,8 @@
 		to_string (b);
 		r = make_integer ((intmax_t)(strcoll (a->u.s, b->u.s) <= 0));
 	} else {
-		(void)to_integer(a);
-		(void)to_integer(b);
+		assert_to_integer(a);
+		assert_to_integer(b);
 		r = make_integer ((intmax_t)(a->u.i <= b->u.i));
 	}
 
@@ -450,8 +447,8 @@
 		to_string (b);
 		r = make_integer ((intmax_t)(strcoll (a->u.s, b->u.s) != 0));
 	} else {
-		(void)to_integer(a);
-		(void)to_integer(b);
+		assert_to_integer(a);
+		assert_to_integer(b);
 		r = make_integer ((intmax_t)(a->u.i != b->u.i));
 	}
 
@@ -479,17 +476,13 @@
 {
 	struct val *r;
 
-	if (!to_integer(a) || !to_integer(b)) {
-		errx(ERR_EXIT, "non-numeric argument");
-	}
+	assert_to_integer(a);
+	assert_to_integer(b);
 
-	if (eflag) {
 		r = make_integer(a->u.i + b->u.i);
 		if (chk_plus(a->u.i, b->u.i, r->u.i)) {
 			errx(ERR_EXIT, "overflow");
 		}
-	} else
-		r = make_integer((long)a->u.i + (long)b->u.i);
 
 	free_value (a);
 	free_value (b);
@@ -516,17 +509,13 @@
 {
 	struct val *r;
 
-	if (!to_integer(a) || !to_integer(b)) {
-		errx(ERR_EXIT, "non-numeric argument");
-	}
+	assert_to_integer(a);
+	assert_to_integer(b);
 
-	if (eflag) {
 		r = make_integer(a->u.i - b->u.i);
 		if (chk_minus(a->u.i, b->u.i, r->u.i)) {
 			errx(ERR_EXIT, "overflow");
 		}
-	} else
-		r = make_integer((long)a->u.i - (long)b->u.i);
 
 	free_value (a);
 	free_value (b);
@@ -550,17 +539,13 @@
 {
 	struct val *r;
 
-	if (!to_integer(a) || !to_integer(b)) {
-		errx(ERR_EXIT, "non-numeric argument");
-	}
+	assert_to_integer(a);
+	assert_to_integer(b);
 
-	if (eflag) {
 		r = make_integer(a->u.i * b->u.i);
 		if (chk_times(a->u.i, b->u.i, r->u.i)) {
 			errx(ERR_EXIT, "overflow");
 		}
-	} else
-		r = make_integer((long)a->u.i * (long)b->u.i);
 
 	free_value (a);
 	free_value (b);
@@ -583,21 +568,16 @@
 {
 	struct val *r;
 
-	if (!to_integer(a) || !to_integer(b)) {
-		errx(ERR_EXIT, "non-numeric argument");
-	}
+	assert_to_integer(a);
+	assert_to_integer(b);
 
 	if (b->u.i == 0) {
 		errx(ERR_EXIT, "division by zero");
 	}
-
-	if (eflag) {
-		r = make_integer(a->u.i / b->u.i);
 		if (chk_div(a->u.i, b->u.i)) {
 			errx(ERR_EXIT, "overflow");
 		}
-	} else
-		r = make_integer((long)a->u.i / (long)b->u.i);
+	r = make_integer(a->u.i / b->u.i);
 
 	free_value (a);
 	free_value (b);
@@ -609,19 +589,12 @@
 {
 	struct val *r;
 
-	if (!to_integer(a) || !to_integer(b)) {
-		errx(ERR_EXIT, "non-numeric argument");
-	}
-
+	assert_to_integer(a);
+	assert_to_integer(b);
 	if (b->u.i == 0) {
 		errx(ERR_EXIT, "division by zero");
 	}
-
-	if (eflag)
 		r = make_integer(a->u.i % b->u.i);
-	        /* chk_rem necessary ??? */
-	else
-		r = make_integer((long)a->u.i % (long)b->u.i);
 
 	free_value (a);
 	free_value (b);

--------------060600010302060201060806
Content-Type: text/plain;
 name="expr.y.diff2"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="expr.y.diff2"

--- expr.y.ORIG	2011-07-01 20:22:29.119607269 +0200
+++ expr.y.NEW2	2011-07-01 20:26:47.212460189 +0200
@@ -40,15 +40,19 @@
 	} u;
 } ;
 
+char		**av;
+int		eflag;
 struct val *result;
 
-int		chk_div(intmax_t, intmax_t);
-int		chk_minus(intmax_t, intmax_t, intmax_t);
-int		chk_plus(intmax_t, intmax_t, intmax_t);
-int		chk_times(intmax_t, intmax_t, intmax_t);
+void		assert_to_integer(struct val *);
+void		assert_div(intmax_t, intmax_t);
+void		assert_minus(intmax_t, intmax_t, intmax_t);
+void		assert_plus(intmax_t, intmax_t, intmax_t);
+void		assert_times(intmax_t, intmax_t, intmax_t);
 void		free_value(struct val *);
-int		is_zero_or_null(struct val *);
+int		is_integer(const char *);
 int		isstring(struct val *);
+int		is_zero_or_null(struct val *);
 struct val	*make_integer(intmax_t);
 struct val	*make_str(const char *);
 struct val	*op_and(struct val *, struct val *);
@@ -65,14 +69,12 @@
 struct val	*op_plus(struct val *, struct val *);
 struct val	*op_rem(struct val *, struct val *);
 struct val	*op_times(struct val *, struct val *);
-intmax_t	to_integer(struct val *);
+int		to_integer(struct val *);
 void		to_string(struct val *);
 int		yyerror(const char *);
 int		yylex(void);
 int		yyparse(void);
 
-static int	eflag;
-char **av;
 %}
 
 %union
@@ -134,37 +136,16 @@
 make_str(const char *s)
 {
 	struct val *vp;
-	char *ep;
 
 	vp = (struct val *) malloc (sizeof (*vp));
 	if (vp == NULL || ((vp->u.s = strdup (s)) == NULL)) {
 		errx(ERR_EXIT, "malloc() failed");
 	}
 
-	/*
-	 * Previously we tried to scan the string to see if it ``looked like''
-	 * an integer (erroneously, as it happened).  Let strtoimax() do the
-	 * dirty work.  We could cache the value, except that we are using
-	 * a union and need to preserve the original string form until we
-	 * are certain that it is not needed.
-	 *
-	 * IEEE Std.1003.1-2001 says:
-	 * /integer/ An argument consisting only of an (optional) unary minus  
-	 *	     followed by digits.          
-	 *
-	 * This means that arguments which consist of digits followed by
-	 * non-digits MUST NOT be considered integers.  strtoimax() will
-	 * figure this out for us.
-	 */
-	if (eflag)
-		(void)strtoimax(s, &ep, 10);
+	if (is_integer(s))
+		vp->type = numeric_string;
 	else
-		(void)strtol(s, &ep, 10);
-
-	if (*ep != '\0')
 		vp->type = string;
-	else	
-		vp->type = numeric_string;
 
 	return vp;
 }
@@ -178,31 +159,33 @@
 }
 
 
-intmax_t
+int
 to_integer(struct val *vp)
 {
 	intmax_t i;
 
-	if (vp->type == integer)
-		return 1;
-
-	if (vp->type == string)
-		return 0;
-
-	/* vp->type == numeric_string, make it numeric */
-	errno = 0;
-	if (eflag) {
+	/* we can only convert numeric_string to integer, here */
+	if (vp->type == numeric_string) {
+		errno = 0;
 		i  = strtoimax(vp->u.s, (char **)NULL, 10);
-		if (errno == ERANGE)
-			err(ERR_EXIT, NULL);
-	} else {
-		i = strtol(vp->u.s, (char **)NULL, 10);
+		/* just keep as numeric_string, if the conversion fails */
+		if (errno != ERANGE) {
+			free (vp->u.s);
+			vp->u.i = i;
+			vp->type = integer;
+		}
 	}
+	return (vp->type == integer);
+}
 
-	free (vp->u.s);
-	vp->u.i = i;
-	vp->type = integer;
-	return 1;
+
+void
+assert_to_integer(struct val *vp)
+{
+	if (vp->type == string)
+		errx(ERR_EXIT, "not a decimal number: '%s'", vp->u.s);
+	if (!to_integer(vp))
+		errx(ERR_EXIT, "operand too large: '%s'", vp->u.s);
 }
 
 void
@@ -230,6 +213,20 @@
 
 
 int
+is_integer(const char *s)
+{
+	if (eflag)
+		while (isspace((unsigned char)*s))
+			s++;
+	if (*s == '-')
+		s++;
+	while (isdigit((unsigned char)*s))
+		s++;
+	return (*s == '\0');
+}
+
+
+int
 isstring(struct val *vp)
 {
 	/* only TRUE if this string is not a valid integer */
@@ -350,8 +347,8 @@
 		to_string (b);	
 		r = make_integer ((intmax_t)(strcoll (a->u.s, b->u.s) == 0));
 	} else {
-		(void)to_integer(a);
-		(void)to_integer(b);
+		assert_to_integer(a);
+		assert_to_integer(b);
 		r = make_integer ((intmax_t)(a->u.i == b->u.i));
 	}
 
@@ -370,8 +367,8 @@
 		to_string (b);
 		r = make_integer ((intmax_t)(strcoll (a->u.s, b->u.s) > 0));
 	} else {
-		(void)to_integer(a);
-		(void)to_integer(b);
+		assert_to_integer(a);
+		assert_to_integer(b);
 		r = make_integer ((intmax_t)(a->u.i > b->u.i));
 	}
 
@@ -390,8 +387,8 @@
 		to_string (b);
 		r = make_integer ((intmax_t)(strcoll (a->u.s, b->u.s) < 0));
 	} else {
-		(void)to_integer(a);
-		(void)to_integer(b);
+		assert_to_integer(a);
+		assert_to_integer(b);
 		r = make_integer ((intmax_t)(a->u.i < b->u.i));
 	}
 
@@ -410,8 +407,8 @@
 		to_string (b);
 		r = make_integer ((intmax_t)(strcoll (a->u.s, b->u.s) >= 0));
 	} else {
-		(void)to_integer(a);
-		(void)to_integer(b);
+		assert_to_integer(a);
+		assert_to_integer(b);
 		r = make_integer ((intmax_t)(a->u.i >= b->u.i));
 	}
 
@@ -430,8 +427,8 @@
 		to_string (b);
 		r = make_integer ((intmax_t)(strcoll (a->u.s, b->u.s) <= 0));
 	} else {
-		(void)to_integer(a);
-		(void)to_integer(b);
+		assert_to_integer(a);
+		assert_to_integer(b);
 		r = make_integer ((intmax_t)(a->u.i <= b->u.i));
 	}
 
@@ -450,8 +447,8 @@
 		to_string (b);
 		r = make_integer ((intmax_t)(strcoll (a->u.s, b->u.s) != 0));
 	} else {
-		(void)to_integer(a);
-		(void)to_integer(b);
+		assert_to_integer(a);
+		assert_to_integer(b);
 		r = make_integer ((intmax_t)(a->u.i != b->u.i));
 	}
 
@@ -460,18 +457,16 @@
 	return r;
 }
 
-int
-chk_plus(intmax_t a, intmax_t b, intmax_t r)
+void
+assert_plus(intmax_t a, intmax_t b, intmax_t r)
 {
-
-	/* sum of two positive numbers must be positive */
-	if (a > 0 && b > 0 && r <= 0)
-		return 1;
-	/* sum of two negative numbers must be negative */
-	if (a < 0 && b < 0 && r >= 0)
-		return 1;
-	/* all other cases are OK */
-	return 0;
+	/*
+	 * sum of two positive numbers must be positive,
+	 * sum of two negative numbers must be negative
+	 */
+	if ((a > 0 && b > 0 && r <= 0) ||
+	    (a < 0 && b < 0 && r >= 0))
+		errx(ERR_EXIT, "overflow");
 }
 
 struct val *
@@ -479,36 +474,26 @@
 {
 	struct val *r;
 
-	if (!to_integer(a) || !to_integer(b)) {
-		errx(ERR_EXIT, "non-numeric argument");
-	}
+	assert_to_integer(a);
+	assert_to_integer(b);
 
-	if (eflag) {
-		r = make_integer(a->u.i + b->u.i);
-		if (chk_plus(a->u.i, b->u.i, r->u.i)) {
-			errx(ERR_EXIT, "overflow");
-		}
-	} else
-		r = make_integer((long)a->u.i + (long)b->u.i);
+	r = make_integer(a->u.i + b->u.i);
+	assert_plus(a->u.i, b->u.i, r->u.i);
 
 	free_value (a);
 	free_value (b);
 	return r;
 }
 
-int
-chk_minus(intmax_t a, intmax_t b, intmax_t r)
+void
+assert_minus(intmax_t a, intmax_t b, intmax_t r)
 {
 
 	/* special case subtraction of INTMAX_MIN */
-	if (b == INTMAX_MIN) {
-		if (a >= 0)
-			return 1;
-		else
-			return 0;
-	}
-	/* this is allowed for b != INTMAX_MIN */
-	return chk_plus (a, -b, r);
+	if (b == INTMAX_MIN && a < 0)
+		errx(ERR_EXIT, "overflow");
+	/* check addition of negative subtrahend */
+	assert_plus(a, -b, r);
 }
 
 struct val *
@@ -516,33 +501,26 @@
 {
 	struct val *r;
 
-	if (!to_integer(a) || !to_integer(b)) {
-		errx(ERR_EXIT, "non-numeric argument");
-	}
+	assert_to_integer(a);
+	assert_to_integer(b);
 
-	if (eflag) {
-		r = make_integer(a->u.i - b->u.i);
-		if (chk_minus(a->u.i, b->u.i, r->u.i)) {
-			errx(ERR_EXIT, "overflow");
-		}
-	} else
-		r = make_integer((long)a->u.i - (long)b->u.i);
+	r = make_integer(a->u.i - b->u.i);
+	assert_minus(a->u.i, b->u.i, r->u.i);
 
 	free_value (a);
 	free_value (b);
 	return r;
 }
 
-int
-chk_times(intmax_t a, intmax_t b, intmax_t r)
+void
+assert_times(intmax_t a, intmax_t b, intmax_t r)
 {
-	/* special case: first operand is 0, no overflow possible */
-	if (a == 0)
-		return 0;
-	/* verify that result of division matches second operand */
-	if (r / a != b)
-		return 1;
-	return 0;
+	/*
+	 * if first operand is 0, no overflow is possible,
+	 * else result of division test must match second operand
+	 */
+	if (a != 0 && r / a != b)
+		errx(ERR_EXIT, "overflow");
 }
 
 struct val *
@@ -550,32 +528,25 @@
 {
 	struct val *r;
 
-	if (!to_integer(a) || !to_integer(b)) {
-		errx(ERR_EXIT, "non-numeric argument");
-	}
+	assert_to_integer(a);
+	assert_to_integer(b);
 
-	if (eflag) {
-		r = make_integer(a->u.i * b->u.i);
-		if (chk_times(a->u.i, b->u.i, r->u.i)) {
-			errx(ERR_EXIT, "overflow");
-		}
-	} else
-		r = make_integer((long)a->u.i * (long)b->u.i);
+	r = make_integer(a->u.i * b->u.i);
+	assert_times(a->u.i, b->u.i, r->u.i);
 
 	free_value (a);
 	free_value (b);
 	return (r);
 }
 
-int
-chk_div(intmax_t a, intmax_t b)
+void
+assert_div(intmax_t a, intmax_t b)
 {
-	/* div by zero has been taken care of before */
+	if (b == 0)
+		errx(ERR_EXIT, "division by zero");
 	/* only INTMAX_MIN / -1 causes overflow */
 	if (a == INTMAX_MIN && b == -1)
-		return 1;
-	/* everything else is OK */
-	return 0;
+		errx(ERR_EXIT, "overflow");
 }
 
 struct val *
@@ -583,21 +554,12 @@
 {
 	struct val *r;
 
-	if (!to_integer(a) || !to_integer(b)) {
-		errx(ERR_EXIT, "non-numeric argument");
-	}
+	assert_to_integer(a);
+	assert_to_integer(b);
 
-	if (b->u.i == 0) {
-		errx(ERR_EXIT, "division by zero");
-	}
-
-	if (eflag) {
-		r = make_integer(a->u.i / b->u.i);
-		if (chk_div(a->u.i, b->u.i)) {
-			errx(ERR_EXIT, "overflow");
-		}
-	} else
-		r = make_integer((long)a->u.i / (long)b->u.i);
+	/* assert based on operands only, not on result */
+	assert_div(a->u.i, b->u.i);
+	r = make_integer(a->u.i / b->u.i);
 
 	free_value (a);
 	free_value (b);
@@ -609,19 +571,11 @@
 {
 	struct val *r;
 
-	if (!to_integer(a) || !to_integer(b)) {
-		errx(ERR_EXIT, "non-numeric argument");
-	}
-
-	if (b->u.i == 0) {
-		errx(ERR_EXIT, "division by zero");
-	}
-
-	if (eflag)
-		r = make_integer(a->u.i % b->u.i);
-	        /* chk_rem necessary ??? */
-	else
-		r = make_integer((long)a->u.i % (long)b->u.i);
+	assert_to_integer(a);
+	assert_to_integer(b);
+	/* pass a=1 to only check for div by zero */
+	assert_div(1, b->u.i);
+	r = make_integer(a->u.i % b->u.i);
 
 	free_value (a);
 	free_value (b);

--------------060600010302060201060806
Content-Type: text/plain;
 name="expr.y.diff3"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="expr.y.diff3"

--- expr.y.ORIG	2011-07-01 20:22:29.119607269 +0200
+++ expr.y.FINAL	2011-07-01 20:25:23.527484480 +0200
@@ -1,6 +1,6 @@
 %{
 /*-
- * Written by Pace Willisson (pace@blitz.com) 
+ * Written by Pace Willisson (pace@blitz.com)
  * and placed in the public domain.
  *
  * Largely rewritten by J.T. Conklin (jtc@wimsey.com)
@@ -21,7 +21,7 @@
 #include <string.h>
 #include <regex.h>
 #include <unistd.h>
-  
+
 /*
  * POSIX specifies a specific error code for syntax errors.  We exit
  * with this code for all errors.
@@ -40,15 +40,19 @@
 	} u;
 } ;
 
-struct val *result;
-
-int		chk_div(intmax_t, intmax_t);
-int		chk_minus(intmax_t, intmax_t, intmax_t);
-int		chk_plus(intmax_t, intmax_t, intmax_t);
-int		chk_times(intmax_t, intmax_t, intmax_t);
+char		**av;
+int		eflag;
+struct val	*result;
+
+void		assert_to_integer(struct val *);
+void		assert_div(intmax_t, intmax_t);
+void		assert_minus(intmax_t, intmax_t, intmax_t);
+void		assert_plus(intmax_t, intmax_t, intmax_t);
+void		assert_times(intmax_t, intmax_t, intmax_t);
 void		free_value(struct val *);
+int		is_integer(const char *);
+int		is_string(struct val *);
 int		is_zero_or_null(struct val *);
-int		isstring(struct val *);
 struct val	*make_integer(intmax_t);
 struct val	*make_str(const char *);
 struct val	*op_and(struct val *, struct val *);
@@ -65,14 +69,12 @@
 struct val	*op_plus(struct val *, struct val *);
 struct val	*op_rem(struct val *, struct val *);
 struct val	*op_times(struct val *, struct val *);
-intmax_t	to_integer(struct val *);
+int		to_integer(struct val *);
 void		to_string(struct val *);
 int		yyerror(const char *);
 int		yylex(void);
 int		yyparse(void);
 
-static int	eflag;
-char **av;
 %}
 
 %union
@@ -96,23 +98,22 @@
 
 expr:	TOKEN
 	| '(' expr ')' { $$ = $2; }
-	| expr '|' expr { $$ = op_or ($1, $3); }
-	| expr '&' expr { $$ = op_and ($1, $3); }
-	| expr '=' expr { $$ = op_eq ($1, $3); }
-	| expr '>' expr { $$ = op_gt ($1, $3); }
-	| expr '<' expr { $$ = op_lt ($1, $3); }
-	| expr GE expr  { $$ = op_ge ($1, $3); }
-	| expr LE expr  { $$ = op_le ($1, $3); }
-	| expr NE expr  { $$ = op_ne ($1, $3); }
-	| expr '+' expr { $$ = op_plus ($1, $3); }
-	| expr '-' expr { $$ = op_minus ($1, $3); }
-	| expr '*' expr { $$ = op_times ($1, $3); }
-	| expr '/' expr { $$ = op_div ($1, $3); }
-	| expr '%' expr { $$ = op_rem ($1, $3); }
-	| expr ':' expr { $$ = op_colon ($1, $3); }
+	| expr '|' expr { $$ = op_or($1, $3); }
+	| expr '&' expr { $$ = op_and($1, $3); }
+	| expr '=' expr { $$ = op_eq($1, $3); }
+	| expr '>' expr { $$ = op_gt($1, $3); }
+	| expr '<' expr { $$ = op_lt($1, $3); }
+	| expr GE expr  { $$ = op_ge($1, $3); }
+	| expr LE expr  { $$ = op_le($1, $3); }
+	| expr NE expr  { $$ = op_ne($1, $3); }
+	| expr '+' expr { $$ = op_plus($1, $3); }
+	| expr '-' expr { $$ = op_minus($1, $3); }
+	| expr '*' expr { $$ = op_times($1, $3); }
+	| expr '/' expr { $$ = op_div($1, $3); }
+	| expr '%' expr { $$ = op_rem($1, $3); }
+	| expr ':' expr { $$ = op_colon($1, $3); }
 	;
 
-
 %%
 
 struct val *
@@ -120,89 +121,65 @@
 {
 	struct val *vp;
 
-	vp = (struct val *) malloc (sizeof (*vp));
-	if (vp == NULL) {
+	vp = (struct val *) malloc(sizeof(*vp));
+	if (vp == NULL)
 		errx(ERR_EXIT, "malloc() failed");
-	}
 
 	vp->type = integer;
 	vp->u.i  = i;
-	return vp; 
+	return (vp);
 }
 
 struct val *
 make_str(const char *s)
 {
 	struct val *vp;
-	char *ep;
 
-	vp = (struct val *) malloc (sizeof (*vp));
-	if (vp == NULL || ((vp->u.s = strdup (s)) == NULL)) {
+	vp = (struct val *) malloc(sizeof(*vp));
+	if (vp == NULL || ((vp->u.s = strdup(s)) == NULL))
 		errx(ERR_EXIT, "malloc() failed");
-	}
 
-	/*
-	 * Previously we tried to scan the string to see if it ``looked like''
-	 * an integer (erroneously, as it happened).  Let strtoimax() do the
-	 * dirty work.  We could cache the value, except that we are using
-	 * a union and need to preserve the original string form until we
-	 * are certain that it is not needed.
-	 *
-	 * IEEE Std.1003.1-2001 says:
-	 * /integer/ An argument consisting only of an (optional) unary minus  
-	 *	     followed by digits.          
-	 *
-	 * This means that arguments which consist of digits followed by
-	 * non-digits MUST NOT be considered integers.  strtoimax() will
-	 * figure this out for us.
-	 */
-	if (eflag)
-		(void)strtoimax(s, &ep, 10);
+	if (is_integer(s))
+		vp->type = numeric_string;
 	else
-		(void)strtol(s, &ep, 10);
-
-	if (*ep != '\0')
 		vp->type = string;
-	else	
-		vp->type = numeric_string;
 
-	return vp;
+	return (vp);
 }
 
-
 void
 free_value(struct val *vp)
 {
 	if (vp->type == string || vp->type == numeric_string)
-		free (vp->u.s);	
+		free(vp->u.s);
 }
 
-
-intmax_t
+int
 to_integer(struct val *vp)
 {
 	intmax_t i;
 
-	if (vp->type == integer)
-		return 1;
-
-	if (vp->type == string)
-		return 0;
-
-	/* vp->type == numeric_string, make it numeric */
-	errno = 0;
-	if (eflag) {
+	/* we can only convert numeric_string to integer, here */
+	if (vp->type == numeric_string) {
+		errno = 0;
 		i  = strtoimax(vp->u.s, (char **)NULL, 10);
-		if (errno == ERANGE)
-			err(ERR_EXIT, NULL);
-	} else {
-		i = strtol(vp->u.s, (char **)NULL, 10);
+		/* just keep as numeric_string, if the conversion fails */
+		if (errno != ERANGE) {
+			free(vp->u.s);
+			vp->u.i = i;
+			vp->type = integer;
+		}
 	}
+	return (vp->type == integer);
+}
 
-	free (vp->u.s);
-	vp->u.i = i;
-	vp->type = integer;
-	return 1;
+void
+assert_to_integer(struct val *vp)
+{
+	if (vp->type == string)
+		errx(ERR_EXIT, "not a decimal number: '%s'", vp->u.s);
+	if (!to_integer(vp))
+		errx(ERR_EXIT, "operand too large: '%s'", vp->u.s);
 }
 
 void
@@ -228,15 +205,26 @@
 	vp->u.s  = tmp;
 }
 
+int
+is_integer(const char *s)
+{
+	if (eflag)
+		while (isspace((unsigned char)*s))
+			s++;
+	if (*s == '-')
+		s++;
+	while (isdigit((unsigned char)*s))
+		s++;
+	return (*s == '\0');
+}
 
 int
-isstring(struct val *vp)
+is_string(struct val *vp)
 {
 	/* only TRUE if this string is not a valid integer */
 	return (vp->type == string);
 }
 
-
 int
 yylex(void)
 {
@@ -247,10 +235,10 @@
 
 	p = *av++;
 
-	if (strlen (p) == 1) {
-		if (strchr ("|&=<>+-*/%:()", *p))
+	if (strlen(p) == 1) {
+		if (strchr("|&=<>+-*/%:()", *p))
 			return (*p);
-	} else if (strlen (p) == 2 && p[1] == '=') {
+	} else if (strlen(p) == 2 && p[1] == '=') {
 		switch (*p) {
 		case '>': return (GE);
 		case '<': return (LE);
@@ -258,19 +246,17 @@
 		}
 	}
 
-	yylval.val = make_str (p);
+	yylval.val = make_str(p);
 	return (TOKEN);
 }
 
 int
 is_zero_or_null(struct val *vp)
 {
-	if (vp->type == integer) {
+	if (vp->type == integer)
 		return (vp->u.i == 0);
-	} else {
-		return (*vp->u.s == 0 || (to_integer (vp) && vp->u.i == 0));
-	}
-	/* NOTREACHED */
+
+	return (*vp->u.s == 0 || (to_integer(vp) && vp->u.i == 0));
 }
 
 int
@@ -278,7 +264,7 @@
 {
 	int c;
 
-	setlocale (LC_ALL, "");
+	setlocale(LC_ALL, "");
 	if (getenv("EXPR_COMPAT") != NULL
 	    || check_utility_compat("expr")) {
 		av = argv + 1;
@@ -291,9 +277,8 @@
 				break;
 
 			default:
-				fprintf(stderr,
+				errx(ERR_EXIT,
 				    "usage: expr [-e] expression\n");
-				exit(ERR_EXIT);
 			}
 		av = argv + optind;
 	}
@@ -314,28 +299,27 @@
 	errx(ERR_EXIT, "syntax error");
 }
 
-
 struct val *
 op_or(struct val *a, struct val *b)
 {
-	if (is_zero_or_null (a)) {
-		free_value (a);
+	if (is_zero_or_null(a)) {
+		free_value(a);
 		return (b);
 	} else {
-		free_value (b);
+		free_value(b);
 		return (a);
 	}
 }
-		
+
 struct val *
 op_and(struct val *a, struct val *b)
 {
-	if (is_zero_or_null (a) || is_zero_or_null (b)) {
-		free_value (a);
-		free_value (b);
-		return (make_integer ((intmax_t)0));
+	if (is_zero_or_null(a) || is_zero_or_null(b)) {
+		free_value(a);
+		free_value(b);
+		return (make_integer((intmax_t)0));
 	} else {
-		free_value (b);
+		free_value(b);
 		return (a);
 	}
 }
@@ -345,19 +329,19 @@
 {
 	struct val *r;
 
-	if (isstring (a) || isstring (b)) {
-		to_string (a);
-		to_string (b);	
-		r = make_integer ((intmax_t)(strcoll (a->u.s, b->u.s) == 0));
-	} else {
-		(void)to_integer(a);
-		(void)to_integer(b);
-		r = make_integer ((intmax_t)(a->u.i == b->u.i));
+	if (is_string(a) || is_string(b)) {
+		to_string(a);
+		to_string(b);
+		r = make_integer((intmax_t)(strcoll(a->u.s, b->u.s) == 0));
+	} else {
+		assert_to_integer(a);
+		assert_to_integer(b);
+		r = make_integer((intmax_t)(a->u.i == b->u.i));
 	}
 
-	free_value (a);
-	free_value (b);
-	return r;
+	free_value(a);
+	free_value(b);
+	return (r);
 }
 
 struct val *
@@ -365,19 +349,19 @@
 {
 	struct val *r;
 
-	if (isstring (a) || isstring (b)) {
-		to_string (a);
-		to_string (b);
-		r = make_integer ((intmax_t)(strcoll (a->u.s, b->u.s) > 0));
-	} else {
-		(void)to_integer(a);
-		(void)to_integer(b);
-		r = make_integer ((intmax_t)(a->u.i > b->u.i));
+	if (is_string(a) || is_string(b)) {
+		to_string(a);
+		to_string(b);
+		r = make_integer((intmax_t)(strcoll(a->u.s, b->u.s) > 0));
+	} else {
+		assert_to_integer(a);
+		assert_to_integer(b);
+		r = make_integer((intmax_t)(a->u.i > b->u.i));
 	}
 
-	free_value (a);
-	free_value (b);
-	return r;
+	free_value(a);
+	free_value(b);
+	return (r);
 }
 
 struct val *
@@ -385,19 +369,19 @@
 {
 	struct val *r;
 
-	if (isstring (a) || isstring (b)) {
-		to_string (a);
-		to_string (b);
-		r = make_integer ((intmax_t)(strcoll (a->u.s, b->u.s) < 0));
-	} else {
-		(void)to_integer(a);
-		(void)to_integer(b);
-		r = make_integer ((intmax_t)(a->u.i < b->u.i));
+	if (is_string(a) || is_string(b)) {
+		to_string(a);
+		to_string(b);
+		r = make_integer((intmax_t)(strcoll(a->u.s, b->u.s) < 0));
+	} else {
+		assert_to_integer(a);
+		assert_to_integer(b);
+		r = make_integer((intmax_t)(a->u.i < b->u.i));
 	}
 
-	free_value (a);
-	free_value (b);
-	return r;
+	free_value(a);
+	free_value(b);
+	return (r);
 }
 
 struct val *
@@ -405,19 +389,19 @@
 {
 	struct val *r;
 
-	if (isstring (a) || isstring (b)) {
-		to_string (a);
-		to_string (b);
-		r = make_integer ((intmax_t)(strcoll (a->u.s, b->u.s) >= 0));
-	} else {
-		(void)to_integer(a);
-		(void)to_integer(b);
-		r = make_integer ((intmax_t)(a->u.i >= b->u.i));
+	if (is_string(a) || is_string(b)) {
+		to_string(a);
+		to_string(b);
+		r = make_integer((intmax_t)(strcoll(a->u.s, b->u.s) >= 0));
+	} else {
+		assert_to_integer(a);
+		assert_to_integer(b);
+		r = make_integer((intmax_t)(a->u.i >= b->u.i));
 	}
 
-	free_value (a);
-	free_value (b);
-	return r;
+	free_value(a);
+	free_value(b);
+	return (r);
 }
 
 struct val *
@@ -425,19 +409,19 @@
 {
 	struct val *r;
 
-	if (isstring (a) || isstring (b)) {
-		to_string (a);
-		to_string (b);
-		r = make_integer ((intmax_t)(strcoll (a->u.s, b->u.s) <= 0));
-	} else {
-		(void)to_integer(a);
-		(void)to_integer(b);
-		r = make_integer ((intmax_t)(a->u.i <= b->u.i));
+	if (is_string(a) || is_string(b)) {
+		to_string(a);
+		to_string(b);
+		r = make_integer((intmax_t)(strcoll(a->u.s, b->u.s) <= 0));
+	} else {
+		assert_to_integer(a);
+		assert_to_integer(b);
+		r = make_integer((intmax_t)(a->u.i <= b->u.i));
 	}
 
-	free_value (a);
-	free_value (b);
-	return r;
+	free_value(a);
+	free_value(b);
+	return (r);
 }
 
 struct val *
@@ -445,33 +429,31 @@
 {
 	struct val *r;
 
-	if (isstring (a) || isstring (b)) {
-		to_string (a);
-		to_string (b);
-		r = make_integer ((intmax_t)(strcoll (a->u.s, b->u.s) != 0));
-	} else {
-		(void)to_integer(a);
-		(void)to_integer(b);
-		r = make_integer ((intmax_t)(a->u.i != b->u.i));
+	if (is_string(a) || is_string(b)) {
+		to_string(a);
+		to_string(b);
+		r = make_integer((intmax_t)(strcoll(a->u.s, b->u.s) != 0));
+	} else {
+		assert_to_integer(a);
+		assert_to_integer(b);
+		r = make_integer((intmax_t)(a->u.i != b->u.i));
 	}
 
-	free_value (a);
-	free_value (b);
-	return r;
+	free_value(a);
+	free_value(b);
+	return (r);
 }
 
-int
-chk_plus(intmax_t a, intmax_t b, intmax_t r)
+void
+assert_plus(intmax_t a, intmax_t b, intmax_t r)
 {
-
-	/* sum of two positive numbers must be positive */
-	if (a > 0 && b > 0 && r <= 0)
-		return 1;
-	/* sum of two negative numbers must be negative */
-	if (a < 0 && b < 0 && r >= 0)
-		return 1;
-	/* all other cases are OK */
-	return 0;
+	/*
+	 * sum of two positive numbers must be positive,
+	 * sum of two negative numbers must be negative
+	 */
+	if ((a > 0 && b > 0 && r <= 0) ||
+	    (a < 0 && b < 0 && r >= 0))
+		errx(ERR_EXIT, "overflow");
 }
 
 struct val *
@@ -479,36 +461,24 @@
 {
 	struct val *r;
 
-	if (!to_integer(a) || !to_integer(b)) {
-		errx(ERR_EXIT, "non-numeric argument");
-	}
+	assert_to_integer(a);
+	assert_to_integer(b);
+	r = make_integer(a->u.i + b->u.i);
+	assert_plus(a->u.i, b->u.i, r->u.i);
 
-	if (eflag) {
-		r = make_integer(a->u.i + b->u.i);
-		if (chk_plus(a->u.i, b->u.i, r->u.i)) {
-			errx(ERR_EXIT, "overflow");
-		}
-	} else
-		r = make_integer((long)a->u.i + (long)b->u.i);
-
-	free_value (a);
-	free_value (b);
-	return r;
+	free_value(a);
+	free_value(b);
+	return (r);
 }
 
-int
-chk_minus(intmax_t a, intmax_t b, intmax_t r)
+void
+assert_minus(intmax_t a, intmax_t b, intmax_t r)
 {
-
 	/* special case subtraction of INTMAX_MIN */
-	if (b == INTMAX_MIN) {
-		if (a >= 0)
-			return 1;
-		else
-			return 0;
-	}
-	/* this is allowed for b != INTMAX_MIN */
-	return chk_plus (a, -b, r);
+	if (b == INTMAX_MIN && a < 0)
+		errx(ERR_EXIT, "overflow");
+	/* check addition of negative subtrahend */
+	assert_plus(a, -b, r);
 }
 
 struct val *
@@ -516,33 +486,25 @@
 {
 	struct val *r;
 
-	if (!to_integer(a) || !to_integer(b)) {
-		errx(ERR_EXIT, "non-numeric argument");
-	}
+	assert_to_integer(a);
+	assert_to_integer(b);
+	r = make_integer(a->u.i - b->u.i);
+	assert_minus(a->u.i, b->u.i, r->u.i);
 
-	if (eflag) {
-		r = make_integer(a->u.i - b->u.i);
-		if (chk_minus(a->u.i, b->u.i, r->u.i)) {
-			errx(ERR_EXIT, "overflow");
-		}
-	} else
-		r = make_integer((long)a->u.i - (long)b->u.i);
-
-	free_value (a);
-	free_value (b);
-	return r;
+	free_value(a);
+	free_value(b);
+	return (r);
 }
 
-int
-chk_times(intmax_t a, intmax_t b, intmax_t r)
+void
+assert_times(intmax_t a, intmax_t b, intmax_t r)
 {
-	/* special case: first operand is 0, no overflow possible */
-	if (a == 0)
-		return 0;
-	/* verify that result of division matches second operand */
-	if (r / a != b)
-		return 1;
-	return 0;
+	/*
+	 * if first operand is 0, no overflow is possible,
+	 * else result of division test must match second operand
+	 */
+	if (a != 0 && r / a != b)
+		errx(ERR_EXIT, "overflow");
 }
 
 struct val *
@@ -550,32 +512,24 @@
 {
 	struct val *r;
 
-	if (!to_integer(a) || !to_integer(b)) {
-		errx(ERR_EXIT, "non-numeric argument");
-	}
-
-	if (eflag) {
-		r = make_integer(a->u.i * b->u.i);
-		if (chk_times(a->u.i, b->u.i, r->u.i)) {
-			errx(ERR_EXIT, "overflow");
-		}
-	} else
-		r = make_integer((long)a->u.i * (long)b->u.i);
+	assert_to_integer(a);
+	assert_to_integer(b);
+	r = make_integer(a->u.i * b->u.i);
+	assert_times(a->u.i, b->u.i, r->u.i);
 
-	free_value (a);
-	free_value (b);
+	free_value(a);
+	free_value(b);
 	return (r);
 }
 
-int
-chk_div(intmax_t a, intmax_t b)
+void
+assert_div(intmax_t a, intmax_t b)
 {
-	/* div by zero has been taken care of before */
+	if (b == 0)
+		errx(ERR_EXIT, "division by zero");
 	/* only INTMAX_MIN / -1 causes overflow */
 	if (a == INTMAX_MIN && b == -1)
-		return 1;
-	/* everything else is OK */
-	return 0;
+		errx(ERR_EXIT, "overflow");
 }
 
 struct val *
@@ -583,51 +537,33 @@
 {
 	struct val *r;
 
-	if (!to_integer(a) || !to_integer(b)) {
-		errx(ERR_EXIT, "non-numeric argument");
-	}
-
-	if (b->u.i == 0) {
-		errx(ERR_EXIT, "division by zero");
-	}
-
-	if (eflag) {
-		r = make_integer(a->u.i / b->u.i);
-		if (chk_div(a->u.i, b->u.i)) {
-			errx(ERR_EXIT, "overflow");
-		}
-	} else
-		r = make_integer((long)a->u.i / (long)b->u.i);
+	assert_to_integer(a);
+	assert_to_integer(b);
+	/* assert based on operands only, not on result */
+	assert_div(a->u.i, b->u.i);
+	r = make_integer(a->u.i / b->u.i);
 
-	free_value (a);
-	free_value (b);
-	return r;
+	free_value(a);
+	free_value(b);
+	return (r);
 }
-	
+
 struct val *
 op_rem(struct val *a, struct val *b)
 {
 	struct val *r;
 
-	if (!to_integer(a) || !to_integer(b)) {
-		errx(ERR_EXIT, "non-numeric argument");
-	}
-
-	if (b->u.i == 0) {
-		errx(ERR_EXIT, "division by zero");
-	}
-
-	if (eflag)
-		r = make_integer(a->u.i % b->u.i);
-	        /* chk_rem necessary ??? */
-	else
-		r = make_integer((long)a->u.i % (long)b->u.i);
+	assert_to_integer(a);
+	assert_to_integer(b);
+	/* pass a=1 to only check for div by zero */
+	assert_div(1, b->u.i);
+	r = make_integer(a->u.i % b->u.i);
 
-	free_value (a);
-	free_value (b);
-	return r;
+	free_value(a);
+	free_value(b);
+	return (r);
 }
-	
+
 struct val *
 op_colon(struct val *a, struct val *b)
 {
@@ -642,33 +578,30 @@
 	to_string(b);
 
 	/* compile regular expression */
-	if ((eval = regcomp (&rp, b->u.s, 0)) != 0) {
-		regerror (eval, &rp, errbuf, sizeof(errbuf));
+	if ((eval = regcomp(&rp, b->u.s, 0)) != 0) {
+		regerror(eval, &rp, errbuf, sizeof(errbuf));
 		errx(ERR_EXIT, "%s", errbuf);
 	}
 
 	/* compare string against pattern */
 	/* remember that patterns are anchored to the beginning of the line */
-	if (regexec(&rp, a->u.s, (size_t)2, rm, 0) == 0 && rm[0].rm_so == 0) {
+	if (regexec(&rp, a->u.s, (size_t)2, rm, 0) == 0 && rm[0].rm_so == 0)
 		if (rm[1].rm_so >= 0) {
 			*(a->u.s + rm[1].rm_eo) = '\0';
-			v = make_str (a->u.s + rm[1].rm_so);
+			v = make_str(a->u.s + rm[1].rm_so);
 
-		} else {
-			v = make_integer ((intmax_t)(rm[0].rm_eo - rm[0].rm_so));
-		}
-	} else {
-		if (rp.re_nsub == 0) {
-			v = make_integer ((intmax_t)0);
-		} else {
-			v = make_str ("");
-		}
-	}
+		} else
+			v = make_integer((intmax_t)(rm[0].rm_eo - rm[0].rm_so));
+	else
+		if (rp.re_nsub == 0)
+			v = make_integer((intmax_t)0);
+		else
+			v = make_str("");
 
 	/* free arguments and pattern buffer */
-	free_value (a);
-	free_value (b);
-	regfree (&rp);
+	free_value(a);
+	free_value(b);
+	regfree(&rp);
 
-	return v;
+	return (v);
 }

--------------060600010302060201060806--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4E0E2535.1060204>