POSIX-compliance fixes for GNU 'test'

[Paul Eggert]

Maciek Olczak's Wednesday message to bug-coreutils got me to looking
at GNU 'test', and I noticed that it disagrees with both Bash and
POSIX in several places.  I think it's better to make it consistent
(if only so that "man test" and "info test" match Bash's behavior
better.  :-)

Here are fixes for all the POSIX-compliance problems that I found with
'test'.

2003-07-24  Paul Eggert  <address@hidden>

        Fix some POSIX-compliance problems with 'test'.  This makes
        'test' more compatible with Bash.

        * NEWS, doc/coreutils.texi: Document the following.
        * src/test.c: Include exitfail.h.
        (TEST_FAILURE): New constant, used for exit status if 'test' fails.
        (test-syntax_error): Use it.
        (binary_operator): Now takes bool arg specifying whether left operand
        is -l ARG, so that caller determines this rather than us.
        All uses changed.
        (term): Use posixtest to evaluate parenthesized subexpressions.
        (unary_operator, one_argument): Remove support for -t without operand.
        (one_argument): Take argument from argv[pos].
        (one_argument, two_arguments, three_arguments): Advance pos.
        All callers changed.
        (three_arguments): Look for binary ops before "!".  Then look
        for parenthesized one_argument expressions, instead of trusting
        expr () to do the right thing.
        (posixtest): Now takes number of args.  All callers changed.
        Treat "( A B )" like "A B".
        (main): Set exit_failure to TEST_FAILURE.  Don't depend on
        POSIXLY_CORRECT, as we now conform to POSIX by default.
        (main) [!LBRACKET]: Do not recognize "--help" or "--verbose" unless.
        * tests/test/Test.pm (test_vector): Add several tests to check
        the above.  Syntax errors now exit with status 2, not 1.

        * src/test.c (PROGRAM_NAME) [LBRACKET]: Now "[", not "test".
        (usage): Say "test" and "[" explicitly, so that the man page
        works out nicer.
        * man/Makefile.am (mapped_name): Use '[' to generate man page
        for 'test', since 'test --help' no longer outputs help.

Index: NEWS
===================================================================
RCS file: /cvsroot/coreutils/coreutils/NEWS,v
retrieving revision 1.114
diff -p -u -r1.114 NEWS
--- NEWS        20 Jul 2003 15:22:42 -0000      1.114
+++ NEWS        24 Jul 2003 22:11:56 -0000
@@ -3,6 +3,15 @@ GNU coreutils NEWS                      
 
 ** New features
 
+  `test' is now more compatible with Bash and POSIX:
+
+    `test -t', `test --help', and `test --version' now silently exit
+    with status 0.  To test whether standard output is a terminal, use
+    `test -t 1'.  To get help and version info for `test', use
+    `[ --help' and `[ --version'.
+
+    `test' now exits with status 2 (not 1) if there is an error.
+
   wc count field widths now are heuristically adjusted depending on the input
   size, if known.  If only one count is printed, it is guaranteed to
   be printed without leading spaces.
Index: doc/coreutils.texi
===================================================================
RCS file: /cvsroot/coreutils/coreutils/doc/coreutils.texi,v
retrieving revision 1.122
diff -p -u -r1.122 coreutils.texi
--- doc/coreutils.texi  20 Jul 2003 15:24:21 -0000      1.122
+++ doc/coreutils.texi  24 Jul 2003 22:12:11 -0000
@@ -8692,15 +8692,36 @@ expression must be a separate argument.
 @command{test} has file status checks, string operators, and numeric
 comparison operators.
 
address@hidden has an alternate form that uses opening and closing
+square brackets instead a leading @samp{test}.  For example, instead
+of @samp{test -d /}, you can write @samp{[ -d / ]}.  The square
+brackets must be separate arguments; for example, @samp{[-d /]} does
+not have the desired effect.  Since @samp{test @var{expr}} and @samp{[
address@hidden ]} have the same meaning, only the former form is discussed
+below.
+
 @cindex conflicts with shell built-ins
 @cindex built-in shell commands, conflicts with
 Because most shells have a built-in command by the same name, using the
 unadorned command name in a script or interactively may get you
 different functionality than that described here.
 
-Besides the options below, @command{test} accepts a lone @option{--help} or
address@hidden  @xref{Common options}.  A single non-option argument
-is also allowed: @command{test} returns true if the argument is not null.
+Besides the options below, a single argument is also allowed:
address@hidden returns true if the argument is not null.  The argument
+can be any string, including strings like @samp{-d}, @samp{-1},
address@hidden, @samp{--help}, and @samp{--version} that most other
+programs would treat as options.  To get help and version information,
+invoke the commands @samp{[ --help} and @samp{[ --version}, without
+the usual closing brackets.  @xref{Common options}.
+
address@hidden exit status of @command{test}
+Exit status:
+
address@hidden
+0 if the expression is true,
+1 if the expression is false,
+2 if an error occurred.
address@hidden display
 
 @menu
 * File type tests::             -[bcdfhLpSt]
@@ -8759,11 +8780,11 @@ True if @var{file} exists and is a named
 @cindex socket check
 True if @var{file} exists and is a socket.
 
address@hidden -t address@hidden
address@hidden -t @var{fd}
 @opindex -t
 @cindex terminal check
-True if @var{fd} is opened on a terminal.  If @var{fd} is omitted, it
-defaults to 1 (standard output).
+True if @var{fd} is a file descriptor that is associated with a
+terminal.
 
 @end table
 
Index: man/Makefile.am
===================================================================
RCS file: /cvsroot/coreutils/coreutils/man/Makefile.am,v
retrieving revision 1.19
diff -p -u -r1.19 Makefile.am
--- man/Makefile.am     22 Jul 2003 16:32:33 -0000      1.19
+++ man/Makefile.am     24 Jul 2003 22:12:13 -0000
@@ -117,8 +117,9 @@ SUFFIXES = .x .1
 
 # Ensure that help2man runs the ../src/ginstall binary as
 # `install' when creating install.1.
+# Similarly, ensure that it uses the ../src/[ binary to create test.1.
 t = $*.td
-mapped_name = `echo $*|sed 's/install/ginstall/'`
+mapped_name = `echo $*|sed 's/install/ginstall/; s/test/[/'`
 
 # Note the use of $t/$*, rather than just `$*' as in other packages.
 # That is necessary to avoid failures for programs that are also shell built-in
Index: src/test.c
===================================================================
RCS file: /cvsroot/coreutils/coreutils/src/test.c,v
retrieving revision 1.92
diff -p -u -r1.92 test.c
--- src/test.c  23 Jul 2003 07:29:55 -0000      1.92
+++ src/test.c  24 Jul 2003 22:16:35 -0000
@@ -27,17 +27,22 @@
 #include <stdio.h>
 #include <sys/types.h>
 
-/* The official name of this program (e.g., no `g' prefix).  */
-#define PROGRAM_NAME "test"
-
 #define TEST_STANDALONE 1
 
 #ifndef LBRACKET
 # define LBRACKET 0
 #endif
 
+/* The official name of this program (e.g., no `g' prefix).  */
+#if LBRACKET
+# define PROGRAM_NAME "["
+#else
+# define PROGRAM_NAME "test"
+#endif
+
 #include "system.h"
 #include "error.h"
+#include "exitfail.h"
 #include "euidaccess.h"
 
 #ifndef _POSIX_VERSION
@@ -63,9 +68,6 @@ extern uid_t geteuid ();
 # define F_OK 0
 #endif /* R_OK */
 
-/* This name is used solely when printing --version information.  */
-#define PROGRAM_NAME "test"
-
 /* The following few defines control the truth and false output of each stage.
    TRUE and FALSE are what we use to compute the final output value.
    SHELL_BOOLEAN is the form which returns truth or falseness in shell terms.
@@ -79,6 +81,9 @@ extern uid_t geteuid ();
 #define TRUTH_OR(a, b) ((a) | (b))
 #define TRUTH_AND(a, b) ((a) & (b))
 
+/* Exit status for syntax errors, etc.  */
+enum { TEST_FAILURE = 2 };
+
 #if defined (TEST_STANDALONE)
 # define test_exit(val) exit (val)
 #else
@@ -94,10 +99,10 @@ static char **argv; /* The argument list
 static int test_unop (char const *s);
 static int binop (char *s);
 static int unary_operator (void);
-static int binary_operator (void);
+static int binary_operator (bool);
 static int two_arguments (void);
 static int three_arguments (void);
-static int posixtest (void);
+static int posixtest (int);
 
 static int expr (void);
 static int term (void);
@@ -114,7 +119,7 @@ test_syntax_error (char const *format, c
   fprintf (stderr, "%s: ", argv[0]);
   fprintf (stderr, format, arg);
   fflush (stderr);
-  test_exit (SHELL_BOOLEAN (FALSE));
+  test_exit (TEST_FAILURE);
 }
 
 #if HAVE_SETREUID && HAVE_SETREGID
@@ -318,8 +323,20 @@ term (void)
   /* A paren-bracketed argument. */
   if (argv[pos][0] == '(' && argv[pos][1] == '\0')
     {
+      int nargs;
+
       advance (1);
-      value = expr ();
+
+      for (nargs = 1;
+          pos + nargs < argc && ! STREQ (argv[pos + nargs], ")");
+          nargs++)
+       if (nargs == 4)
+         {
+           nargs = argc - pos;
+           break;
+         }
+             
+      value = posixtest (nargs);
       if (argv[pos] == 0)
        test_syntax_error (_("')' expected\n"), NULL);
       else
@@ -330,9 +347,10 @@ term (void)
     }
 
   /* are there enough arguments left that this could be dyadic? */
-  if (((pos + 3 <= argc) && binop (argv[pos + 1])) ||
-      ((pos + 4 <= argc && STREQ (argv[pos], "-l") && binop (argv[pos + 2]))))
-    value = binary_operator ();
+  if (pos + 4 <= argc && STREQ (argv[pos], "-l") && binop (argv[pos + 2]))
+    value = binary_operator (true);
+  else if (pos + 3 <= argc && binop (argv[pos + 1]))
+    value = binary_operator (false);
 
   /* Might be a switch type argument */
   else if (argv[pos][0] == '-' && argv[pos][1] && argv[pos][2] == '\0')
@@ -352,31 +370,18 @@ term (void)
 }
 
 static int
-binary_operator (void)
+binary_operator (bool l_is_l)
 {
   register int op;
   struct stat stat_buf, stat_spare;
   intmax_t l, r;
   int value;
-  /* Are the left and right integer expressions of the form '-l string'? */
-  int l_is_l, r_is_l;
-
-  if (strcmp (argv[pos], "-l") == 0)
-    {
-      l_is_l = 1;
-      op = pos + 2;
+  /* Is the right integer expression of the form '-l string'? */
+  int r_is_l;
 
-      /* Make sure that OP is still a valid binary operator. */
-      if ((op >= argc - 1) || (binop (argv[op]) == 0))
-       test_syntax_error (_("%s: binary operator expected\n"), argv[op]);
-
-      advance (0);
-    }
-  else
-    {
-      l_is_l = 0;
-      op = pos + 1;
-    }
+  if (l_is_l)
+    advance (0);
+  op = pos + 1;
 
   if ((op < argc - 2) && (strcmp (argv[op + 1], "-l") == 0))
     {
@@ -758,17 +763,9 @@ unary_operator (void)
     case 't':                  /* File (fd) is a terminal? */
       {
        intmax_t fd;
-       advance (0);
-       if (pos < argc)
-         {
-           if (!isint (argv[pos], &fd))
-             integer_expected_error (_("after -t"));
-           advance (0);
-         }
-       else
-         {
-           fd = 1;
-         }
+       unary_advance ();
+       if (!isint (argv[pos - 1], &fd))
+         integer_expected_error (_("after -t"));
        return (TRUE == (fd == (int) fd && isatty (fd)));
       }
 
@@ -866,12 +863,9 @@ test_unop (char const *op)
 }
 
 static int
-one_argument (const char *s)
+one_argument (void)
 {
-  if (! getenv ("POSIXLY_CORRECT") && STREQ (s, "-t"))
-    return (TRUE == (isatty (1)));
-
-  return strlen (s) != 0;
+  return argv[pos++][0] != '\0';
 }
 
 static int
@@ -880,7 +874,10 @@ two_arguments (void)
   int value;
 
   if (STREQ (argv[pos], "!"))
-    value = ! one_argument (argv[pos+1]);
+    {
+      advance (0);
+      value = ! one_argument ();
+    }
   else if (argv[pos][0] == '-'
           && argv[pos][1] != '\0'
           && argv[pos][2] == '\0')
@@ -900,18 +897,20 @@ three_arguments (void)
 {
   int value;
 
-  if (STREQ (argv[pos], "!"))
+  if (binop (argv[pos + 1]))
+    value = binary_operator (false);
+  else if (STREQ (argv[pos], "!"))
     {
       advance (1);
       value = !two_arguments ();
     }
-  else if (binop (argv[pos+1]))
+  else if (STREQ (argv[pos], "(") && STREQ (argv[pos + 2], ")"))
     {
-      value = binary_operator ();
-      pos = argc;
+      advance (0);
+      value = one_argument ();
+      advance (0);
     }
-  else if ((STREQ (argv[pos+1], "-a")) || (STREQ (argv[pos+1], "-o")) ||
-          (argv[pos][0] == '('))
+  else if (STREQ (argv[pos + 1], "-a") || STREQ (argv[pos + 1], "-o"))
     value = expr ();
   else
     test_syntax_error (_("%s: binary operator expected\n"), argv[pos+1]);
@@ -920,25 +919,18 @@ three_arguments (void)
 
 /* This is an implementation of a Posix.2 proposal by David Korn. */
 static int
-posixtest (void)
+posixtest (int nargs)
 {
   int value;
 
-  switch (argc - 1)    /* one extra passed in */
+  switch (nargs)
     {
-      case 0:
-       value = FALSE;
-       pos = argc;
-       break;
-
       case 1:
-       value = one_argument (argv[1]);
-       pos = argc;
+       value = one_argument ();
        break;
 
       case 2:
        value = two_arguments ();
-       pos = argc;
        break;
 
       case 3:
@@ -952,9 +944,18 @@ posixtest (void)
            value = !three_arguments ();
            break;
          }
+       if (STREQ (argv[pos], "(") && STREQ (argv[pos + 3], ")"))
+         {
+           advance (0);
+           value = two_arguments ();
+           advance (0);
+           break;
+         }
        /* FALLTHROUGH */
       case 5:
       default:
+       if (nargs <= 0)
+         abort ();
        value = expr ();
     }
 
@@ -972,13 +973,10 @@ usage (int status)
             program_name);
   else
     {
-      printf (_("\
-Usage: %s EXPRESSION\n\
-  or:  [ EXPRESSION ]\n\
-  or:  %s OPTION\n\
-"),
-             program_name, program_name);
       fputs (_("\
+Usage: test EXPRESSION\n\
+  or:  [ EXPRESSION ]\n\
+  or:  [ OPTION\n\
 Exit with the status determined by EXPRESSION.\n\
 \n\
 "), stdout);
@@ -1087,22 +1085,27 @@ main (int margc, char **margv)
   bindtextdomain (PACKAGE, LOCALEDIR);
   textdomain (PACKAGE);
 
+  exit_failure = TEST_FAILURE;
   atexit (close_stdout);
 #endif /* TEST_STANDALONE */
 
-  /* Recognize --help or --version unless POSIXLY_CORRECT is set.  */
-  if (! getenv ("POSIXLY_CORRECT"))
-    parse_long_options (margc, margv, PROGRAM_NAME, GNU_PACKAGE, VERSION,
-                       AUTHORS, usage);
-
   argv = margv;
 
   if (LBRACKET)
     {
-      --margc;
+      /* Recognize --help or --version, but only when invoked in the
+        "[" form, and when the last argument is not "]".  POSIX
+        allows "[ --help" and "[ --version" to have the usual GNU
+        behavior, but it requires "test --help" and "test --version"
+        to exit silently with status 1.  */
+      if (margc < 2 || strcmp (margv[margc - 1], "]") != 0)
+       {
+         parse_long_options (margc, margv, PROGRAM_NAME, GNU_PACKAGE, VERSION,
+                             AUTHORS, usage);
+         test_syntax_error (_("missing `]'\n"), NULL);
+       }
 
-      if (margc < 1 || strcmp (margv[margc], "]") != 0)
-       test_syntax_error (_("missing `]'\n"), NULL);
+      --margc;
     }
 
   argc = margc;
@@ -1111,7 +1114,7 @@ main (int margc, char **margv)
   if (pos >= argc)
     test_exit (SHELL_BOOLEAN (FALSE));
 
-  value = posixtest ();
+  value = posixtest (argc - 1);
 
   if (pos != argc)
     test_syntax_error (_("too many arguments\n"), NULL);
Index: tests/test/Test.pm
===================================================================
RCS file: /cvsroot/coreutils/coreutils/tests/test/Test.pm,v
retrieving revision 1.4
diff -p -u -r1.4 Test.pm
--- tests/test/Test.pm  7 Jun 1998 14:40:04 -0000       1.4
+++ tests/test/Test.pm  24 Jul 2003 22:16:35 -0000
@@ -13,21 +13,36 @@ sub test_vector
      ['1b', "-z ''", {}, '', 0],
      ['1c', 'any-string', {}, '', 0],
      ['1d', '-n any-string', {}, '', 0],
+     ['1e', "''", {}, '', 1],
+     ['1f', '-', {}, '', 0],
+     ['1g', '--', {}, '', 0],
+     ['1h', '-0', {}, '', 0],
+     ['1i', '-f', {}, '', 0],
+     ['1j', '--help', {}, '', 0],
+     ['1k', '--version', {}, '', 0],
 
      ['streq-1', 't = t', {}, '', 0],
      ['streq-2', 't = f', {}, '', 1],
+     ['streq-3', '! = !', {}, '', 0],
+     ['streq-4', '= = =', {}, '', 0],
+     ['streq-5', "'(' = '('", {}, '', 0],
+     ['streq-6', "'(' = ')'", {}, '', 1],
      ['strne-1', 't != t', {}, '', 1],
      ['strne-2', 't != f', {}, '', 0],
+     ['strne-3', '! != !', {}, '', 1],
+     ['strne-4', '= != =', {}, '', 1],
+     ['strne-5', "'(' != '('", {}, '', 1],
+     ['strne-6', "'(' != ')'", {}, '', 0],
 
      ['and-1', 't -a t', {}, '', 0],
-     ['and-2', '"" -a t', {}, '', 1],
-     ['and-3', 't -a ""', {}, '', 1],
-     ['and-4', '"" -a ""', {}, '', 1],
+     ['and-2', "'' -a t", {}, '', 1],
+     ['and-3', "t -a ''", {}, '', 1],
+     ['and-4', "'' -a ''", {}, '', 1],
 
      ['or-1', 't -o t', {}, '', 0],
-     ['or-2', '"" -o t', {}, '', 0],
-     ['or-3', 't -o ""', {}, '', 0],
-     ['or-4', '"" -o ""', {}, '', 1],
+     ['or-2', "'' -o t", {}, '', 0],
+     ['or-3', "t -o ''", {}, '', 0],
+     ['or-4', "'' -o ''", {}, '', 1],
 
      ['eq-1', '9 -eq 9', {}, '', 0],
      ['eq-2', '0 -eq 0', {}, '', 0],
@@ -46,10 +61,16 @@ sub test_vector
      ['lt-4', '-1 -lt -2', {}, '', 1],
 
      # This evokes `test: 0x0: integer expression expected'.
-     ['inv-1', '0x0 -eq 00', {}, '', 1],
+     ['inv-1', '0x0 -eq 00', {}, '', 2],
 
-     ['t1', "-t", {}, '', 1],
+     ['t1', "-t", {}, '', 0],
      ['t2', "-t 1", {}, '', 1],
+
+     ['paren-1', "'(' '' ')'", {}, '', 1],
+     ['paren-2', "'(' '(' ')'", {}, '', 0],
+     ['paren-3', "'(' ')' ')'", {}, '', 0],
+     ['paren-4', "'(' ! ')'", {}, '', 0],
+     ['paren-5', "'(' -a ')'", {}, '', 0],
     );
 
   my %inverse_op =
@@ -81,15 +102,24 @@ sub test_vector
        }
     }
 
-  # Generate a negated and a double-negated version of each test.
+  # Generate parenthesized and negated versions of each test.
   # There are a few exceptions.
-  my %not_invertible = map {$_ => 1} qw (1a inv-1 t1);
+  my %not_N   = map {$_ => 1} qw (1a);
+  my %not_P   = map {$_ => 1} qw (1a
+                                 streq-6 strne-6
+                                 paren-1 paren-2 paren-3 paren-4 paren-5);
   foreach $t (@tvec)
     {
       my ($test_name, $flags, $in, $exp, $ret) = @$t;
-      next if $not_invertible{$test_name};
-      push (@tv, ["N-$test_name", "! '(' $flags ')'", $in, $exp, 1 - $ret]);
-      push (@tv, ["NN-$test_name", "! ! '(' $flags ')'", $in, $exp, $ret]);
+      next if $ret == 2;
+      push (@tv, ["N-$test_name", "! $flags", $in, $exp, 1 - $ret])
+         unless $not_N{$test_name};
+      push (@tv, ["P-$test_name", "'(' $flags ')'", $in, $exp, $ret])
+         unless $not_P{$test_name};
+      push (@tv, ["NP-$test_name", "! '(' $flags ')'", $in, $exp, 1 - $ret])
+         unless $not_P{$test_name};
+      push (@tv, ["NNP-$test_name", "! ! '(' $flags ')'", $in, $exp, $ret])
+         unless $not_P{$test_name};
     }
 
   return (@tv, @tvec);