nohup patches for POSIX conformance and manual clarification

[Paul Eggert]

Dan Jacobson <address@hidden> writes:

> "same as that of `nohup'."... Maybe you mean "same as that of `nohup'
> when run on a terminal, which is ..."?

I installed the following patch to try to clarify the manual.

Also, in reviewing the manual and the implementation, I found a couple
of deviations from POSIX in this area, and a file descriptor leak, and
a couple of very minor internal coding infelicities, so this patch
fixes them as well.  Thanks for reporting the problem.

2005-03-18  Paul Eggert  <address@hidden>

        * NEWS: nohup now ignores the umask when creating nohup.out.
        nohup now closes stderr if it is a terminal and stdout is closed.
        * src/nohup.c (main): Likewise.  Be a little more paranoid about
        return values; e.g., check for any negative return from open.
        Assume free (NULL) works.
        Close file descriptor leak when redirecting standard output to a file.
        * doc/coreutils.texi (nohup invocation): Clarify nohup.out creation.

Index: NEWS
===================================================================
RCS file: /fetish/cu/NEWS,v
retrieving revision 1.273
diff -p -u -r1.273 NEWS
--- NEWS        14 Mar 2005 18:36:31 -0000      1.273
+++ NEWS        19 Mar 2005 00:44:00 -0000
@@ -4,6 +4,12 @@ GNU coreutils NEWS                      
 
 ** Changes for better compliance with POSIX
 
+  nohup changes:
+
+    nohup now ignores the umask when creating nohup.out.
+
+    nohup now closes stderr if it is a terminal and stdout is closed.
+
   pathchk changes:
 
     It now rejects the empty name in the normal case.  That is,
Index: doc/coreutils.texi
===================================================================
RCS file: /fetish/cu/doc/coreutils.texi,v
retrieving revision 1.244
diff -p -u -r1.244 coreutils.texi
--- doc/coreutils.texi  11 Mar 2005 23:13:19 -0000      1.244
+++ doc/coreutils.texi  19 Mar 2005 00:44:01 -0000
@@ -12473,20 +12473,16 @@ nohup @var{command} address@hidden@dots{}
 @end example
 
 @flindex nohup.out
-If standard output is a terminal, it is redirected so that it is appended
+If standard output is a terminal, the command's standard output is appended
 to the file @file{nohup.out}; if that cannot be written to, it is appended
-to the file @file{$HOME/nohup.out}.  If that cannot be written to, the
+to the file @file{$HOME/nohup.out}; and if that cannot be written to, the
 command is not run.
-If standard output is not a terminal, then the standard output of
address@hidden will be the same as that of @command{nohup}.
-
-If @command{nohup} creates either @file{nohup.out} or
address@hidden/nohup.out}, it creates it with no ``group'' or ``other''
-access permissions.  It does not change the permissions if the output
-file already existed.
+Any @file{nohup.out} or @file{$HOME/nohup.out} file created by
address@hidden is made readable and writable only to the user,
+regardless of the current umask settings.
 
 If standard error is a terminal, it is redirected to the same file
-descriptor as the standard output.
+descriptor as the (possibly-redirected) standard output.
 
 @command{nohup} does not automatically put the command it runs in the
 background; you must do that explicitly, by ending the command line
Index: src/nohup.c
===================================================================
RCS file: /fetish/cu/src/nohup.c,v
retrieving revision 1.19
diff -p -u -r1.19 nohup.c
--- src/nohup.c 17 Nov 2004 00:56:25 -0000      1.19
+++ src/nohup.c 19 Mar 2005 00:44:01 -0000
@@ -1,5 +1,5 @@
 /* nohup -- run a command immume to hangups, with output to a non-tty
-   Copyright (C) 2003, 2004 Free Software Foundation, Inc.
+   Copyright (C) 2003, 2004, 2005 Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -72,9 +72,7 @@ Run COMMAND, ignoring hangup signals.\n\
 int
 main (int argc, char **argv)
 {
-  int fd;
-  int saved_stderr_fd = -1;
-  bool stderr_isatty;
+  int saved_stderr_fd = STDERR_FILENO;
 
   initialize_main (&argc, &argv);
   program_name = argv[0];
@@ -104,9 +102,10 @@ main (int argc, char **argv)
       char const *file = "nohup.out";
       int flags = O_CREAT | O_WRONLY | O_APPEND;
       mode_t mode = S_IRUSR | S_IWUSR;
+      mode_t umask_value = umask (~mode);
+      int fd = open (file, flags, mode);
 
-      fd = open (file, flags, mode);
-      if (fd == -1)
+      if (fd < 0)
        {
          int saved_errno = errno;
          char const *home = getenv ("HOME");
@@ -115,7 +114,7 @@ main (int argc, char **argv)
              in_home = path_concat (home, file, NULL);
              fd = open (in_home, flags, mode);
            }
-         if (fd == -1)
+         if (fd < 0)
            {
              int saved_errno2 = errno;
              error (0, saved_errno, _("failed to open %s"), quote (file));
@@ -127,21 +126,19 @@ main (int argc, char **argv)
          file = in_home;
        }
 
+      umask (umask_value);
+
       /* Redirect standard output to the file.  */
-      if (dup2 (fd, STDOUT_FILENO) == -1)
+      if (fd != STDOUT_FILENO
+         && (dup2 (fd, STDOUT_FILENO) < 0 || close (fd) != 0))
        error (NOHUP_FAILURE, errno, _("failed to redirect standard output"));
 
       error (0, 0, _("appending output to %s"), quote (file));
-      if (in_home)
-       free (in_home);
-    }
-  else
-    {
-      fd = STDOUT_FILENO;
+      free (in_home);
     }
 
   /* If stderr is on a tty, redirect it to stdout.  */
-  if ((stderr_isatty = isatty (STDERR_FILENO)))
+  if (isatty (STDERR_FILENO))
     {
       /* Save a copy of stderr before redirecting, so we can use the original
         if execve fails.  It's no big deal if this dup fails.  It might
@@ -149,13 +146,18 @@ main (int argc, char **argv)
         the post-failed-execve diagnostic.  */
       saved_stderr_fd = dup (STDERR_FILENO);
 
-      if (saved_stderr_fd != -1
+      if (0 <= saved_stderr_fd
          && set_cloexec_flag (saved_stderr_fd, true) != 0)
        error (NOHUP_FAILURE, errno,
               _("failed to set the copy of stderr to close on exec"));
 
-      if (dup2 (fd, STDERR_FILENO) == -1)
-       error (NOHUP_FAILURE, errno, _("failed to redirect standard error"));
+      if (dup2 (STDOUT_FILENO, STDERR_FILENO) < 0)
+       {
+         if (errno != EBADF)
+           error (NOHUP_FAILURE, errno,
+                  _("failed to redirect standard error"));
+         close (STDERR_FILENO);
+       }
     }
 
   signal (SIGHUP, SIG_IGN);
@@ -171,13 +173,11 @@ main (int argc, char **argv)
 
     /* The execve failed.  Output a diagnostic to stderr only if:
        - stderr was initially redirected to a non-tty, or
-       - stderr was initially directed to a tty, and we've
-        just dup2'd it to point back to that same tty.
-       In other words, output the diagnostic if possible, but not if
-       it'd go to nohup.out.  */
-    if ( ! stderr_isatty
-       || (saved_stderr_fd != -1
-           && dup2 (saved_stderr_fd, STDERR_FILENO) != -1))
+       - stderr was initially directed to a tty, and we
+        can dup2 it to point back to that same tty.
+       In other words, output the diagnostic if possible, but only if
+       it will go to the original stderr.  */
+    if (dup2 (saved_stderr_fd, STDERR_FILENO) == STDERR_FILENO)
       error (0, saved_errno, _("cannot run command %s"), quote (*cmd));
 
     exit (exit_status);