[SV 33134] Don't try to close stdout when it's already closed.

5 files changed, 64 insertions, 55 deletions

diff --git a/ChangeLog b/ChangeLog
index fdb921f..68c771d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2013-09-14  Paul Smith  <psmith@gnu.org>
 
+	Fix Savannah bug #33134.  Suggested by David Boyce <dsb@boyski.com>.
+
+	* misc.c (close_stdout): Move to output.c.
+	* main.c (main): Move atexit call to output_init().
+	* makeint.h: Remove close_stdout() declaration.
+	* output.c (output_init): Add close_stdout at exit only if it's	open.
+
+2013-09-14  Paul Smith  <psmith@gnu.org>
+
 	* misc.c (set_append_mode, open_tmpfd, open_tmpfile): Move to output.c.
 	* misc.h: Ditto.
 	* output.h: Ditto.
@@ -12,6 +21,7 @@
 	(output_dump): In recurse mode print enter/leave once for the
 	whole makefile.
 	(output_init): Initialize this processes stdio as well as child's.
+
 	* vmsjobs.c: Reformat to be closer to convention.
 
 2013-09-12  Paul Smith  <psmith@gnu.org>
diff --git a/main.c b/main.c
index 4d6b5b6..eacab28 100644
--- a/main.c
+++ b/main.c
@@ -1063,10 +1063,6 @@ main (int argc, char **argv, char **envp)
   }
 #endif
 
-#ifdef HAVE_ATEXIT
-  atexit (close_stdout);
-#endif
-
   /* Needed for OS/2 */
   initialize_main (&argc, &argv);
 
diff --git a/makeint.h b/makeint.h
index 276a13d..dac5858 100644
--- a/makeint.h
+++ b/makeint.h
@@ -493,8 +493,6 @@ void user_access (void);
 void make_access (void);
 void child_access (void);
 
-void close_stdout (void);
-
 char *strip_whitespace (const char **begpp, const char **endpp);
 
 /* String caching  */
diff --git a/misc.c b/misc.c
index df8bd43..d915032 100644
--- a/misc.c
+++ b/misc.c
@@ -723,50 +723,3 @@ get_path_max (void)
   return value;
 }
 #endif
-
-
-/* This code is stolen from gnulib.
-   If/when we abandon the requirement to work with K&R compilers, we can
-   remove this (and perhaps other parts of GNU make!) and migrate to using
-   gnulib directly.
-
-   This is called only through atexit(), which means die() has already been
-   invoked.  So, call exit() here directly.  Apparently that works...?
-*/
-
-/* Close standard output, exiting with status 'exit_failure' on failure.
-   If a program writes *anything* to stdout, that program should close
-   stdout and make sure that it succeeds before exiting.  Otherwise,
-   suppose that you go to the extreme of checking the return status
-   of every function that does an explicit write to stdout.  The last
-   printf can succeed in writing to the internal stream buffer, and yet
-   the fclose(stdout) could still fail (due e.g., to a disk full error)
-   when it tries to write out that buffered data.  Thus, you would be
-   left with an incomplete output file and the offending program would
-   exit successfully.  Even calling fflush is not always sufficient,
-   since some file systems (NFS and CODA) buffer written/flushed data
-   until an actual close call.
-
-   Besides, it's wasteful to check the return value from every call
-   that writes to stdout -- just let the internal stream state record
-   the failure.  That's what the ferror test is checking below.
-
-   It's important to detect such failures and exit nonzero because many
-   tools (most notably 'make' and other build-management systems) depend
-   on being able to detect failure in other tools via their exit status.  */
-
-void
-close_stdout (void)
-{
-  int prev_fail = ferror (stdout);
-  int fclose_fail = fclose (stdout);
-
-  if (prev_fail || fclose_fail)
-    {
-      if (fclose_fail)
-        error (NILF, _("write error: %s"), strerror (errno));
-      else
-        error (NILF, _("write error"));
-      exit (EXIT_FAILURE);
-    }
-}
diff --git a/output.c b/output.c
index 2e69c6d..6463432 100644
--- a/output.c
+++ b/output.c
@@ -178,9 +178,9 @@ set_append_mode (int fd)
 /* Semaphore for use in -j mode with output_sync. */
 static sync_handle_t sync_handle = -1;
 
-#define STREAM_OK(_s)       ((fcntl (fileno (_s), F_GETFD) != -1) || (errno != EBADF))
+#define STREAM_OK(_s)    ((fcntl (fileno (_s), F_GETFD) != -1) || (errno != EBADF))
 
-#define FD_NOT_EMPTY(_f)    ((_f) != OUTPUT_NONE && lseek ((_f), 0, SEEK_END) > 0)
+#define FD_NOT_EMPTY(_f) ((_f) != OUTPUT_NONE && lseek ((_f), 0, SEEK_END) > 0)
 
 /* Set up the sync handle.  Disables output_sync on error.  */
 static int
@@ -460,6 +460,53 @@ output_tmpfile (char **name, const char *template)
 }
 
 
+/* This code is stolen from gnulib.
+   If/when we abandon the requirement to work with K&R compilers, we can
+   remove this (and perhaps other parts of GNU make!) and migrate to using
+   gnulib directly.
+
+   This is called only through atexit(), which means die() has already been
+   invoked.  So, call exit() here directly.  Apparently that works...?
+*/
+
+/* Close standard output, exiting with status 'exit_failure' on failure.
+   If a program writes *anything* to stdout, that program should close
+   stdout and make sure that it succeeds before exiting.  Otherwise,
+   suppose that you go to the extreme of checking the return status
+   of every function that does an explicit write to stdout.  The last
+   printf can succeed in writing to the internal stream buffer, and yet
+   the fclose(stdout) could still fail (due e.g., to a disk full error)
+   when it tries to write out that buffered data.  Thus, you would be
+   left with an incomplete output file and the offending program would
+   exit successfully.  Even calling fflush is not always sufficient,
+   since some file systems (NFS and CODA) buffer written/flushed data
+   until an actual close call.
+
+   Besides, it's wasteful to check the return value from every call
+   that writes to stdout -- just let the internal stream state record
+   the failure.  That's what the ferror test is checking below.
+
+   It's important to detect such failures and exit nonzero because many
+   tools (most notably 'make' and other build-management systems) depend
+   on being able to detect failure in other tools via their exit status.  */
+
+static void
+close_stdout (void)
+{
+  int prev_fail = ferror (stdout);
+  int fclose_fail = fclose (stdout);
+
+  if (prev_fail || fclose_fail)
+    {
+      if (fclose_fail)
+        error (NILF, _("write error: %s"), strerror (errno));
+      else
+        error (NILF, _("write error"));
+      exit (EXIT_FAILURE);
+    }
+}
+
+
 void
 output_init (struct output *out)
 {
@@ -487,6 +534,11 @@ output_init (struct output *out)
      lose output due to overlapping writes.  */
   set_append_mode (fileno (stdout));
   set_append_mode (fileno (stderr));
+
+#ifdef HAVE_ATEXIT
+  if (STREAM_OK (stdout))
+    atexit (close_stdout);
+#endif
 }
 
 void