Re: stdin seekable failure

[Eric Blake]

Eric Blake <ebb9 <at> byu.net> writes:

> > 2007-04-12  Eric Blake  <ebb9 <at> byu.net>
> > 
> >     Work around glibc's failure to reset seekable stdin on exit.
> >     * modules/closein: New module.
> 
> And it still failed on Linux, since glibc won't even flush input streams
> on fclose.  But rather than write rpl_fclose, I think the simpler fix is
> just explicitly flushing as part of close_stdin, seeing as how so few
> programs actually need to flush stdin on exit.  At which point, the m4
> test that triggered this jaunt passes on Linux again (or skips, if you are
> still using too-old sed).
> 
> 2007-04-12  Eric Blake  <ebb9 <at> byu.net>
> 
>       Work around glibc's failure to flush stdin on fclose.
>       * lib/closein.c (close_stdin): Flush stdin before closing.

That patch fails on mingw, because the fflush module fails with EBADF (as 
allowed) when stdin is not seekable.  So to keep it from regressing, I added a 
test.  In unistd_.h, I blindly included <stdlib.h>, since unlike SEEK_CUR, 
there is no easy way (short of another configure test) to detect the mingw bug 
of declaring _exit in the wrong header; let me know if I should instead protect 
that by a configure test, to avoid namespace pollution on platforms with 
compliant headers.

However, the fflush module is still broken for mingw when reading files in text 
mode that have plain LF line endings.  It appears that mingw's ftell tries to 
compensate for \r\n line endings in text mode, but does the compensation even 
when there is just a plain \n line ending, which means that ftell can report 
the wrong offset (since the \r got stripped before the read buffer was 
populated, there is no way to tell if the remaining \n in the read buffer were 
coupled with a stripped \r or not).  I'm still trying to think of ways to make 
fflush reliable in spite of that limitation.

Also, in testing this, I discoverd that cygwin stdin has a bug - when a process 
starts, stdin is always in 32-bit mode (even though cygwin has been compiling 
in 64-bit mode since cygwin 1.5.0).  But since cygwin ftello really invokes 
ftello64 under the hood, and ftello64 fails unless the file is open in 64-bit 
mode, it is possible for ftell(stdin) to return non-negative while ftello
(stdin) fails with EOF.  freopen(NULL, "r", stdin) fixes the problem for stdin, 
and ftello does not have the problem on FILEs obtained by fopen.  I don't know 
if the problem can be worked around in the ftello and fseeko modules (and it 
also implies needing fgetpos and fsetpos modules), or if we just wait until the 
fix in cygwin 1.7.0.

2007-04-27  Eric Blake  <address@hidden>

        Fix closein for mingw.
        * modules/closein-tests: Add tests for closein.
        * tests/test-closein.c: New file.
        * tests/test-closein.sh: Likewise.
        * lib/unistd_.h [!SEEK_CUR]: Mingw also needs stdlib.h for _exit.
        * lib/closein.c (close_stdin): Don't fflush non-seekable streams.

Index: lib/closein.c
===================================================================
RCS file: /sources/gnulib/gnulib/lib/closein.c,v
retrieving revision 1.2
diff -u -r1.2 closein.c
--- lib/closein.c       12 Apr 2007 20:51:24 -0000      1.2
+++ lib/closein.c       27 Apr 2007 16:43:08 -0000
@@ -19,7 +19,6 @@
 #include <config.h>
 
 #include "closein.h"
-#include "closeout.h"
 
 #include <errno.h>
 #include <stdbool.h>
@@ -30,6 +29,7 @@
 #define _(msgid) gettext (msgid)
 
 #include "close-stream.h"
+#include "closeout.h"
 #include "error.h"
 #include "exitfail.h"
 #include "quotearg.h"
@@ -79,18 +79,23 @@
 close_stdin (void)
 {
   bool fail = false;
-  if (fflush (stdin) != 0 || close_stream (stdin) != 0)
+
+  /* Only attempt flush if stdin is seekable, as fflush is entitled to
+     fail on non-seekable streams.  */
+  if (fseeko (stdin, 0, SEEK_CUR) == 0 && fflush (stdin) != 0)
+    fail = true;
+  if (close_stream (stdin) != 0)
+    fail = true;
+  if (fail)
     {
+      /* Report failure, but defer exit until after closing stdout,
+        since the failure report should still be flushed.  */
       char const *close_error = _("error closing file");
       if (file_name)
        error (0, errno, "%s: %s", quotearg_colon (file_name),
               close_error);
       else
        error (0, errno, "%s", close_error);
-
-      /* Defer failure until after closing stdout, since the output
-        can still usefully be flushed.  */
-      fail = true;
     }
 
   close_stdout ();
Index: lib/unistd_.h
===================================================================
RCS file: /sources/gnulib/gnulib/lib/unistd_.h,v
retrieving revision 1.5
diff -u -r1.5 unistd_.h
--- lib/unistd_.h       25 Apr 2007 06:46:57 -0000      1.5
+++ lib/unistd_.h       27 Apr 2007 16:43:08 -0000
@@ -27,6 +27,8 @@
 # include <stdio.h>
 #endif
 
+/* mingw fails to declare _exit in <unistd.h>.  */
+#include <stdlib.h>
 
 /* The definition of GL_LINK_WARNING is copied here.  */
 
Index: modules/closein-tests
===================================================================
RCS file: modules/closein-tests
diff -N modules/closein-tests
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ modules/closein-tests       27 Apr 2007 16:43:08 -0000
@@ -0,0 +1,13 @@
+Files:
+tests/test-closein.sh
+tests/test-closein.c
+
+Depends-on:
+binary-io
+
+configure.ac:
+
+Makefile.am:
+TESTS += test-closein.sh
+TESTS_ENVIRONMENT += EXEEXT='@EXEEXT@'
+check_PROGRAMS += test-closein
Index: tests/test-closein.c
===================================================================
RCS file: tests/test-closein.c
diff -N tests/test-closein.c
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ tests/test-closein.c        27 Apr 2007 16:43:08 -0000
@@ -0,0 +1,49 @@
+/* Test of closein module.
+   Copyright (C) 2007 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 2, 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, write to the Free Software Foundation,
+   Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  */
+
+/* Written by Eric Blake.  */
+
+#include <config.h>
+
+#include "closein.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "binary-io.h"
+
+char *program_name;
+
+/* With no arguments, do nothing.  With arguments, attempt to consume
+   first 6 bytes of stdin.  In either case, let exit() take care of
+   closing std streams and changing exit status if ferror(stdin).  */
+int
+main (int argc, char **argv)
+{
+  char buf[7];
+  int i = -1;
+  atexit(close_stdin);
+  program_name = argv[0];
+
+  /* close_stdin currently relies on ftell, but mingw ftell is
+     unreliable on text mode input.  */
+  SET_BINARY (0);
+
+  if (argc > 1)
+    i = fread (buf, 1, 6, stdin);
+  return 0;
+}
Index: tests/test-closein.sh
===================================================================
RCS file: tests/test-closein.sh
diff -N tests/test-closein.sh
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ tests/test-closein.sh       27 Apr 2007 16:43:08 -0000
@@ -0,0 +1,38 @@
+#!/bin/sh
+
+tmpfiles=
+trap 'rm -fr $tmpfiles' 1 2 3 15
+
+p=t-closein-
+tmpfiles="${p}in.tmp ${p}xout.tmp ${p}out1.tmp ${p}out2.tmp"
+
+echo Hello world > ${p}in.tmp
+echo world > ${p}xout.tmp
+
+# Test with seekable stdin; followon process must see remaining data
+(./test-closein${EXEEXT}; cat) < ${p}in.tmp > ${p}out1.tmp || exit 1
+cmp ${p}out1.tmp ${p}in.tmp || exit 1
+
+(./test-closein${EXEEXT} consume; cat) < ${p}in.tmp > ${p}out2.tmp || exit 1
+cmp ${p}out2.tmp ${p}xout.tmp || exit 1
+
+# Test for lack of error on pipe
+cat ${p}in.tmp | ./test-closein${EXEEXT} || exit 1
+
+cat ${p}in.tmp | ./test-closein${EXEEXT} consume || exit 1
+
+# Test for lack of error when nothing is read
+./test-closein${EXEEXT} </dev/null || exit 1
+
+./test-closein${EXEEXT} <&- || exit 1
+
+# Test for no error when EOF is read early
+./test-closein${EXEEXT} consume </dev/null || exit 1
+
+# Test for error when read fails because no file available
+./test-closein${EXEEXT} consume <&- 2>/dev/null && exit 1
+
+# Cleanup
+rm -fr $tmpfiles
+
+exit 0