From 7ccfb720b477df05042729e0e300bae5922f5120 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sat, 3 Nov 2012 11:54:17 -0700 Subject: [PATCH] Fix data-loss with --batch. * admin/merge-gnulib (GNULIB_MODULES): Add close-stream. * lib/close-stream.c, lib/close-stream.h, lib/fpending.c * lib/fpending.h, m4/close-stream.m4, m4/fpending.m4: New files, from gnulib. * lib/gnulib.mk, m4/gnulib-comp.m4: Regenerate. * src/emacs.c: Include . (close_output_streams): New function. (main): Pass it to atexit, so that Emacs closes stdout and stderr and handles errors appropriately. (Fkill_emacs): Don't worry about flushing, as close_output_stream does that now. Fixes: debbugs:9574 --- ChangeLog | 8 +++++ admin/ChangeLog | 5 +++ admin/merge-gnulib | 2 +- lib/close-stream.c | 78 ++++++++++++++++++++++++++++++++++++++++ lib/close-stream.h | 2 ++ lib/fpending.c | 30 ++++++++++++++++ lib/fpending.h | 30 ++++++++++++++++ lib/gnulib.mk | 19 +++++++++- m4/close-stream.m4 | 11 ++++++ m4/fpending.m4 | 90 ++++++++++++++++++++++++++++++++++++++++++++++ m4/gnulib-comp.m4 | 15 ++++++++ src/ChangeLog | 8 +++++ src/emacs.c | 21 +++++++++-- 13 files changed, 315 insertions(+), 4 deletions(-) create mode 100644 lib/close-stream.c create mode 100644 lib/close-stream.h create mode 100644 lib/fpending.c create mode 100644 lib/fpending.h create mode 100644 m4/close-stream.m4 create mode 100644 m4/fpending.m4 diff --git a/ChangeLog b/ChangeLog index 45fe253e04..e67369492d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2012-11-03 Paul Eggert + + Fix data-loss with --batch (Bug#9574). + * lib/close-stream.c, lib/close-stream.h, lib/fpending.c + * lib/fpending.h, m4/close-stream.m4, m4/fpending.m4: + New files, from gnulib. + * lib/gnulib.mk, m4/gnulib-comp.m4: Regenerate. + 2012-11-02 Glenn Morris * Makefile.in (EMACS_ICON): New variable. diff --git a/admin/ChangeLog b/admin/ChangeLog index e21293d618..da318dd662 100644 --- a/admin/ChangeLog +++ b/admin/ChangeLog @@ -1,3 +1,8 @@ +2012-11-01 Paul Eggert + + Fix data-loss with --batch (Bug#9574). + * merge-gnulib (GNULIB_MODULES): Add close-stream. + 2012-10-12 Kenichi Handa * charsets/Makefile (JISC6226.map): Add missing mappings. diff --git a/admin/merge-gnulib b/admin/merge-gnulib index 7fc0b5f484..901daf4e44 100755 --- a/admin/merge-gnulib +++ b/admin/merge-gnulib @@ -27,7 +27,7 @@ GNULIB_URL=git://git.savannah.gnu.org/gnulib.git GNULIB_MODULES=' alloca-opt c-ctype c-strcase - careadlinkat crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 + careadlinkat close-stream crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 dtoastr dtotimespec dup2 environ execinfo filemode getloadavg getopt-gnu gettime gettimeofday ignore-value intprops largefile lstat diff --git a/lib/close-stream.c b/lib/close-stream.c new file mode 100644 index 0000000000..04fa5ece09 --- /dev/null +++ b/lib/close-stream.c @@ -0,0 +1,78 @@ +/* Close a stream, with nicer error checking than fclose's. + + Copyright (C) 1998-2002, 2004, 2006-2012 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 + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#include + +#include "close-stream.h" + +#include +#include + +#include "fpending.h" + +#if USE_UNLOCKED_IO +# include "unlocked-io.h" +#endif + +/* Close STREAM. Return 0 if successful, EOF (setting errno) + otherwise. A failure might set errno to 0 if the error number + cannot be determined. + + A failure with errno set to EPIPE may or may not indicate an error + situation worth signaling to the user. See the documentation of the + close_stdout_set_ignore_EPIPE function for details. + + If a program writes *anything* to STREAM, that program should close + STREAM 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 STREAM. The last + printf can succeed in writing to the internal stream buffer, and yet + the fclose(STREAM) 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 STREAM -- just let the internal stream state record + the failure. That's what the ferror test is checking below. */ + +int +close_stream (FILE *stream) +{ + const bool some_pending = (__fpending (stream) != 0); + const bool prev_fail = (ferror (stream) != 0); + const bool fclose_fail = (fclose (stream) != 0); + + /* Return an error indication if there was a previous failure or if + fclose failed, with one exception: ignore an fclose failure if + there was no previous error, no data remains to be flushed, and + fclose failed with EBADF. That can happen when a program like cp + is invoked like this 'cp a b >&-' (i.e., with standard output + closed) and doesn't generate any output (hence no previous error + and nothing to be flushed). */ + + if (prev_fail || (fclose_fail && (some_pending || errno != EBADF))) + { + if (! fclose_fail) + errno = 0; + return EOF; + } + + return 0; +} diff --git a/lib/close-stream.h b/lib/close-stream.h new file mode 100644 index 0000000000..be3d4196b0 --- /dev/null +++ b/lib/close-stream.h @@ -0,0 +1,2 @@ +#include +int close_stream (FILE *stream); diff --git a/lib/fpending.c b/lib/fpending.c new file mode 100644 index 0000000000..2591d53437 --- /dev/null +++ b/lib/fpending.c @@ -0,0 +1,30 @@ +/* fpending.c -- return the number of pending output bytes on a stream + Copyright (C) 2000, 2004, 2006-2007, 2009-2012 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 + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +/* Written by Jim Meyering. */ + +#include + +#include "fpending.h" + +/* Return the number of pending (aka buffered, unflushed) + bytes on the stream, FP, that is open for writing. */ +size_t +__fpending (FILE *fp) +{ + return PENDING_OUTPUT_N_BYTES; +} diff --git a/lib/fpending.h b/lib/fpending.h new file mode 100644 index 0000000000..0365287ba7 --- /dev/null +++ b/lib/fpending.h @@ -0,0 +1,30 @@ +/* Declare __fpending. + + Copyright (C) 2000, 2003, 2005-2006, 2009-2012 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 + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . + + Written by Jim Meyering. */ + +#include +#include + +#if HAVE_DECL___FPENDING +# if HAVE_STDIO_EXT_H +# include +# endif +#else +size_t __fpending (FILE *); +#endif diff --git a/lib/gnulib.mk b/lib/gnulib.mk index 23749331a8..324e5cb78f 100644 --- a/lib/gnulib.mk +++ b/lib/gnulib.mk @@ -21,7 +21,7 @@ # the same distribution terms as the rest of that program. # # Generated by gnulib-tool. -# Reproduce by: gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --avoid=errno --avoid=fcntl --avoid=fcntl-h --avoid=fstat --avoid=msvc-inval --avoid=msvc-nothrow --avoid=raise --avoid=select --avoid=sigprocmask --avoid=sys_types --avoid=threadlib --makefile-name=gnulib.mk --conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca-opt c-ctype c-strcase careadlinkat crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 dtoastr dtotimespec dup2 environ execinfo filemode getloadavg getopt-gnu gettime gettimeofday ignore-value intprops largefile lstat manywarnings mktime pselect pthread_sigmask readlink socklen stat-time stdalign stdarg stdbool stdio strftime strtoimax strtoumax symlink sys_stat sys_time time timer-time timespec-add timespec-sub utimens warnings +# Reproduce by: gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --avoid=errno --avoid=fcntl --avoid=fcntl-h --avoid=fstat --avoid=msvc-inval --avoid=msvc-nothrow --avoid=raise --avoid=select --avoid=sigprocmask --avoid=sys_types --avoid=threadlib --makefile-name=gnulib.mk --conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca-opt c-ctype c-strcase careadlinkat close-stream crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 dtoastr dtotimespec dup2 environ execinfo filemode getloadavg getopt-gnu gettime gettimeofday ignore-value intprops largefile lstat manywarnings mktime pselect pthread_sigmask readlink socklen stat-time stdalign stdarg stdbool stdio strftime strtoimax strtoumax symlink sys_stat sys_time time timer-time timespec-add timespec-sub utimens warnings MOSTLYCLEANFILES += core *.stackdump @@ -84,6 +84,14 @@ EXTRA_DIST += careadlinkat.h ## end gnulib module careadlinkat +## begin gnulib module close-stream + +libgnu_a_SOURCES += close-stream.c + +EXTRA_DIST += close-stream.h + +## end gnulib module close-stream + ## begin gnulib module crypto/md5 libgnu_a_SOURCES += md5.c @@ -183,6 +191,15 @@ EXTRA_DIST += filemode.h ## end gnulib module filemode +## begin gnulib module fpending + + +EXTRA_DIST += fpending.c fpending.h + +EXTRA_libgnu_a_SOURCES += fpending.c + +## end gnulib module fpending + ## begin gnulib module getloadavg diff --git a/m4/close-stream.m4 b/m4/close-stream.m4 new file mode 100644 index 0000000000..be0c8a2297 --- /dev/null +++ b/m4/close-stream.m4 @@ -0,0 +1,11 @@ +#serial 4 +dnl Copyright (C) 2006-2007, 2009-2012 Free Software Foundation, Inc. +dnl This file is free software; the Free Software Foundation +dnl gives unlimited permission to copy and/or distribute it, +dnl with or without modifications, as long as this notice is preserved. + +dnl Prerequisites of lib/close-stream.c. +AC_DEFUN([gl_CLOSE_STREAM], +[ + : +]) diff --git a/m4/fpending.m4 b/m4/fpending.m4 new file mode 100644 index 0000000000..33a5c94c3a --- /dev/null +++ b/m4/fpending.m4 @@ -0,0 +1,90 @@ +# serial 19 + +# Copyright (C) 2000-2001, 2004-2012 Free Software Foundation, Inc. +# This file is free software; the Free Software Foundation +# gives unlimited permission to copy and/or distribute it, +# with or without modifications, as long as this notice is preserved. + +dnl From Jim Meyering +dnl Using code from emacs, based on suggestions from Paul Eggert +dnl and Ulrich Drepper. + +dnl Find out how to determine the number of pending output bytes on a stream. +dnl glibc (2.1.93 and newer) and Solaris provide __fpending. On other systems, +dnl we have to grub around in the FILE struct. + +AC_DEFUN([gl_FUNC_FPENDING], +[ + AC_CHECK_HEADERS_ONCE([stdio_ext.h]) + AC_CHECK_FUNCS_ONCE([__fpending]) + fp_headers=' +# include +# if HAVE_STDIO_EXT_H +# include +# endif +' + AC_CHECK_DECLS([__fpending], , , $fp_headers) +]) + +AC_DEFUN([gl_PREREQ_FPENDING], +[ + AC_CACHE_CHECK( + [how to determine the number of pending output bytes on a stream], + ac_cv_sys_pending_output_n_bytes, + [ + for ac_expr in \ + \ + '# glibc2' \ + 'fp->_IO_write_ptr - fp->_IO_write_base' \ + \ + '# traditional Unix' \ + 'fp->_ptr - fp->_base' \ + \ + '# BSD' \ + 'fp->_p - fp->_bf._base' \ + \ + '# SCO, Unixware' \ + '(fp->__ptr ? fp->__ptr - fp->__base : 0)' \ + \ + '# QNX' \ + '(fp->_Mode & 0x2000 /*_MWRITE*/ ? fp->_Next - fp->_Buf : 0)' \ + \ + '# old glibc?' \ + 'fp->__bufp - fp->__buffer' \ + \ + '# old glibc iostream?' \ + 'fp->_pptr - fp->_pbase' \ + \ + '# emx+gcc' \ + 'fp->_ptr - fp->_buffer' \ + \ + '# Minix' \ + 'fp->_ptr - fp->_buf' \ + \ + '# Plan9' \ + 'fp->wp - fp->buf' \ + \ + '# VMS' \ + '(*fp)->_ptr - (*fp)->_base' \ + \ + '# e.g., DGUX R4.11; the info is not available' \ + 1 \ + ; do + + # Skip each embedded comment. + case "$ac_expr" in '#'*) continue;; esac + + AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include ]], + [[FILE *fp = stdin; (void) ($ac_expr);]])], + [fp_done=yes] + ) + test "$fp_done" = yes && break + done + + ac_cv_sys_pending_output_n_bytes=$ac_expr + ] + ) + AC_DEFINE_UNQUOTED([PENDING_OUTPUT_N_BYTES], + $ac_cv_sys_pending_output_n_bytes, + [the number of pending output bytes on stream 'fp']) +]) diff --git a/m4/gnulib-comp.m4 b/m4/gnulib-comp.m4 index de2355d87c..5cd278454e 100644 --- a/m4/gnulib-comp.m4 +++ b/m4/gnulib-comp.m4 @@ -44,6 +44,7 @@ AC_DEFUN([gl_EARLY], # Code from module c-strcase: # Code from module careadlinkat: # Code from module clock-time: + # Code from module close-stream: # Code from module crypto/md5: # Code from module crypto/sha1: # Code from module crypto/sha256: @@ -58,6 +59,7 @@ AC_DEFUN([gl_EARLY], AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS]) # Code from module extern-inline: # Code from module filemode: + # Code from module fpending: # Code from module getloadavg: # Code from module getopt-gnu: # Code from module getopt-posix: @@ -141,6 +143,8 @@ AC_DEFUN([gl_INIT], gl_FUNC_ALLOCA AC_CHECK_FUNCS_ONCE([readlinkat]) gl_CLOCK_TIME + gl_CLOSE_STREAM + gl_MODULE_INDICATOR([close-stream]) gl_MD5 gl_SHA1 gl_SHA256 @@ -157,6 +161,11 @@ AC_DEFUN([gl_INIT], gl_EXECINFO_H AC_REQUIRE([gl_EXTERN_INLINE]) gl_FILEMODE + gl_FUNC_FPENDING + if test $ac_cv_func___fpending = no; then + AC_LIBOBJ([fpending]) + gl_PREREQ_FPENDING + fi gl_GETLOADAVG if test $HAVE_GETLOADAVG = 0; then AC_LIBOBJ([getloadavg]) @@ -534,6 +543,8 @@ AC_DEFUN([gl_FILE_LIST], [ lib/c-strncasecmp.c lib/careadlinkat.c lib/careadlinkat.h + lib/close-stream.c + lib/close-stream.h lib/dosname.h lib/dtoastr.c lib/dtotimespec.c @@ -542,6 +553,8 @@ AC_DEFUN([gl_FILE_LIST], [ lib/execinfo.in.h lib/filemode.c lib/filemode.h + lib/fpending.c + lib/fpending.h lib/ftoastr.c lib/ftoastr.h lib/getloadavg.c @@ -609,12 +622,14 @@ AC_DEFUN([gl_FILE_LIST], [ m4/alloca.m4 m4/c-strtod.m4 m4/clock_time.m4 + m4/close-stream.m4 m4/dup2.m4 m4/environ.m4 m4/execinfo.m4 m4/extensions.m4 m4/extern-inline.m4 m4/filemode.m4 + m4/fpending.m4 m4/getloadavg.m4 m4/getopt.m4 m4/gettime.m4 diff --git a/src/ChangeLog b/src/ChangeLog index 2f040accd9..be10016fc3 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,5 +1,13 @@ 2012-11-03 Paul Eggert + Fix data-loss with --batch (Bug#9574). + * emacs.c: Include . + (close_output_streams): New function. + (main): Pass it to atexit, so that Emacs closes stdout and stderr + and handles errors appropriately. + (Fkill_emacs): Don't worry about flushing, as close_output_stream + does that now. + Fix a race condition that causes Emacs to mess up glib (Bug#8855). The symptom is a diagnostic "GLib-WARNING **: In call to g_spawn_sync(), exit status of a child process was requested but diff --git a/src/emacs.c b/src/emacs.c index 98e3f11f0c..6588bcd78b 100644 --- a/src/emacs.c +++ b/src/emacs.c @@ -27,6 +27,7 @@ along with GNU Emacs. If not, see . */ #include #include +#include #include #include "lisp.h" @@ -675,6 +676,22 @@ void (*__malloc_initialize_hook) (void) EXTERNALLY_VISIBLE = malloc_initialize_h #endif /* DOUG_LEA_MALLOC */ +/* Close standard output and standard error, reporting any write + errors as best we can. This is intended for use with atexit. */ +static void +close_output_streams (void) +{ + if (close_stream (stdout) != 0) + { + fprintf (stderr, "Write error to standard output: %s\n", + emacs_strerror (errno)); + fflush (stderr); + _exit (EXIT_FAILURE); + } + + if (close_stream (stderr) != 0) + _exit (EXIT_FAILURE); +} /* ARGSUSED */ int @@ -890,6 +907,8 @@ main (int argc, char **argv) if (do_initial_setlocale) setlocale (LC_ALL, ""); + atexit (close_output_streams); + inhibit_window_system = 0; /* Handle the -t switch, which specifies filename to use as terminal. */ @@ -1867,8 +1886,6 @@ all of which are called before Emacs is actually killed. */) exit_code = (XINT (arg) < 0 ? XINT (arg) | INT_MIN : XINT (arg) & INT_MAX); - else if (noninteractive && (fflush (stdout) || ferror (stdout))) - exit_code = EXIT_FAILURE; else exit_code = EXIT_SUCCESS; exit (exit_code); -- 2.20.1