coreutils "test" int fixups

[Paul Eggert]

I installed this.  The only bug here is an entirely implausible one:
there can be an integer overflow if "test" is invoked with INT_MAX
arguments.

2004-08-03  Paul Eggert  <address@hidden>

        * src/test.c (TRUE, FALSE, SHELL_BOOLEAN, TRUTH_OR, TRUTH_AND):
        Remove.  All uses replaced by C99 boolean primitives.
        (TEST_TRUE, TEST_FALSE): New constants, for readability.
        (test_unop, binop, unary_operator, binary_operator, two_arguments,
        three_arguments, posixtest, expr, term, and, or, is_int, age_of,
        one_argument, main): Use bool for booleans.
        (advance, unary_advance): Now inline procedures rather than a macros.
        (is_int): Renamed from isint, to avoid namespace clash with ctype.h.
        (term, and, or): When it's easy, loop instead of recursing.
        (term): Avoid integer overflow if there are INT_MAX-3 args (!).
        (binary_operator, unary_operator): Simplify by systematically rewriting
        true==FOO to FOO (where FOO is a boolean).
        (unary_operator): Don't consider a file to be a regular file
        merely because its mode&S_IFMT is zero.  Just use S_ISREG.
        Remove unnecessary casts.  Remove ifdefs for things like
        S_ISSOCK that are no longer needed, since stat-macros.h always
        defines them now.

Index: test.c
===================================================================
RCS file: /home/eggert/coreutils/cu/src/test.c,v
retrieving revision 1.103
retrieving revision 1.104
diff -p -u -r1.103 -r1.104
--- test.c      25 Jul 2004 07:39:02 -0000      1.103
+++ test.c      3 Aug 2004 23:03:04 -0000       1.104
@@ -61,21 +61,8 @@ char *program_name;
 extern gid_t getegid ();
 extern uid_t geteuid ();
 
-/* 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.
-   TRUTH_OR is how to do logical or with TRUE and FALSE.
-   TRUTH_AND is how to do logical and with TRUE and FALSE..
-   Default is TRUE = 1, FALSE = 0, TRUTH_OR = a | b, TRUTH_AND = a & b,
-    SHELL_BOOLEAN = (!value). */
-#define TRUE 1
-#define FALSE 0
-#define SHELL_BOOLEAN(value) (!(value))
-#define TRUTH_OR(a, b) ((a) | (b))
-#define TRUTH_AND(a, b) ((a) & (b))
-
 /* Exit status for syntax errors, etc.  */
-enum { TEST_FAILURE = 2 };
+enum { TEST_TRUE, TEST_FALSE, TEST_FAILURE };
 
 #if defined (TEST_STANDALONE)
 # define test_exit(val) exit (val)
@@ -89,18 +76,18 @@ static int pos;             /* The offset of the cu
 static int argc;       /* The number of arguments present in ARGV. */
 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 (bool);
-static int two_arguments (void);
-static int three_arguments (void);
-static int posixtest (int);
-
-static int expr (void);
-static int term (void);
-static int and (void);
-static int or (void);
+static bool test_unop (char const *s);
+static bool binop (char *s);
+static bool unary_operator (void);
+static bool binary_operator (bool);
+static bool two_arguments (void);
+static bool three_arguments (void);
+static bool posixtest (int);
+
+static bool expr (void);
+static bool term (void);
+static bool and (void);
+static bool or (void);
 
 static void test_syntax_error (char const *format, char const *arg)
      ATTRIBUTE_NORETURN;
@@ -117,34 +104,23 @@ test_syntax_error (char const *format, c
 
 /* Increment our position in the argument list.  Check that we're not
    past the end of the argument list.  This check is supressed if the
-   argument is FALSE.  Made a macro for efficiency. */
-#define advance(f)                                                     \
-  do                                                                   \
-    {                                                                  \
-      ++pos;                                                           \
-      if ((f) && pos >= argc)                                          \
-       beyond ();                                                      \
-    }                                                                  \
-  while (0)
-
-#if !defined (advance)
-static int
-advance (int f)
+   argument is false.  */
+
+static inline void
+advance (bool f)
 {
   ++pos;
 
   if (f && pos >= argc)
     beyond ();
 }
-#endif /* advance */
 
-#define unary_advance()                                                        
\
-  do                                                                   \
-    {                                                                  \
-      advance (1);                                                     \
-      ++pos;                                                           \
-    }                                                                  \
-  while (0)
+static inline void
+unary_advance (void)
+{
+  advance (true);
+  ++pos;
+}
 
 /*
  * beyond - call when we're beyond the end of the argument list (an
@@ -164,11 +140,11 @@ integer_expected_error (char const *pch)
   test_syntax_error (_("%s: integer expression expected\n"), pch);
 }
 
-/* Return nonzero if the characters pointed to by STRING constitute a
+/* Return true if the characters pointed to by STRING constitute a
    valid number.  Stuff the converted number into RESULT if RESULT is
    not null.  */
-static int
-isint (register char *string, intmax_t *result)
+static bool
+is_int (register char *string, intmax_t *result)
 {
   int sign;
   intmax_t value;
@@ -184,13 +160,13 @@ isint (register char *string, intmax_t *
     string++;
 
   if (!*string)
-    return (0);
+    return false;
 
   /* We allow leading `-' or `+'. */
   if (*string == '-' || *string == '+')
     {
       if (!digit (string[1]))
-       return (0);
+       return false;
 
       if (*string == '-')
        sign = -1;
@@ -211,7 +187,7 @@ isint (register char *string, intmax_t *
 
   /* Error if not at end of string. */
   if (*string)
-    return (0);
+    return false;
 
   if (result)
     {
@@ -219,19 +195,19 @@ isint (register char *string, intmax_t *
       *result = value;
     }
 
-  return (1);
+  return true;
 }
 
 /* Find the modification time of FILE, and stuff it into *AGE.
-   Return 0 if successful, -1 if not.  */
-static int
+   Return true if successful.  */
+static bool
 age_of (char *filename, time_t *age)
 {
   struct stat finfo;
-  int r = stat (filename, &finfo);
-  if (r == 0)
+  bool ok = (stat (filename, &finfo) == 0);
+  if (ok)
     *age = finfo.st_mtime;
-  return r;
+  return ok;
 }
 
 /*
@@ -252,33 +228,28 @@ age_of (char *filename, time_t *age)
  *     '-l' string
  *     positive and negative integers
  */
-static int
+static bool
 term (void)
 {
-  int value;
+  bool value;
+  bool negated = false;
 
-  if (pos >= argc)
-    beyond ();
-
-  /* Deal with leading `not's. */
-  if (argv[pos][0] == '!' && argv[pos][1] == '\0')
+  /* Deal with leading `not's.  */
+  while (pos < argc && argv[pos][0] == '!' && argv[pos][1] == '\0')
     {
-      value = 0;
-      while (pos < argc && argv[pos][0] == '!' && argv[pos][1] == '\0')
-       {
-         advance (1);
-         value = 1 - value;
-       }
-
-      return (value ? !term() : term());
+      advance (true);
+      negated = !negated;
     }
 
+  if (pos >= argc)
+    beyond ();
+
   /* A paren-bracketed argument. */
   if (argv[pos][0] == '(' && argv[pos][1] == '\0')
     {
       int nargs;
 
-      advance (1);
+      advance (true);
 
       for (nargs = 1;
           pos + nargs < argc && ! STREQ (argv[pos + nargs], ")");
@@ -295,17 +266,16 @@ term (void)
       else
         if (argv[pos][0] != ')' || argv[pos][1])
          test_syntax_error (_("')' expected, found %s\n"), argv[pos]);
-      advance (0);
-      return (value);
+      advance (false);
     }
 
-  /* are there enough arguments left that this could be dyadic? */
-  if (pos + 4 <= argc && STREQ (argv[pos], "-l") && binop (argv[pos + 2]))
+  /* Are there enough arguments left that this could be dyadic?  */
+  else if (4 <= argc - pos && STREQ (argv[pos], "-l") && binop (argv[pos + 2]))
     value = binary_operator (true);
-  else if (pos + 3 <= argc && binop (argv[pos + 1]))
+  else if (3 <= argc - pos && binop (argv[pos + 1]))
     value = binary_operator (false);
 
-  /* Might be a switch type argument */
+  /* It might be a switch type argument.  */
   else if (argv[pos][0] == '-' && argv[pos][1] && argv[pos][2] == '\0')
     {
       if (test_unop (argv[pos]))
@@ -316,33 +286,32 @@ term (void)
   else
     {
       value = (argv[pos][0] != '\0');
-      advance (0);
+      advance (false);
     }
 
-  return (value);
+  return negated ^ value;
 }
 
-static int
+static bool
 binary_operator (bool l_is_l)
 {
   register int op;
   struct stat stat_buf, stat_spare;
   intmax_t l, r;
-  int value;
   /* Is the right integer expression of the form '-l string'? */
-  int r_is_l;
+  bool r_is_l;
 
   if (l_is_l)
-    advance (0);
+    advance (false);
   op = pos + 1;
 
   if ((op < argc - 2) && STREQ (argv[op + 1], "-l"))
     {
-      r_is_l = 1;
-      advance (0);
+      r_is_l = true;
+      advance (false);
     }
   else
-    r_is_l = 0;
+    r_is_l = false;
 
   if (argv[op][0] == '-')
     {
@@ -360,7 +329,7 @@ binary_operator (bool l_is_l)
                l = strlen (argv[op - 1]);
              else
                {
-                 if (!isint (argv[op - 1], &l))
+                 if (!is_int (argv[op - 1], &l))
                    integer_expected_error (_("before -lt"));
                }
 
@@ -368,11 +337,11 @@ binary_operator (bool l_is_l)
                r = strlen (argv[op + 2]);
              else
                {
-                 if (!isint (argv[op + 1], &r))
+                 if (!is_int (argv[op + 1], &r))
                    integer_expected_error (_("after -lt"));
                }
              pos += 3;
-             return (TRUE == (l < r));
+             return l < r;
            }
 
          if (argv[op][2] == 'e' && !argv[op][3])
@@ -382,18 +351,18 @@ binary_operator (bool l_is_l)
                l = strlen (argv[op - 1]);
              else
                {
-                 if (!isint (argv[op - 1], &l))
+                 if (!is_int (argv[op - 1], &l))
                    integer_expected_error (_("before -le"));
                }
              if (r_is_l)
                r = strlen (argv[op + 2]);
              else
                {
-                 if (!isint (argv[op + 1], &r))
+                 if (!is_int (argv[op + 1], &r))
                    integer_expected_error (_("after -le"));
                }
              pos += 3;
-             return (TRUE == (l <= r));
+             return l <= r;
            }
          break;
 
@@ -405,18 +374,18 @@ binary_operator (bool l_is_l)
                l = strlen (argv[op - 1]);
              else
                {
-                 if (!isint (argv[op - 1], &l))
+                 if (!is_int (argv[op - 1], &l))
                    integer_expected_error (_("before -gt"));
                }
              if (r_is_l)
                r = strlen (argv[op + 2]);
              else
                {
-                 if (!isint (argv[op + 1], &r))
+                 if (!is_int (argv[op + 1], &r))
                    integer_expected_error (_("after -gt"));
                }
              pos += 3;
-             return (TRUE == (l > r));
+             return l > r;
            }
 
          if (argv[op][2] == 'e' && !argv[op][3])
@@ -426,18 +395,18 @@ binary_operator (bool l_is_l)
                l = strlen (argv[op - 1]);
              else
                {
-                 if (!isint (argv[op - 1], &l))
+                 if (!is_int (argv[op - 1], &l))
                    integer_expected_error (_("before -ge"));
                }
              if (r_is_l)
                r = strlen (argv[op + 2]);
              else
                {
-                 if (!isint (argv[op + 1], &r))
+                 if (!is_int (argv[op + 1], &r))
                    integer_expected_error (_("after -ge"));
                }
              pos += 3;
-             return (TRUE == (l >= r));
+             return l >= r;
            }
          break;
 
@@ -446,13 +415,13 @@ binary_operator (bool l_is_l)
            {
              /* nt - newer than */
              time_t lt, rt;
-             int le, re;
+             bool le, re;
              pos += 3;
-             if (l_is_l || r_is_l)
+             if (l_is_l | r_is_l)
                test_syntax_error (_("-nt does not accept -l\n"), NULL);
              le = age_of (argv[op - 1], &lt);
              re = age_of (argv[op + 1], &rt);
-             return le > re || (le == 0 && lt > rt);
+             return le > re || (le && lt > rt);
            }
 
          if (argv[op][2] == 'e' && !argv[op][3])
@@ -462,18 +431,18 @@ binary_operator (bool l_is_l)
                l = strlen (argv[op - 1]);
              else
                {
-                 if (!isint (argv[op - 1], &l))
+                 if (!is_int (argv[op - 1], &l))
                    integer_expected_error (_("before -ne"));
                }
              if (r_is_l)
                r = strlen (argv[op + 2]);
              else
                {
-                 if (!isint (argv[op + 1], &r))
+                 if (!is_int (argv[op + 1], &r))
                    integer_expected_error (_("after -ne"));
                }
              pos += 3;
-             return (TRUE == (l != r));
+             return l != r;
            }
          break;
 
@@ -485,33 +454,30 @@ binary_operator (bool l_is_l)
                l = strlen (argv[op - 1]);
              else
                {
-                 if (!isint (argv[op - 1], &l))
+                 if (!is_int (argv[op - 1], &l))
                    integer_expected_error (_("before -eq"));
                }
              if (r_is_l)
                r = strlen (argv[op + 2]);
              else
                {
-                 if (!isint (argv[op + 1], &r))
+                 if (!is_int (argv[op + 1], &r))
                    integer_expected_error (_("after -eq"));
                }
              pos += 3;
-             return (TRUE == (l == r));
+             return l == r;
            }
 
          if (argv[op][2] == 'f' && !argv[op][3])
            {
              /* ef - hard link? */
              pos += 3;
-             if (l_is_l || r_is_l)
+             if (l_is_l | r_is_l)
                test_syntax_error (_("-ef does not accept -l\n"), NULL);
-             if (stat (argv[op - 1], &stat_buf) < 0)
-               return (FALSE);
-             if (stat (argv[op + 1], &stat_spare) < 0)
-               return (FALSE);
-             return (TRUE ==
-                     (stat_buf.st_dev == stat_spare.st_dev &&
-                      stat_buf.st_ino == stat_spare.st_ino));
+             return (stat (argv[op - 1], &stat_buf) == 0
+                     && stat (argv[op + 1], &stat_spare) == 0
+                     && stat_buf.st_dev == stat_spare.st_dev
+                     && stat_buf.st_ino == stat_spare.st_ino);
            }
          break;
 
@@ -520,13 +486,13 @@ binary_operator (bool l_is_l)
            {
              /* ot - older than */
              time_t lt, rt;
-             int le, re;
+             bool le, re;
              pos += 3;
-             if (l_is_l || r_is_l)
+             if (l_is_l | r_is_l)
                test_syntax_error (_("-ot does not accept -l\n"), NULL);
              le = age_of (argv[op - 1], &lt);
              re = age_of (argv[op + 1], &rt);
-             return le < re || (re == 0 && lt < rt);
+             return le < re || (re && lt < rt);
            }
          break;
        }
@@ -537,32 +503,31 @@ binary_operator (bool l_is_l)
 
   if (argv[op][0] == '=' && !argv[op][1])
     {
-      value = STREQ (argv[pos], argv[pos + 2]);
+      bool value = STREQ (argv[pos], argv[pos + 2]);
       pos += 3;
-      return (TRUE == value);
+      return value;
     }
 
   if (STREQ (argv[op], "!="))
     {
-      value = !STREQ (argv[pos], argv[pos + 2]);
+      bool value = !STREQ (argv[pos], argv[pos + 2]);
       pos += 3;
-      return (TRUE == value);
+      return value;
     }
 
   /* Not reached.  */
   abort ();
 }
 
-static int
+static bool
 unary_operator (void)
 {
-  int value;
   struct stat stat_buf;
 
   switch (argv[pos][1])
     {
     default:
-      return (FALSE);
+      return false;
 
       /* All of the following unary operators use unary_advance (), which
         checks to make sure that there is an argument, and then advances
@@ -572,163 +537,106 @@ unary_operator (void)
     case 'a':                  /* file exists in the file system? */
     case 'e':
       unary_advance ();
-      value = -1 != stat (argv[pos - 1], &stat_buf);
-      return (TRUE == value);
+      return stat (argv[pos - 1], &stat_buf) == 0;
 
     case 'r':                  /* file is readable? */
       unary_advance ();
-      value = -1 != euidaccess (argv[pos - 1], R_OK);
-      return (TRUE == value);
+      return euidaccess (argv[pos - 1], R_OK) == 0;
 
     case 'w':                  /* File is writable? */
       unary_advance ();
-      value = -1 != euidaccess (argv[pos - 1], W_OK);
-      return (TRUE == value);
+      return euidaccess (argv[pos - 1], W_OK) == 0;
 
     case 'x':                  /* File is executable? */
       unary_advance ();
-      value = -1 != euidaccess (argv[pos - 1], X_OK);
-      return (TRUE == value);
+      return euidaccess (argv[pos - 1], X_OK) == 0;
 
     case 'O':                  /* File is owned by you? */
       unary_advance ();
-      if (stat (argv[pos - 1], &stat_buf) < 0)
-       return (FALSE);
-
-      return (TRUE == (geteuid () == stat_buf.st_uid));
+      return (stat (argv[pos - 1], &stat_buf) == 0
+             && (geteuid () == stat_buf.st_uid));
 
     case 'G':                  /* File is owned by your group? */
       unary_advance ();
-      if (stat (argv[pos - 1], &stat_buf) < 0)
-       return (FALSE);
-
-      return (TRUE == (getegid () == stat_buf.st_gid));
+      return (stat (argv[pos - 1], &stat_buf) == 0
+             && (getegid () == stat_buf.st_gid));
 
     case 'f':                  /* File is a file? */
       unary_advance ();
-      if (stat (argv[pos - 1], &stat_buf) < 0)
-       return (FALSE);
-
       /* Under POSIX, -f is true if the given file exists
         and is a regular file. */
-      return (TRUE == ((S_ISREG (stat_buf.st_mode)) ||
-                      (0 == (stat_buf.st_mode & S_IFMT))));
+      return (stat (argv[pos - 1], &stat_buf) == 0
+             && S_ISREG (stat_buf.st_mode));
 
     case 'd':                  /* File is a directory? */
       unary_advance ();
-      if (stat (argv[pos - 1], &stat_buf) < 0)
-       return (FALSE);
-
-      return (TRUE == (S_ISDIR (stat_buf.st_mode)));
+      return (stat (argv[pos - 1], &stat_buf) == 0
+             && S_ISDIR (stat_buf.st_mode));
 
     case 's':                  /* File has something in it? */
       unary_advance ();
-      if (stat (argv[pos - 1], &stat_buf) < 0)
-       return (FALSE);
-
-      return (TRUE == (stat_buf.st_size > (off_t) 0));
+      return (stat (argv[pos - 1], &stat_buf) == 0
+             && 0 < stat_buf.st_size);
 
     case 'S':                  /* File is a socket? */
-#if !defined (S_ISSOCK)
-      return (FALSE);
-#else
       unary_advance ();
-
-      if (stat (argv[pos - 1], &stat_buf) < 0)
-       return (FALSE);
-
-      return (TRUE == (S_ISSOCK (stat_buf.st_mode)));
-#endif                         /* S_ISSOCK */
+      return (stat (argv[pos - 1], &stat_buf) == 0
+             && S_ISSOCK (stat_buf.st_mode));
 
     case 'c':                  /* File is character special? */
       unary_advance ();
-      if (stat (argv[pos - 1], &stat_buf) < 0)
-       return (FALSE);
-
-      return (TRUE == (S_ISCHR (stat_buf.st_mode)));
+      return (stat (argv[pos - 1], &stat_buf) == 0
+             && S_ISCHR (stat_buf.st_mode));
 
     case 'b':                  /* File is block special? */
       unary_advance ();
-      if (stat (argv[pos - 1], &stat_buf) < 0)
-       return (FALSE);
-
-      return (TRUE == (S_ISBLK (stat_buf.st_mode)));
+      return (stat (argv[pos - 1], &stat_buf) == 0
+             && S_ISBLK (stat_buf.st_mode));
 
     case 'p':                  /* File is a named pipe? */
       unary_advance ();
-#ifndef S_ISFIFO
-      return (FALSE);
-#else
-      if (stat (argv[pos - 1], &stat_buf) < 0)
-       return (FALSE);
-      return (TRUE == (S_ISFIFO (stat_buf.st_mode)));
-#endif                         /* S_ISFIFO */
+      return (stat (argv[pos - 1], &stat_buf) == 0
+             && S_ISFIFO (stat_buf.st_mode));
 
     case 'L':                  /* Same as -h  */
       /*FALLTHROUGH*/
 
     case 'h':                  /* File is a symbolic link? */
       unary_advance ();
-#ifndef S_ISLNK
-      return (FALSE);
-#else
-      /* An empty filename is not a valid pathname. */
-      if ((argv[pos - 1][0] == '\0') ||
-         (lstat (argv[pos - 1], &stat_buf) < 0))
-       return (FALSE);
-
-      return (TRUE == (S_ISLNK (stat_buf.st_mode)));
-#endif                         /* S_IFLNK */
+      return (lstat (argv[pos - 1], &stat_buf) == 0
+             && S_ISLNK (stat_buf.st_mode));
 
     case 'u':                  /* File is setuid? */
       unary_advance ();
-#ifndef S_ISUID
-      return (FALSE);
-#else
-      if (stat (argv[pos - 1], &stat_buf) < 0)
-       return (FALSE);
-
-      return (TRUE == (0 != (stat_buf.st_mode & S_ISUID)));
-#endif
+      return (stat (argv[pos - 1], &stat_buf) == 0
+             && (stat_buf.st_mode & S_ISUID));
 
     case 'g':                  /* File is setgid? */
       unary_advance ();
-#ifndef S_ISGID
-      return (FALSE);
-#else
-      if (stat (argv[pos - 1], &stat_buf) < 0)
-       return (FALSE);
-
-      return (TRUE == (0 != (stat_buf.st_mode & S_ISGID)));
-#endif
+      return (stat (argv[pos - 1], &stat_buf) == 0
+             && (stat_buf.st_mode & S_ISGID));
 
     case 'k':                  /* File has sticky bit set? */
       unary_advance ();
-      if (stat (argv[pos - 1], &stat_buf) < 0)
-       return (FALSE);
-#ifndef S_ISVTX
-      /* This is not Posix, and is not defined on some Posix systems. */
-      return (FALSE);
-#else
-      return (TRUE == (0 != (stat_buf.st_mode & S_ISVTX)));
-#endif
+      return (stat (argv[pos - 1], &stat_buf) == 0
+             && (stat_buf.st_mode & S_ISVTX));
 
     case 't':                  /* File (fd) is a terminal? */
       {
        intmax_t fd;
        unary_advance ();
-       if (!isint (argv[pos - 1], &fd))
+       if (!is_int (argv[pos - 1], &fd))
          integer_expected_error (_("after -t"));
-       return (TRUE == (fd == (int) fd && isatty (fd)));
+       return INT_MIN <= fd && fd <= INT_MAX && isatty (fd);
       }
 
     case 'n':                  /* True if arg has some length. */
       unary_advance ();
-      return (TRUE == (argv[pos - 1][0] != 0));
+      return argv[pos - 1][0] != 0;
 
     case 'z':                  /* True if arg has no length. */
       unary_advance ();
-      return (TRUE == (argv[pos - 1][0] == '\0'));
+      return argv[pos - 1][0] == '\0';
     }
 }
 
@@ -737,18 +645,18 @@ unary_operator (void)
  *     term
  *     term '-a' and
  */
-static int
+static bool
 and (void)
 {
-  int value;
+  bool value = true;
 
-  value = term ();
-  while ((pos < argc) && STREQ (argv[pos], "-a"))
+  for (;;)
     {
-      advance (0);
-      value = TRUTH_AND (value, and ());
+      value &= term ();
+      if (! (pos < argc && STREQ (argv[pos], "-a")))
+       return value;
+      advance (false);
     }
-  return (TRUE == value);
 }
 
 /*
@@ -756,37 +664,35 @@ and (void)
  *     and
  *     and '-o' or
  */
-static int
+static bool
 or (void)
 {
-  int value;
+  bool value = false;
 
-  value = and ();
-
-  while ((pos < argc) && STREQ (argv[pos], "-o"))
+  for (;;)
     {
-      advance (0);
-      value = TRUTH_OR (value, or ());
+      value |= and ();
+      if (! (pos < argc && STREQ (argv[pos], "-o")))
+       return value;
+      advance (false);
     }
-
-  return (TRUE == value);
 }
 
 /*
  * expr:
  *     or
  */
-static int
+static bool
 expr (void)
 {
   if (pos >= argc)
     beyond ();
 
-  return (FALSE ^ (or ()));            /* Same with this. */
+  return or ();                /* Same with this. */
 }
 
-/* Return TRUE if S is one of the test command's binary operators. */
-static int
+/* Return true if S is one of the test command's binary operators.  */
+static bool
 binop (char *s)
 {
   return ((STREQ (s,   "=")) || (STREQ (s,  "!=")) || (STREQ (s, "-nt")) ||
@@ -795,12 +701,12 @@ binop (char *s)
          (STREQ (s, "-gt")) || (STREQ (s, "-ge")));
 }
 
-/* Return nonzero if OP is one of the test command's unary operators. */
-static int
+/* Return true if OP is one of the test command's unary operators. */
+static bool
 test_unop (char const *op)
 {
   if (op[0] != '-')
-    return (0);
+    return false;
 
   switch (op[1])
     {
@@ -809,26 +715,26 @@ test_unop (char const *op)
     case 'o': case 'p': case 'r': case 's': case 't':
     case 'u': case 'w': case 'x': case 'z':
     case 'G': case 'L': case 'O': case 'S': case 'N':
-      return (1);
+      return true;
     }
 
-  return (0);
+  return false;
 }
 
-static int
+static bool
 one_argument (void)
 {
   return argv[pos++][0] != '\0';
 }
 
-static int
+static bool
 two_arguments (void)
 {
-  int value;
+  bool value;
 
   if (STREQ (argv[pos], "!"))
     {
-      advance (0);
+      advance (false);
       value = ! one_argument ();
     }
   else if (argv[pos][0] == '-'
@@ -845,23 +751,23 @@ two_arguments (void)
   return (value);
 }
 
-static int
+static bool
 three_arguments (void)
 {
-  int value;
+  bool value;
 
   if (binop (argv[pos + 1]))
     value = binary_operator (false);
   else if (STREQ (argv[pos], "!"))
     {
-      advance (1);
+      advance (true);
       value = !two_arguments ();
     }
   else if (STREQ (argv[pos], "(") && STREQ (argv[pos + 2], ")"))
     {
-      advance (0);
+      advance (false);
       value = one_argument ();
-      advance (0);
+      advance (false);
     }
   else if (STREQ (argv[pos + 1], "-a") || STREQ (argv[pos + 1], "-o"))
     value = expr ();
@@ -871,10 +777,10 @@ three_arguments (void)
 }
 
 /* This is an implementation of a Posix.2 proposal by David Korn. */
-static int
+static bool
 posixtest (int nargs)
 {
-  int value;
+  bool value;
 
   switch (nargs)
     {
@@ -893,15 +799,15 @@ posixtest (int nargs)
       case 4:
        if (STREQ (argv[pos], "!"))
          {
-           advance (1);
+           advance (true);
            value = !three_arguments ();
            break;
          }
        if (STREQ (argv[pos], "(") && STREQ (argv[pos + 3], ")"))
          {
-           advance (0);
+           advance (false);
            value = two_arguments ();
-           advance (0);
+           advance (false);
            break;
          }
        /* FALLTHROUGH */
@@ -1022,7 +928,7 @@ INTEGER may also be -l STRING, which eva
 int
 main (int margc, char **margv)
 {
-  int value;
+  bool value;
 
 #if !defined (TEST_STANDALONE)
   int code;
@@ -1065,12 +971,12 @@ main (int margc, char **margv)
   pos = 1;
 
   if (pos >= argc)
-    test_exit (SHELL_BOOLEAN (FALSE));
+    test_exit (TEST_FALSE);
 
   value = posixtest (argc - 1);
 
   if (pos != argc)
     test_syntax_error (_("extra argument %s"), quote (argv[pos]));
 
-  test_exit (SHELL_BOOLEAN (value));
+  test_exit (value ? TEST_TRUE : TEST_FALSE);
 }