EXIT_CANCELED (was: system.h cleanups)

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Eric Blake on 10/9/2009 6:28 AM:
>>>> nohup enums NOHUP_FAILURE=127 which clashes with EXIT_ENOENT?
>>> Required by POSIX to fail with 127 on internal failure (bummer).
> 
> If POSIX agrees with my bug report, then we can blindly use EXIT_CANCELED
> in nohup; if not, I'm almost willing to make it a POSIXLY_CORRECT issue
> (return 125 unless we're complying with POSIX).

Well, the Austin Group rejected it for the current version of POSIX, for
fear that returning something other than 127 would break existing
compliant applications.  They did, however, recommend that I try again
with wording that permits either 125 or 127 in the next version of POSIX
(however many years down the road).
(look for item #165)
https://www.opengroup.org/sophocles/show_mail.tpl?CALLER=index.tpl&source=L&listname=austin-group-l&id=12935

[That meeting also discussed trailing slash cleanups, so maybe I'll have
to review all my recent work to see whether Linux behavior and gnulib
wrappers still fit in with the new wording on when ENOTDIR should be
returned, item #146.]

So, how about this patch series?  (I guess I should check whether this
change causes any testsuite failures, and if not, add some tests.)

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkrhqR4ACgkQ84KuGfSFAYCLLQCgx81TYW/QdhFFG80AA5GjRg+B
gXgAoLMccYUihP5B3QBQTg4rdhZ5tRN1
=Balh
-----END PGP SIGNATURE-----

From ba7af14bf9d1809dbd135fd3440b26c24786ad3d Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Fri, 23 Oct 2009 06:59:02 -0600
Subject: [PATCH 1/2] chroot, env, nice, su: use EXIT_CANCELED for internal 
failure

* src/chroot.c (main): Use EXIT_CANCELED, not EXIT_FAILURE.
* src/env.c (main): Likewise.
* src/nice.c (main): Likewise.
* src/su.c (change_identity, main): Likewise.
* doc/coreutils.texi (chroot invocation, env invocation)
(nice invocation, su invocation): Document this.
* NEWS: Likewise.
---
 NEWS               |    7 +++++++
 doc/coreutils.texi |    8 ++++----
 src/chroot.c       |   14 +++++++-------
 src/env.c          |    4 ++--
 src/nice.c         |   14 +++++++-------
 src/su.c           |   14 +++++++-------
 6 files changed, 34 insertions(+), 27 deletions(-)

diff --git a/NEWS b/NEWS
index 7057d87..f359133 100644
--- a/NEWS
+++ b/NEWS
@@ -24,6 +24,13 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   were first renamed or unlinked or never modified.
   [The race was introduced in coreutils-7.5]

+** Changes in behavior
+
+  chroot, env, nice, and su fail with status 125, rather than 1, on
+  internal error such as failure to parse command line arguments; this
+  is for consistency with stdbuf and timeout, and avoids ambiguity
+  with the invoked command failing with status 1.
+
 ** New features

   md5sum --check now also accepts openssl-style checksums.
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 64e0e95..31c3d5c 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -14363,7 +14363,7 @@ chroot invocation
 Exit status:

 @display
-1   if @command{chroot} itself fails
+125 if @command{chroot} itself fails
 126 if @var{command} is found but cannot be invoked
 127 if @var{command} cannot be found
 the exit status of @var{command} otherwise
@@ -14441,7 +14441,7 @@ env invocation

 @display
 0   if no @var{command} is specified and the environment is output
-1   if @command{env} itself fails
+125 if @command{env} itself fails
 126 if @var{command} is found but cannot be invoked
 127 if @var{command} cannot be found
 the exit status of @var{command} otherwise
@@ -14516,7 +14516,7 @@ nice invocation

 @display
 0   if no @var{command} is specified and the niceness is output
-1   if @command{nice} itself fails
+125 if @command{nice} itself fails
 126 if @var{command} is found but cannot be invoked
 127 if @var{command} cannot be found
 the exit status of @var{command} otherwise
@@ -14843,7 +14843,7 @@ su invocation
 Exit status:

 @display
-1   if @command{su} itself fails
+125 if @command{su} itself fails
 126 if subshell is found but cannot be invoked
 127 if subshell cannot be found
 the exit status of the subshell otherwise
diff --git a/src/chroot.c b/src/chroot.c
index 9269f1b..e4765ba 100644
--- a/src/chroot.c
+++ b/src/chroot.c
@@ -160,7 +160,7 @@ main (int argc, char **argv)
   bindtextdomain (PACKAGE, LOCALEDIR);
   textdomain (PACKAGE);

-  initialize_exit_failure (EXIT_FAILURE);
+  initialize_exit_failure (EXIT_CANCELED);
   atexit (close_stdout);

   parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
@@ -177,22 +177,22 @@ main (int argc, char **argv)
           groups = optarg;
           break;
         default:
-          usage (EXIT_FAILURE);
+          usage (EXIT_CANCELED);
         }
     }

   if (argc <= optind)
     {
       error (0, 0, _("missing operand"));
-      usage (EXIT_FAILURE);
+      usage (EXIT_CANCELED);
     }

   if (chroot (argv[optind]) != 0)
-    error (EXIT_FAILURE, errno, _("cannot change root directory to %s"),
+    error (EXIT_CANCELED, errno, _("cannot change root directory to %s"),
            argv[optind]);

   if (chdir ("/"))
-    error (EXIT_FAILURE, errno, _("cannot chdir to root directory"));
+    error (EXIT_CANCELED, errno, _("cannot chdir to root directory"));

   if (argc == optind + 1)
     {
@@ -223,7 +223,7 @@ main (int argc, char **argv)
       char const *err = parse_user_spec (userspec, &uid, &gid, &user, &group);

       if (err)
-        error (EXIT_FAILURE, errno, "%s", err);
+        error (EXIT_CANCELED, errno, "%s", err);

       free (user);
       free (group);
@@ -254,7 +254,7 @@ main (int argc, char **argv)
     }

   if (fail)
-    exit (EXIT_FAILURE);
+    exit (EXIT_CANCELED);

   /* Execute the given command.  */
   execvp (argv[0], argv);
diff --git a/src/env.c b/src/env.c
index 90edc5a..b69a29a 100644
--- a/src/env.c
+++ b/src/env.c
@@ -142,7 +142,7 @@ main (int argc, char **argv)
   bindtextdomain (PACKAGE, LOCALEDIR);
   textdomain (PACKAGE);

-  initialize_exit_failure (EXIT_FAILURE);
+  initialize_exit_failure (EXIT_CANCELED);
   atexit (close_stdout);

   while ((optc = getopt_long (argc, argv, "+iu:", longopts, NULL)) != -1)
@@ -157,7 +157,7 @@ main (int argc, char **argv)
         case_GETOPT_HELP_CHAR;
         case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS);
         default:
-          usage (EXIT_FAILURE);
+          usage (EXIT_CANCELED);
         }
     }

diff --git a/src/nice.c b/src/nice.c
index 6cd5f31..b04f675 100644
--- a/src/nice.c
+++ b/src/nice.c
@@ -101,7 +101,7 @@ main (int argc, char **argv)
   bindtextdomain (PACKAGE, LOCALEDIR);
   textdomain (PACKAGE);

-  initialize_exit_failure (EXIT_FAILURE);
+  initialize_exit_failure (EXIT_CANCELED);
   atexit (close_stdout);

   parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
@@ -132,7 +132,7 @@ main (int argc, char **argv)
           i += optind - 1;

           if (optc == '?')
-            usage (EXIT_FAILURE);
+            usage (EXIT_CANCELED);
           else if (optc == 'n')
             adjustment_given = optarg;
           else /* optc == -1 */
@@ -148,7 +148,7 @@ main (int argc, char **argv)
       enum { MIN_ADJUSTMENT = 1 - 2 * NZERO, MAX_ADJUSTMENT = 2 * NZERO - 1 };
       long int tmp;
       if (LONGINT_OVERFLOW < xstrtol (adjustment_given, NULL, 10, &tmp, ""))
-        error (EXIT_FAILURE, 0, _("invalid adjustment %s"),
+        error (EXIT_CANCELED, 0, _("invalid adjustment %s"),
                quote (adjustment_given));
       adjustment = MAX (MIN_ADJUSTMENT, MIN (tmp, MAX_ADJUSTMENT));
     }
@@ -158,13 +158,13 @@ main (int argc, char **argv)
       if (adjustment_given)
         {
           error (0, 0, _("a command must be given with an adjustment"));
-          usage (EXIT_FAILURE);
+          usage (EXIT_CANCELED);
         }
       /* No command given; print the niceness.  */
       errno = 0;
       current_niceness = GET_NICENESS ();
       if (current_niceness == -1 && errno != 0)
-        error (EXIT_FAILURE, errno, _("cannot get niceness"));
+        error (EXIT_CANCELED, errno, _("cannot get niceness"));
       printf ("%d\n", current_niceness);
       exit (EXIT_SUCCESS);
     }
@@ -175,11 +175,11 @@ main (int argc, char **argv)
 #else
   current_niceness = GET_NICENESS ();
   if (current_niceness == -1 && errno != 0)
-    error (EXIT_FAILURE, errno, _("cannot get niceness"));
+    error (EXIT_CANCELED, errno, _("cannot get niceness"));
   ok = (setpriority (PRIO_PROCESS, 0, current_niceness + adjustment) == 0);
 #endif
   if (!ok)
-    error (errno == EPERM ? 0 : EXIT_FAILURE, errno, _("cannot set niceness"));
+    error (errno == EPERM ? 0 : EXIT_CANCELED, errno, _("cannot set 
niceness"));

   execvp (argv[i], &argv[i]);

diff --git a/src/su.c b/src/su.c
index 25b8838..0de67c9 100644
--- a/src/su.c
+++ b/src/su.c
@@ -286,13 +286,13 @@ change_identity (const struct passwd *pw)
 #ifdef HAVE_INITGROUPS
   errno = 0;
   if (initgroups (pw->pw_name, pw->pw_gid) == -1)
-    error (EXIT_FAILURE, errno, _("cannot set groups"));
+    error (EXIT_CANCELED, errno, _("cannot set groups"));
   endgrent ();
 #endif
   if (setgid (pw->pw_gid))
-    error (EXIT_FAILURE, errno, _("cannot set group id"));
+    error (EXIT_CANCELED, errno, _("cannot set group id"));
   if (setuid (pw->pw_uid))
-    error (EXIT_FAILURE, errno, _("cannot set user id"));
+    error (EXIT_CANCELED, errno, _("cannot set user id"));
 }

 /* Run SHELL, or DEFAULT_SHELL if SHELL is empty.
@@ -406,7 +406,7 @@ main (int argc, char **argv)
   bindtextdomain (PACKAGE, LOCALEDIR);
   textdomain (PACKAGE);

-  initialize_exit_failure (EXIT_FAILURE);
+  initialize_exit_failure (EXIT_CANCELED);
   atexit (close_stdout);

   fast_startup = false;
@@ -443,7 +443,7 @@ main (int argc, char **argv)
         case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS);

         default:
-          usage (EXIT_FAILURE);
+          usage (EXIT_CANCELED);
         }
     }

@@ -458,7 +458,7 @@ main (int argc, char **argv)
   pw = getpwnam (new_user);
   if (! (pw && pw->pw_name && pw->pw_name[0] && pw->pw_dir && pw->pw_dir[0]
          && pw->pw_passwd))
-    error (EXIT_FAILURE, 0, _("user %s does not exist"), new_user);
+    error (EXIT_CANCELED, 0, _("user %s does not exist"), new_user);

   /* Make a copy of the password information and point pw at the local
      copy instead.  Otherwise, some systems (e.g. GNU/Linux) would clobber
@@ -481,7 +481,7 @@ main (int argc, char **argv)
 #ifdef SYSLOG_FAILURE
       log_su (pw, false);
 #endif
-      error (EXIT_FAILURE, 0, _("incorrect password"));
+      error (EXIT_CANCELED, 0, _("incorrect password"));
     }
 #ifdef SYSLOG_SUCCESS
   else
-- 
1.6.5.rc1


From 47dc111827bfb852476fd0160b8a61c2e1562cbd Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Fri, 23 Oct 2009 06:59:23 -0600
Subject: [PATCH 2/2] nohup: use EXIT_CANCELED if not POSIXLY_CORRECT

* src/nohup.c (NOHUP_FAILURE): Rename...
(POSIX_NOHUP_FAILURE): ...to this.
(main): Pay attention to POSIXLY_CORRECT, to determine whether to
use status 125 or 127.
* doc/coreutils.texi (nohup invocation): Document this.
* NEWS: Likewise.
---
 NEWS               |    3 +++
 doc/coreutils.texi |    6 +++++-
 src/nohup.c        |   22 +++++++++++++++-------
 3 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/NEWS b/NEWS
index f359133..62590dd 100644
--- a/NEWS
+++ b/NEWS
@@ -31,6 +31,9 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   is for consistency with stdbuf and timeout, and avoids ambiguity
   with the invoked command failing with status 1.

+  nohup fails with status 125, rather than 127, on internal error when
+  POSIXLY_CORRECT is not set, for consistency with other commands.
+
 ** New features

   md5sum --check now also accepts openssl-style checksums.
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 31c3d5c..c54ffb8 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -14634,11 +14634,15 @@ nohup invocation
 Exit status:

 @display
+125 if @command{nohup} itself fails, and @env{POSIXLY_CORRECT} is not set
 126 if @var{command} is found but cannot be invoked
-127 if @command{nohup} itself fails or if @var{command} cannot be found
+127 if @var{command} cannot be found
 the exit status of @var{command} otherwise
 @end display

+If @env{POSIXLY_CORRECT} is set, internal failures give status 127
+instead of 125.
+

 @node stdbuf invocation
 @section @command{stdbuf}: Run a command with modified I/O stream buffering
diff --git a/src/nohup.c b/src/nohup.c
index 99bb865..880cc74 100644
--- a/src/nohup.c
+++ b/src/nohup.c
@@ -40,7 +40,7 @@
 enum
   {
     /* `nohup' itself failed.  */
-    NOHUP_FAILURE = 127
+    POSIX_NOHUP_FAILURE = 127
   };

 void
@@ -85,6 +85,7 @@ main (int argc, char **argv)
   bool redirecting_stdout;
   bool stdout_is_closed;
   bool redirecting_stderr;
+  int exit_internal_failure;

   initialize_main (&argc, &argv);
   set_program_name (argv[0]);
@@ -92,18 +93,24 @@ main (int argc, char **argv)
   bindtextdomain (PACKAGE, LOCALEDIR);
   textdomain (PACKAGE);

-  initialize_exit_failure (NOHUP_FAILURE);
+  /* POSIX 2008 requires that internal failure give status 127; unlike
+     for env, exec, nice, time, and xargs where it requires internal
+     failure give something in the range 1-125.  For consistency with
+     other tools, fail with EXIT_CANCELED unless POSIXLY_CORRECT.  */
+  exit_internal_failure = (getenv ("POSIXLY_CORRECT")
+                           ? POSIX_NOHUP_FAILURE : EXIT_CANCELED);
+  initialize_exit_failure (exit_internal_failure);
   atexit (close_stdout);

   parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
                       usage, AUTHORS, (char const *) NULL);
   if (getopt_long (argc, argv, "+", NULL, NULL) != -1)
-    usage (NOHUP_FAILURE);
+    usage (exit_internal_failure);

   if (argc <= optind)
     {
       error (0, 0, _("missing operand"));
-      usage (NOHUP_FAILURE);
+      usage (exit_internal_failure);
     }

   ignoring_input = isatty (STDIN_FILENO);
@@ -154,7 +161,7 @@ main (int argc, char **argv)
               if (in_home)
                 error (0, saved_errno2, _("failed to open %s"),
                        quote (in_home));
-              exit (NOHUP_FAILURE);
+              exit (exit_internal_failure);
             }
           file = in_home;
         }
@@ -179,7 +186,7 @@ main (int argc, char **argv)

       if (0 <= saved_stderr_fd
           && set_cloexec_flag (saved_stderr_fd, true) != 0)
-        error (NOHUP_FAILURE, errno,
+        error (exit_internal_failure, errno,
                _("failed to set the copy of stderr to close on exec"));

       if (!redirecting_stdout)
@@ -189,7 +196,8 @@ main (int argc, char **argv)
                  : N_("redirecting stderr to stdout")));

       if (dup2 (out_fd, STDERR_FILENO) < 0)
-        error (NOHUP_FAILURE, errno, _("failed to redirect standard error"));
+        error (exit_internal_failure, errno,
+               _("failed to redirect standard error"));

       if (stdout_is_closed)
         close (out_fd);
-- 
1.6.5.rc1