On Tue, Mar 27, 2018 at 10:36:17PM +0200, Ingo Schwarze wrote:
> > + int sig;
>
> Drop this variable, it does no good but only harm.
Yeah, too bad that I didn't notice this left-over from a previous
development step. Strange enough that regression tests didn't
choke on it...
> > + /* 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;
As tb@ pointed out, the !isdigit check can be removed as well.
> > + c = strncmp(p1, p2, len1);
>
> That's another bug:
>
> schwarze@isnote $ /bin/test -1 -gt -2 ; echo $?
> 1
> schwarze@isnote $ /bin/test 1 -gt 2 ; echo $?
> 1
It's really great to have people who do extensive reviews. Thanks
a lot for spotting this. I have added it to the regression tests.
> With these changes, OK schwarze@. See below for the patch that
> i tested.
Modified with tb@'s remark. Also I changed the error message
"bad number" to "invalid", so we stay fully in sync with strtonum's
error messages.
> Given that breaking test(1) might cause hard-to-find breakage
> deep down in build systems and scripts, you might with a to
> have a second OK for commit, though.
Definitely, I'm not out to rush this one in.
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 20:43:38 -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,70 @@ 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;
+
+ /* 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')
+ *signum = 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: invalid", s);
+
+ 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) * sig1;
+
+ return c;
+}
+
static int
binop(void)
{
@@ -313,17 +380,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 +522,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 20:43:38 -0000
@@ -124,6 +124,17 @@ 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 0 '-1 -gt -2'
+t 1 '1 -gt 2'
+t 1 '4294967296 -eq 0'
+t 0 '12345678901234567890 -eq +0012345678901234567890'
t 1 '"" -o ""'
t 1 '"" -a ""'
