The test(1) utility is prone to integer overflows on 64 bit systems.
By using strtol and casting the result to an int, it is possible to
run tests which succeed even though they should fail:

$ arch
OpenBSD.amd64
$ /bin/test 4294967296 -eq 0; echo $?
0

I could have chosen the way of FreeBSD, NetBSD, or built-in test of
pdksh and extend the number to intmax_t, but I actually prefered the
way of coreutils, i.e. validating the number as strings.

This version allows numbers based on the rules which I figured out
by reading strtol's man page and POSIX's rather vague specification:

- Numbers are always base 10, regardless of starting with 0
- A number can start with arbitrary amounts of whitespaces
- Then a sign char is allowed (POSIX only mentions -)
- An arbitrary amount of digits follows
- Then an arbitrary amount of whitespaces can follow

Therefore, comparing numbers can be done in a human-understandable
form as strings:

1. If signum differs, the smaller number is the one having a -
2. Leading zeros are skipped
3. If both numbers are positive, the one with more digits is larger
4. If both numbers are negative, the one with more digits is smaller
5. Equality is based on a simple string comparison

I have extended the regression test suite. The last two additions
fail with current implementation.

While at it, -t has a proper 0-INT_MAX limitation now as well.

This is quite a refactoring so tests are very appreciated. Please note
that we have overflows in pdksh's builtin test as well. If bin/test
is refactored, I'll try to unify both implementations much closer to
each other where possible.


Tobias

Index: bin/test/test.c
===================================================================
RCS file: /cvs/src/bin/test/test.c,v
retrieving revision 1.18
diff -u -p -u -p -r1.18 test.c
--- bin/test/test.c     24 Jul 2017 22:15:52 -0000      1.18
+++ bin/test/test.c     27 Mar 2018 17:23:51 -0000
@@ -16,6 +16,7 @@
 #include <unistd.h>
 #include <ctype.h>
 #include <errno.h>
+#include <limits.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -145,6 +146,8 @@ static int aexpr(enum token n);
 static int nexpr(enum token n);
 static int binop(void);
 static int primary(enum token n);
+static const char *getnstr(const char *, int *, size_t *);
+static int intcmp(const char *, const char *);
 static int filstat(char *nm, enum token mode);
 static int getn(const char *s);
 static int newerf(const char *, const char *);
@@ -290,6 +293,72 @@ primary(enum token n)
        return strlen(*t_wp) > 0;
 }
 
+static const char *
+getnstr(const char *s, int *signum, size_t *len)
+{
+       const char *p, *start;
+       int sig;
+
+       /* skip leading whitespaces */
+       p = s;
+       while (isspace((unsigned char)*p))
+               p++;
+
+       /* accept optional sign */
+       if (*p == '-') {
+               *signum = -1;
+               p++;
+       } else {
+               *signum = 1;
+               if (*p == '+')
+                       p++;
+       }
+
+       /* skip leading zeros */
+       while (*p == '0' && isdigit((unsigned char)p[1]))
+               p++;
+
+       /* turn 0 always positive */
+       if (*p == '0' && !isdigit((unsigned char)p[1]))
+               sig = 1;
+
+       start = p;
+       while (isdigit((unsigned char)*p))
+               p++;
+       *len = p - start;
+
+       /* allow trailing whitespaces */
+       while (isspace((unsigned char)*p))
+               p++;
+
+       /* validate number */
+       if (*p != '\0' || *start == '\0')
+               errx(2, "%s: bad number", s);
+
+       *signum = sig;
+       return start;
+}
+
+static int
+intcmp(const char *opnd1, const char *opnd2)
+{
+       const char *p1, *p2;
+       size_t len1, len2;
+       int c, sig1, sig2;
+
+       p1 = getnstr(opnd1, &sig1, &len1);
+       p2 = getnstr(opnd2, &sig2, &len2);
+
+       if (sig1 != sig2)
+               c = sig1;
+       else if (len1 != len2)
+               c = (len1 < len2) ? -sig1 : sig1;
+       else
+               c = strncmp(p1, p2, len1);
+
+       return c;
+}
+
 static int
 binop(void)
 {
@@ -313,17 +382,17 @@ binop(void)
        case STRGT:
                return strcmp(opnd1, opnd2) > 0;
        case INTEQ:
-               return getn(opnd1) == getn(opnd2);
+               return intcmp(opnd1, opnd2) == 0;
        case INTNE:
-               return getn(opnd1) != getn(opnd2);
+               return intcmp(opnd1, opnd2) != 0;
        case INTGE:
-               return getn(opnd1) >= getn(opnd2);
+               return intcmp(opnd1, opnd2) >= 0;
        case INTGT:
-               return getn(opnd1) > getn(opnd2);
+               return intcmp(opnd1, opnd2) > 0;
        case INTLE:
-               return getn(opnd1) <= getn(opnd2);
+               return intcmp(opnd1, opnd2) <= 0;
        case INTLT:
-               return getn(opnd1) < getn(opnd2);
+               return intcmp(opnd1, opnd2) < 0;
        case FILNT:
                return newerf(opnd1, opnd2);
        case FILOT:
@@ -455,22 +524,26 @@ t_lex(char *s)
 static int
 getn(const char *s)
 {
-       char *p;
-       long r;
-
-       errno = 0;
-       r = strtol(s, &p, 10);
-
-       if (errno != 0)
-               errx(2, "%s: out of range", s);
-
-       while (isspace((unsigned char)*p))
-               p++;
+       char buf[32];
+       const char *errstr, *p;
+       size_t len;
+       int r, sig;
+
+       p = getnstr(s, &sig, &len);
+       if (sig != 1)
+               errstr = "too small";
+       else if (len >= sizeof(buf))
+               errstr = "too large";
+       else {
+               strlcpy(buf, p, sizeof(buf));
+               buf[len] = '\0';
+               r = strtonum(buf, 0, INT_MAX, &errstr);
+       }
 
-       if (*p)
-               errx(2, "%s: bad number", s);
+       if (errstr != NULL)
+               errx(2, "%s: %s", s, errstr);
 
-       return (int) r;
+       return r;
 }
 
 static int
Index: regress/bin/test/TEST.sh
===================================================================
RCS file: /cvs/src/regress/bin/test/TEST.sh,v
retrieving revision 1.2
diff -u -p -u -p -r1.2 TEST.sh
--- regress/bin/test/TEST.sh    25 Nov 2014 23:09:22 -0000      1.2
+++ regress/bin/test/TEST.sh    27 Mar 2018 17:23:51 -0000
@@ -124,6 +124,15 @@ t 0 '0 -eq 0'
 t 1 '-5 -eq 5'
 t 0 '\( 0 -eq 0 \)'
 t 1 '1 -eq 0 -o a = a -a 1 -eq 0 -o a = aa'
+t 0 '" +123 " -eq 123'
+t 1 '"-123  " -gt " -1"'
+t 0 '123 -gt -123'
+t 0 '-0 -eq +0'
+t 1 '+0 -gt 0'
+t 0 '0 -eq 0'
+t 0 '0000 -eq -0'
+t 1 '4294967296 -eq 0'
+t 0 '12345678901234567890 -eq +0012345678901234567890'
 
 t 1 '"" -o ""'
 t 1 '"" -a ""'

Reply via email to