bug#9574: data-loss with --batch: ignored write failure

[Paul Eggert]

On 10/29/2012 02:27 AM, Chong Yidong wrote:

> Has this bug been fixed since the above was written 6 months ago?

Sorry, no.  Here's a patch, relative to trunk bzr 110763.
I won't install it right now, since we're so close
to creating a branch.


=== modified file 'ChangeLog'
--- ChangeLog   2012-10-26 18:35:36 +0000
+++ ChangeLog   2012-11-01 20:07:42 +0000
@@ -1,3 +1,11 @@
+2012-11-01  Paul Eggert  <eggert@cs.ucla.edu>
+
+       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-10-26  Glenn Morris  <rgm@gnu.org>
 
        * Makefile.in (EMACS_NAME): New variable.

=== modified file 'admin/ChangeLog'
--- admin/ChangeLog     2012-10-19 19:25:18 +0000
+++ admin/ChangeLog     2012-11-01 20:07:42 +0000
@@ -1,3 +1,8 @@
+2012-11-01  Paul Eggert  <eggert@cs.ucla.edu>
+
+       Fix data-loss with --batch (Bug#9574).
+       * merge-gnulib (GNULIB_MODULES): Add close-stream.
+
 2012-10-12  Kenichi Handa  <handa@gnu.org>
 
        * charsets/Makefile (JISC6226.map): Add missing mappings.

=== modified file 'admin/merge-gnulib'
--- admin/merge-gnulib  2012-10-19 19:25:18 +0000
+++ admin/merge-gnulib  2012-11-01 20:07:42 +0000
@@ -27,7 +27,7 @@
 
 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

=== added file 'lib/close-stream.c'
--- lib/close-stream.c  1970-01-01 00:00:00 +0000
+++ lib/close-stream.c  2012-11-01 20:07:42 +0000
@@ -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 <http://www.gnu.org/licenses/>.  */
+
+#include <config.h>
+
+#include "close-stream.h"
+
+#include <errno.h>
+#include <stdbool.h>
+
+#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;
+}

=== added file 'lib/close-stream.h'
--- lib/close-stream.h  1970-01-01 00:00:00 +0000
+++ lib/close-stream.h  2012-11-01 20:07:42 +0000
@@ -0,0 +1,2 @@
+#include <stdio.h>
+int close_stream (FILE *stream);

=== added file 'lib/fpending.c'
--- lib/fpending.c      1970-01-01 00:00:00 +0000
+++ lib/fpending.c      2012-11-01 20:07:42 +0000
@@ -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 <http://www.gnu.org/licenses/>.  */
+
+/* Written by Jim Meyering. */
+
+#include <config.h>
+
+#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;
+}

=== added file 'lib/fpending.h'
--- lib/fpending.h      1970-01-01 00:00:00 +0000
+++ lib/fpending.h      2012-11-01 20:07:42 +0000
@@ -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 <http://www.gnu.org/licenses/>.
+
+   Written by Jim Meyering.  */
+
+#include <stddef.h>
+#include <stdio.h>
+
+#if HAVE_DECL___FPENDING
+# if HAVE_STDIO_EXT_H
+#  include <stdio_ext.h>
+# endif
+#else
+size_t __fpending (FILE *);
+#endif

=== modified file 'lib/gnulib.mk'
--- lib/gnulib.mk       2012-10-19 19:25:18 +0000
+++ lib/gnulib.mk       2012-11-01 20:07:42 +0000
@@ -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 @@
 
 ## 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 @@
 
 ## 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
 
 

=== added file 'm4/close-stream.m4'
--- m4/close-stream.m4  1970-01-01 00:00:00 +0000
+++ m4/close-stream.m4  2012-11-01 20:07:42 +0000
@@ -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],
+[
+  :
+])

=== added file 'm4/fpending.m4'
--- m4/fpending.m4      1970-01-01 00:00:00 +0000
+++ m4/fpending.m4      2012-11-01 20:07:42 +0000
@@ -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 <stdio.h>
+#     if HAVE_STDIO_EXT_H
+#      include <stdio_ext.h>
+#     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 <stdio.h>]],
+          [[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'])
+])

=== modified file 'm4/gnulib-comp.m4'
--- m4/gnulib-comp.m4   2012-10-19 19:25:18 +0000
+++ m4/gnulib-comp.m4   2012-11-01 20:07:42 +0000
@@ -44,6 +44,7 @@
   # 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_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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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

=== modified file 'src/ChangeLog'
--- src/ChangeLog       2012-11-01 14:21:45 +0000
+++ src/ChangeLog       2012-11-01 20:07:42 +0000
@@ -1,3 +1,13 @@
+2012-11-01  Paul Eggert  <eggert@cs.ucla.edu>
+
+       Fix data-loss with --batch (Bug#9574).
+       * emacs.c: Include <close-stream.h>.
+       (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.
+
 2012-11-01  Eli Zaretskii  <eliz@gnu.org>
 
        * w32proc.c (getpgrp, setpgid): New functions.  (Bug#12776)

=== modified file 'src/emacs.c'
--- src/emacs.c 2012-10-31 17:27:29 +0000
+++ src/emacs.c 2012-11-01 20:07:42 +0000
@@ -27,6 +27,7 @@
 #include <sys/file.h>
 #include <unistd.h>
 
+#include <close-stream.h>
 #include <ignore-value.h>
 
 #include "lisp.h"
@@ -675,6 +676,22 @@
 
 #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 @@
   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 @@
     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);