Date: Thu, 21 Mar 2002 19:56:20 -0500 (EST) From: Garrett Wollman <wollman@lcs.mit.edu> To: standards@FreeBSD.org Cc: audit@FreeBSD.org Subject: Expr fixes Message-ID: <200203220056.g2M0uKU77186@khavrinen.lcs.mit.edu>
next in thread | raw e-mail | index | archive | help
[Followups to -standards, please.]
I've fixed expr to be able to deal correctly with arguments and
results consisting of a single '-'. At the same time, I also changed
the data type used internally by expr to intmax_t, away from
(deprecated) quad_t. The whole ``pre-parse to see if it might be an
integer'' thing is silly, and should probably be removed entirely --
function calls are *not* that expensive. There are many style bugs in
this utility.
I have a couple of defect reports in to the Austin Group about the
lack of specificity in 1003.1-2001 as regards how expr's integer math
actually works, and whether expr should be prepared to accept an
initial argument of "--" to indicate that there are no options.
This change was created and tested on FreeBSD/sparc64, while tracking
down a test-suite failure in autoconf 2.52.
-GAWollman
Index: expr.y
===================================================================
RCS file: /home/cvs/src/bin/expr/expr.y,v
retrieving revision 1.18
diff -u -r1.18 expr.y
--- expr.y 2002/02/02 06:36:49 1.18
+++ expr.y 2002/03/22 00:47:57
@@ -9,6 +9,7 @@
#include <sys/types.h>
#include <stdio.h>
+#include <inttypes.h>
#include <stdlib.h>
#include <string.h>
#include <locale.h>
@@ -26,20 +27,20 @@
enum valtype type;
union {
char *s;
- quad_t i;
+ intmax_t i;
} u;
} ;
struct val *result;
-int chk_div(quad_t, quad_t);
-int chk_minus(quad_t, quad_t, quad_t);
-int chk_plus(quad_t, quad_t, quad_t);
-int chk_times(quad_t, quad_t, quad_t);
+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 isstring(struct val *);
-struct val *make_integer(quad_t);
+struct val *make_integer(intmax_t);
struct val *make_str(const char *);
struct val *op_and(struct val *, struct val *);
struct val *op_colon(struct val *, struct val *);
@@ -55,7 +56,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 *);
-quad_t to_integer(struct val *);
+intmax_t to_integer(struct val *);
void to_string(struct val *);
int yyerror(const char *);
int yylex(void);
@@ -105,7 +106,7 @@
%%
struct val *
-make_integer(quad_t i)
+make_integer(intmax_t i)
{
struct val *vp;
@@ -123,26 +124,34 @@
make_str(const char *s)
{
struct val *vp;
- size_t i;
- int isint;
+ char *ep;
vp = (struct val *) malloc (sizeof (*vp));
if (vp == NULL || ((vp->u.s = strdup (s)) == NULL)) {
errx (2, "malloc() failed");
}
- for(i = 1, isint = isdigit(s[0]) || s[0] == '-';
- isint && i < strlen(s);
- i++)
- {
- if(!isdigit(s[i]))
- isint = 0;
- }
+ /*
+ * 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.
+ */
+ (void)strtoimax(s, &ep, 10);
- if (isint)
- vp->type = numeric_string;
- else
+ if (*ep != '\0')
vp->type = string;
+ else
+ vp->type = numeric_string;
return vp;
}
@@ -156,10 +165,10 @@
}
-quad_t
+intmax_t
to_integer(struct val *vp)
{
- quad_t i;
+ intmax_t i;
if (vp->type == integer)
return 1;
@@ -169,10 +178,10 @@
/* vp->type == numeric_string, make it numeric */
errno = 0;
- i = strtoq(vp->u.s, (char**)NULL, 10);
- if (errno != 0) {
- errx (2, "overflow");
- }
+ i = strtoimax(vp->u.s, (char **)NULL, 10);
+ if (errno == ERANGE)
+ err(2, NULL);
+
free (vp->u.s);
vp->u.i = i;
vp->type = integer;
@@ -187,12 +196,18 @@
if (vp->type == string || vp->type == numeric_string)
return;
- tmp = malloc ((size_t)25);
+ /*
+ * log_10(x) ~= 0.3 * log_2(x). Rounding up gives the number
+ * of digits; add one each for the sign and terminating null
+ * character, respectively.
+ */
+#define NDIGITS(x) (3 * (sizeof(x) * CHAR_BIT) / 10 + 1 + 1 + 1)
+ tmp = malloc (NDIGITS(vp->u.i));
if (tmp == NULL) {
errx (2, "malloc() failed");
}
- sprintf (tmp, "%lld", (long long)vp->u.i);
+ sprintf (tmp, "%jd", vp->u.i);
vp->type = string;
vp->u.s = tmp;
}
@@ -252,7 +267,7 @@
yyparse ();
if (result->type == integer)
- printf ("%lld\n", (long long)result->u.i);
+ printf ("%jd\n", result->u.i);
else
printf ("%s\n", result->u.s);
@@ -284,7 +299,7 @@
if (is_zero_or_null (a) || is_zero_or_null (b)) {
free_value (a);
free_value (b);
- return (make_integer ((quad_t)0));
+ return (make_integer ((intmax_t)0));
} else {
free_value (b);
return (a);
@@ -299,11 +314,11 @@
if (isstring (a) || isstring (b)) {
to_string (a);
to_string (b);
- r = make_integer ((quad_t)(strcoll (a->u.s, b->u.s) == 0));
+ 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 ((quad_t)(a->u.i == b->u.i));
+ r = make_integer ((intmax_t)(a->u.i == b->u.i));
}
free_value (a);
@@ -319,11 +334,11 @@
if (isstring (a) || isstring (b)) {
to_string (a);
to_string (b);
- r = make_integer ((quad_t)(strcoll (a->u.s, b->u.s) > 0));
+ 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 ((quad_t)(a->u.i > b->u.i));
+ r = make_integer ((intmax_t)(a->u.i > b->u.i));
}
free_value (a);
@@ -339,11 +354,11 @@
if (isstring (a) || isstring (b)) {
to_string (a);
to_string (b);
- r = make_integer ((quad_t)(strcoll (a->u.s, b->u.s) < 0));
+ 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 ((quad_t)(a->u.i < b->u.i));
+ r = make_integer ((intmax_t)(a->u.i < b->u.i));
}
free_value (a);
@@ -359,11 +374,11 @@
if (isstring (a) || isstring (b)) {
to_string (a);
to_string (b);
- r = make_integer ((quad_t)(strcoll (a->u.s, b->u.s) >= 0));
+ 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 ((quad_t)(a->u.i >= b->u.i));
+ r = make_integer ((intmax_t)(a->u.i >= b->u.i));
}
free_value (a);
@@ -379,11 +394,11 @@
if (isstring (a) || isstring (b)) {
to_string (a);
to_string (b);
- r = make_integer ((quad_t)(strcoll (a->u.s, b->u.s) <= 0));
+ 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 ((quad_t)(a->u.i <= b->u.i));
+ r = make_integer ((intmax_t)(a->u.i <= b->u.i));
}
free_value (a);
@@ -399,11 +414,11 @@
if (isstring (a) || isstring (b)) {
to_string (a);
to_string (b);
- r = make_integer ((quad_t)(strcoll (a->u.s, b->u.s) != 0));
+ 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 ((quad_t)(a->u.i != b->u.i));
+ r = make_integer ((intmax_t)(a->u.i != b->u.i));
}
free_value (a);
@@ -412,7 +427,7 @@
}
int
-chk_plus(quad_t a, quad_t b, quad_t r)
+chk_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)
@@ -433,7 +448,7 @@
errx (2, "non-numeric argument");
}
- r = make_integer (/*(quad_t)*/(a->u.i + b->u.i));
+ r = make_integer (/*(intmax_t)*/(a->u.i + b->u.i));
if (chk_plus (a->u.i, b->u.i, r->u.i)) {
errx (2, "overflow");
}
@@ -443,16 +458,16 @@
}
int
-chk_minus(quad_t a, quad_t b, quad_t r)
+chk_minus(intmax_t a, intmax_t b, intmax_t r)
{
- /* special case subtraction of QUAD_MIN */
- if (b == QUAD_MIN) {
+ /* special case subtraction of INTMAX_MIN */
+ if (b == INTMAX_MIN) {
if (a >= 0)
return 1;
else
return 0;
}
- /* this is allowed for b != QUAD_MIN */
+ /* this is allowed for b != INTMAX_MIN */
return chk_plus (a, -b, r);
}
@@ -465,7 +480,7 @@
errx (2, "non-numeric argument");
}
- r = make_integer (/*(quad_t)*/(a->u.i - b->u.i));
+ r = make_integer (/*(intmax_t)*/(a->u.i - b->u.i));
if (chk_minus (a->u.i, b->u.i, r->u.i)) {
errx (2, "overflow");
}
@@ -475,7 +490,7 @@
}
int
-chk_times(quad_t a, quad_t b, quad_t r)
+chk_times(intmax_t a, intmax_t b, intmax_t r)
{
/* special case: first operand is 0, no overflow possible */
if (a == 0)
@@ -495,7 +510,7 @@
errx (2, "non-numeric argument");
}
- r = make_integer (/*(quad_t)*/(a->u.i * b->u.i));
+ r = make_integer (/*(intmax_t)*/(a->u.i * b->u.i));
if (chk_times (a->u.i, b->u.i, r->u.i)) {
errx (2, "overflow");
}
@@ -505,11 +520,11 @@
}
int
-chk_div(quad_t a, quad_t b)
+chk_div(intmax_t a, intmax_t b)
{
/* div by zero has been taken care of before */
- /* only QUAD_MIN / -1 causes overflow */
- if (a == QUAD_MIN && b == -1)
+ /* only INTMAX_MIN / -1 causes overflow */
+ if (a == INTMAX_MIN && b == -1)
return 1;
/* everything else is OK */
return 0;
@@ -528,7 +543,7 @@
errx (2, "division by zero");
}
- r = make_integer (/*(quad_t)*/(a->u.i / b->u.i));
+ r = make_integer (/*(intmax_t)*/(a->u.i / b->u.i));
if (chk_div (a->u.i, b->u.i)) {
errx (2, "overflow");
}
@@ -550,7 +565,7 @@
errx (2, "division by zero");
}
- r = make_integer (/*(quad_t)*/(a->u.i % b->u.i));
+ r = make_integer (/*(intmax_t)*/(a->u.i % b->u.i));
/* chk_rem necessary ??? */
free_value (a);
free_value (b);
@@ -584,11 +599,11 @@
v = make_str (a->u.s + rm[1].rm_so);
} else {
- v = make_integer ((quad_t)(rm[0].rm_eo - rm[0].rm_so));
+ v = make_integer ((intmax_t)(rm[0].rm_eo - rm[0].rm_so));
}
} else {
if (rp.re_nsub == 0) {
- v = make_integer ((quad_t)0);
+ v = make_integer ((intmax_t)0);
} else {
v = make_str ("");
}
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-audit" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200203220056.g2M0uKU77186>