Package: emacs;
Reported by: Jim Meyering <jim <at> meyering.net>
Date: Thu, 22 Sep 2011 14:40:02 UTC
Severity: minor
Tags: patch
Done: Paul Eggert <eggert <at> cs.ucla.edu>
Bug is archived. No further changes may be made.
Message #37 received at 9574 <at> debbugs.gnu.org (full text, mbox):
From: Paul Eggert <eggert <at> cs.ucla.edu> To: Chong Yidong <cyd <at> gnu.org> Cc: Glenn Morris <rgm <at> gnu.org>, 9574 <at> debbugs.gnu.org, Jim Meyering <jim <at> meyering.net> Subject: Re: bug#9574: data-loss with --batch: ignored write failure Date: Thu, 01 Nov 2012 13:09:03 -0700
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 <at> 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 <at> 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 <at> cs.ucla.edu> + + Fix data-loss with --batch (Bug#9574). + * merge-gnulib (GNULIB_MODULES): Add close-stream. + 2012-10-12 Kenichi Handa <handa <at> 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 <at> 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 <at> 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);
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.