test's internal getn() makes integers out of strings although boundaries
are checked for long which leads to wrong test results when operands are
greater than INT_MAX but smaller than LONG_MAX:
$ t() { /bin/test "$1" -lt 0 && : overflow; }
$ t 1
+ /bin/test 1 -lt 0
$ t $((1 << 31)) # INT32_MAX + 1
+ /bin/test 2147483648 -lt 0
+ : overflow
n=$((0x7fffffffffffffff))
n=${n%7}8
$ t $n # INT64_MAX + 1
+ /bin/test 9223372036854775808 -lt 0
test: 9223372036854775808: out of range
The INT64_MAX case is expected behaviour, just wanted to illustrate it.
Note how $n is crafted manually since $((1 << 63)) overflows in our KSH
already.
$ t "0 "
+ /bin/test 0 -lt 0
With this diff overflows are properly catched:
$ t $((1 << 31))
+ /usr/obj/bin/test 2147483648 -lt 0
test: 2147483648: too large
Another intended side effect is that trailing whitespaces in integer
operands will now cause test(1) to fail:
$ t "0 "
+ /usr/obj/bin/test/test 0 -lt 0
test: 0 : invalid
Expecting some people to oppose, I'd like to discuss this particular
change in behaviour. Here are my four cents:
- When used in shell scripts, integer operands should usually
not require quoting, whether they're passed verbatim, through
variables or subshells or the like. If they do contain
whitespaces, the shell already takes care of them when
omitting quotes since after variable substitution/expansion
and before passing arguments to the executable or built-in.
If quotes are used (as in "take this as is"), I generally
prefer to be warned instead of letting unexpected characters
slip through, especially when it comes to arithmetic
operations.
- While properly returing integers, strtol(3) still considers
trailing non-digits as error, stripping them in test(1) as of
now seems more like a convenience hack (possibly problematic).
- Using the more strict strtonum(3) is not only safer but also
way simpler and shorter. The manual pages tell us that leading
whitespaces are ok but trailing ones are not.
- Behaviour across binaries and built-ins of various shells
already differs so changing/improving our test(1) won't break
consistency. This was tested with `test "0 " -lt 0':
* test(1) from GNU coreutils 8.25 returns 0
* bash, dash and ksh return 0
* zsh returns 2 and warns "integer expression expected: 0 "
* fish silently returns 1
Feedback and comments, please.
Index: test.c
===================================================================
RCS file: /cvs/src/bin/test/test.c,v
retrieving revision 1.17
diff -u -p -r1.17 test.c
--- test.c 21 Jan 2017 11:03:42 -0000 1.17
+++ test.c 24 Jul 2017 15:02:03 -0000
@@ -13,6 +13,7 @@
#include <sys/types.h>
#include <sys/stat.h>
+#include <limits.h>
#include <unistd.h>
#include <ctype.h>
#include <errno.h>
@@ -451,26 +448,17 @@ t_lex(char *s)
return OPERAND;
}
-/* atoi with error detection */
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++;
+ const char *e;
+ int r;
- if (*p)
- errx(2, "%s: bad number", s);
+ r = strtonum(s, INT_MIN, INT_MAX, &e);
+ if (e)
+ errx(2, "%s: %s", s, e);
- return (int) r;
+ return r;
}
static int
