diff options
author | Paul Smith <psmith@gnu.org> | 2013-09-14 20:40:30 -0400 |
---|---|---|
committer | Paul Smith <psmith@gnu.org> | 2013-09-14 20:40:30 -0400 |
commit | 29a94ceb76936b88e74052dcb81fe506145f6ff4 (patch) | |
tree | 9ecba816136119896da6df2a94c74c5116a5d688 | |
parent | ab78cbc71ce16dd39e4b6b9e42c02f75bf1d8a50 (diff) | |
download | gunmake-29a94ceb76936b88e74052dcb81fe506145f6ff4.tar.gz |
[SV 33134] Don't try to close stdout when it's already closed.
-rw-r--r-- | ChangeLog | 10 | ||||
-rw-r--r-- | main.c | 4 | ||||
-rw-r--r-- | makeint.h | 2 | ||||
-rw-r--r-- | misc.c | 47 | ||||
-rw-r--r-- | output.c | 56 |
5 files changed, 64 insertions, 55 deletions
@@ -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> @@ -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); @@ -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 */ @@ -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); - } -} @@ -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 |