From 29a94ceb76936b88e74052dcb81fe506145f6ff4 Mon Sep 17 00:00:00 2001
From: Paul Smith <psmith@gnu.org>
Date: Sat, 14 Sep 2013 20:40:30 -0400
Subject: [PATCH] [SV 33134] Don't try to close stdout when it's already
closed.
---
ChangeLog | 10 ++++++++++
main.c | 4 ----
makeint.h | 2 --
misc.c | 47 ----------------------------------------------
output.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
5 files changed, 64 insertions(+), 55 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index fdb921f6..68c771d2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+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.
@@ -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 4d6b5b6c..eacab286 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 276a13d9..dac58587 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 df8bd43a..d9150325 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 2e69c6d2..64634325 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