Re: seekable stdin test failure on OS X

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Paul Eggert on 4/9/2007 11:51 AM:
> Eric Blake <address@hidden> writes:
> 
>> +#elif HAVE___FPURGE
>> +  /* __fpurge has no return value, so we must check errno.  */
>> +  errno = 0;
>> +  __fpurge (stream);
>> +  if (errno)
>> +    result = EOF;
>> +  else
>> +    {
>> +      result = 0;
>> +      errno = e1;
>> +    }
> 
> This doesn't look right.  __fpurge is not documented to set errno
> on Solaris, and I suspect it sets errno to garbage.  I would just
> call __fpurge and ignore errno here.

Fair enough.  I made that change.

> 
>> +    /* Set position of underlying fd first; hopefully we don't confuse
>> +       the stdio routines.  */
>> +    else if (lseek (fileno (stream), position, SEEK_SET) != position
>> +         || fseeko (stream, position, SEEK_SET) != 0)
> 
> I don't get the logic here.  Why call both lseek and fseeko?
> Won't fseeko suffice?  I think the lseek might cause problems.

But some stdio implementations try to optimize, and if the fseeko falls
within the already-read buffer, it does not adjust the offset of the
underlying fd.  However, for now I will omit the lseek (and thus the
dependence on the unistd module), check in the modified patch (attached),
and see how the module fares in testing.

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

        * lib/stdio_.h [REPLACE_FFLUSH]: Declare rpl_fflush.
        * modules/stdio (Makefile.am): Support fflush.
        * m4/stdio_h.m4 (gl_STDIO_H_DEFAULTS): Likewise.
        * modules/fflush: New file.
        * lib/fflush.c: Likewise.
        * m4/fflush.m4: Likewise.
        * modules/fflush-tests: New test.
        * tests/test-fflush.c: Likewise.
        * MODULES.html.sh (Input/output <stdio.h>): Document new module.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGGvqu84KuGfSFAYARAulWAJ922uUMyUU3WyTz+FXzX11DWNVloACg1ery
CE34cp/Tl60xLgIOVclT6bw=
=9DBY
-----END PGP SIGNATURE-----

diff --git a/MODULES.html.sh b/MODULES.html.sh
index 6e6d8c0..505792c 100755
--- a/MODULES.html.sh
+++ b/MODULES.html.sh
@@ -1543,6 +1543,7 @@ func_all_modules ()
   func_echo "$element"
 
   func_begin_table
+  func_module fflush
   func_module fseterr
   func_module tmpfile
   func_end_table
diff --git a/lib/fflush.c b/lib/fflush.c
new file mode 100644
index 0000000..c4c575f
--- /dev/null
+++ b/lib/fflush.c
@@ -0,0 +1,96 @@
+/* fflush.c -- allow flushing input streams
+   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 <errno.h>
+#include <stdio.h>
+
+#if HAVE_STDIO_EXT_H
+# include <stdio_ext.h>
+#endif
+
+#if HAVE_FPURGE && ! HAVE_DECL_FPURGE
+int fpurge (FILE *);
+#endif
+
+#undef fflush
+
+/* Flush all pending data on STREAM according to POSIX rules.  Both
+   output and seekable input streams are supported.  */
+int
+rpl_fflush (FILE *stream)
+{
+  int e1; /* Leave errno unchanged on success.  */
+  int e2; /* Capture errno of first fflush if nothing else succeeds.  */
+  int result;
+
+  /* Try flushing the stream.  C89 guarantees behavior of output
+     streams, so we only need to worry if failure might have been on
+     an input stream.  When stream is NULL, POSIX only requires
+     flushing of output streams.  */
+  e1 = errno;
+  result = fflush (stream);
+  if (! stream || result == 0 || errno != EBADF)
+    return result;
+
+  /* POSIX does not specify behavior for non-seekable streams.  */
+  e2 = errno;
+  if (fseeko (stream, 0, SEEK_CUR) != 0)
+    {
+      errno = e2;
+      return EOF;
+    }
+
+  /* To get here, we must be flushing a seekable input stream, so the
+     semantics of fpurge are now appropriate.  */
+#if HAVE_FPURGE
+  errno = e1;
+  result = fpurge (stream);
+#elif HAVE___FPURGE
+  /* __fpurge has no return value, and on Solaris, we can't even trust
+     errno.  So assume it succeeds.  */
+  __fpurge (stream);
+  result = 0;
+  errno = e1;
+#else /* ! HAVE___FPURGE */
+
+  /* No single replacement; do it manually.  */
+  {
+    off_t position = ftello (stream);
+    if (position == -1)
+      {
+       result = EOF; /* Should not happen; we know stream is seekable.  */
+      }
+    /* Set position of stream; hopefully the stdio routines don't
+       overoptimize by not setting the underlying file position.  */
+    else if (fseeko (stream, position, SEEK_SET) != 0)
+      {
+       result = EOF;
+       errno = e2;
+      }
+    else
+      {
+       result = 0;
+       errno = e1;
+      }
+  }
+#endif /* ! HAVE___FPURGE */
+
+  return result;
+}
diff --git a/lib/stdio_.h b/lib/stdio_.h
index 86d04e1..a87bf8d 100644
--- a/lib/stdio_.h
+++ b/lib/stdio_.h
@@ -207,6 +207,13 @@ extern int vsprintf (char *str, const char *format, 
va_list args)
 # endif
 #endif
 
+#if @GNULIB_FFLUSH@ && @REPLACE_FFLUSH@
+# define fflush rpl_fflush
+  /* Flush all pending data on STREAM according to POSIX rules.  Both
+     output and seekable input streams are supported.  */
+  extern int fflush (FILE *gl_stream);
+#endif
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/m4/fflush.m4 b/m4/fflush.m4
new file mode 100644
index 0000000..a9889d4
--- /dev/null
+++ b/m4/fflush.m4
@@ -0,0 +1,60 @@
+#serial 1
+
+# Copyright (C) 2007 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 Eric Blake
+
+dnl Find out how to obey POSIX semantics of fflush(stdin) discarding
+dnl unread input, rather than C99 undefined semantics.  fpurge is not
+dnl standardized, but has the desired properties.
+
+AC_DEFUN([gl_FUNC_FFLUSH],
+[
+  AC_CACHE_CHECK([whether fflush works on input streams],
+    [gl_cv_func_fflush_stdin],
+    [echo hello world > conftest.txt
+     AC_RUN_IFELSE([AC_LANG_PROGRAM(
+       [[
+#include <stdio.h>
+#include <unistd.h>
+       ]], [[FILE *f = fopen ("conftest.txt", "r");
+         char buffer[10];
+        int fd = fileno (f);
+         if (!f || 0 > fd || fread (buffer, 1, 5, f) != 5)
+          return 2;
+        /* For deterministic results, ensure f read a bigger buffer.  */
+        if (lseek (fd, 0, SEEK_CUR) == 5)
+          return 3;
+        /* POSIX requires fflush-fseek to set file offset of fd.  */
+        if (fflush (f) != 0 || fseek (f, 0, SEEK_CUR) != 0)
+          return 4;
+        return !(lseek (fd, 0, SEEK_CUR) == 5);
+       ]])], [gl_cv_func_fflush_stdin=yes], [gl_cv_func_fflush_stdin=no],
+     [dnl Pessimistically assume fflush is broken.  This is wrong for
+      dnl at least glibc and cygwin; but lib/fflush.c takes this into account.
+      gl_cv_func_fflush_stdin=no])
+     rm conftest.txt
+    ])
+  if test $gl_cv_func_fflush_stdin = no; then
+    gl_REPLACE_FFLUSH
+  fi
+])
+
+AC_DEFUN([gl_REPLACE_FFLUSH],
+[
+  AC_CHECK_HEADERS_ONCE([stdio_ext.h])
+  AC_CHECK_FUNCS_ONCE([fpurge __fpurge])
+dnl Linux documents int fpurge(), but only declares void __fpurge().
+  AC_CHECK_DECLS([fpurge], [], [], [[
+#include <stdio.h>
+#if HAVE_STDIO_EXT_H
+# include <stdio_ext.h>
+#endif
+]])
+  AC_LIBOBJ([fflush])
+  AC_REQUIRE([gl_STDIO_H_DEFAULTS])
+  REPLACE_FFLUSH=1
+])
diff --git a/m4/stdio_h.m4 b/m4/stdio_h.m4
index 5561626..d53997f 100644
--- a/m4/stdio_h.m4
+++ b/m4/stdio_h.m4
@@ -30,6 +30,7 @@ AC_DEFUN([gl_STDIO_H_DEFAULTS],
   GNULIB_VSNPRINTF=0;      AC_SUBST([GNULIB_VSNPRINTF])
   GNULIB_VSPRINTF_POSIX=0; AC_SUBST([GNULIB_VSPRINTF_POSIX])
   GNULIB_VASPRINTF=0;      AC_SUBST([GNULIB_VASPRINTF])
+  GNULIB_FFLUSH=0;         AC_SUBST([GNULIB_FFLUSH])
   dnl Assume proper GNU behavior unless another module says otherwise.
   REPLACE_FPRINTF=0;       AC_SUBST([REPLACE_FPRINTF])
   REPLACE_VFPRINTF=0;      AC_SUBST([REPLACE_VFPRINTF])
@@ -43,4 +44,5 @@ AC_DEFUN([gl_STDIO_H_DEFAULTS],
   REPLACE_VSPRINTF=0;      AC_SUBST([REPLACE_VSPRINTF])
   HAVE_VASPRINTF=1;        AC_SUBST([HAVE_VASPRINTF])
   REPLACE_VASPRINTF=0;     AC_SUBST([REPLACE_VASPRINTF])
+  REPLACE_FFLUSH=0;        AC_SUBST([REPLACE_FFLUSH])
 ])
diff --git a/modules/fflush b/modules/fflush
new file mode 100644
index 0000000..3b2cb99
--- /dev/null
+++ b/modules/fflush
@@ -0,0 +1,23 @@
+Description:
+Discard pending data on both input and output streams.
+
+Files:
+lib/fflush.c
+m4/fflush.m4
+
+Depends-on:
+stdio
+
+configure.ac:
+gl_FUNC_FFLUSH
+gl_STDIO_MODULE_INDICATOR([fflush])
+
+Makefile.am:
+
+Include:
+
+License:
+GPL
+
+Maintainer:
+Eric Blake
diff --git a/modules/fflush-tests b/modules/fflush-tests
new file mode 100644
index 0000000..64b3840
--- /dev/null
+++ b/modules/fflush-tests
@@ -0,0 +1,12 @@
+Files:
+tests/test-fflush.c
+
+Depends-on:
+unistd
+
+configure.ac:
+
+Makefile.am:
+TESTS += test-fflush
+check_PROGRAMS += test-fflush
+EXTRA_DIST += test-fflush.c
diff --git a/modules/stdio b/modules/stdio
index 456e323..3ee2d0b 100644
--- a/modules/stdio
+++ b/modules/stdio
@@ -30,6 +30,7 @@ stdio.h: stdio_.h
              -e 's|@''GNULIB_VSNPRINTF''@|$(GNULIB_VSNPRINTF)|g' \
              -e 's|@''GNULIB_VSPRINTF_POSIX''@|$(GNULIB_VSPRINTF_POSIX)|g' \
              -e 's|@''GNULIB_VASPRINTF''@|$(GNULIB_VASPRINTF)|g' \
+             -e 's|@''GNULIB_FFLUSH''@|$(GNULIB_FFLUSH)|g' \
              -e 's|@''REPLACE_FPRINTF''@|$(REPLACE_FPRINTF)|g' \
              -e 's|@''REPLACE_VFPRINTF''@|$(REPLACE_VFPRINTF)|g' \
              -e 's|@''REPLACE_PRINTF''@|$(REPLACE_PRINTF)|g' \
@@ -42,6 +43,7 @@ stdio.h: stdio_.h
              -e 's|@''REPLACE_VSPRINTF''@|$(REPLACE_VSPRINTF)|g' \
              -e 's|@''HAVE_VASPRINTF''@|$(HAVE_VASPRINTF)|g' \
              -e 's|@''REPLACE_VASPRINTF''@|$(REPLACE_VASPRINTF)|g' \
+             -e 's|@''REPLACE_FFLUSH''@|$(REPLACE_FFLUSH)|g' \
              -e '/definition of GL_LINK_WARNING/r $(LINK_WARNING_H)' \
              < $(srcdir)/stdio_.h; \
        } > address@hidden
diff --git a/tests/test-fflush.c b/tests/test-fflush.c
new file mode 100644
index 0000000..276d8bf
--- /dev/null
+++ b/tests/test-fflush.c
@@ -0,0 +1,88 @@
+/* Test of POSIX compatible fflush() function.
+   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, 2007.  */
+
+#ifdef HAVE_CONFIG_H
+# include <config.h>
+#endif
+
+#include <stdio.h>
+#include <unistd.h>
+
+int
+main (int argc, char *argv[])
+{
+  FILE *f;
+  char buffer[10];
+  int fd;
+
+  /* Create test file.  */
+  f = fopen ("test-fflush.txt", "w");
+  if (!f || fwrite ("1234567890", 1, 10, f) != 10 || fclose (f) != 0)
+    {
+      fputs ("Failed to create sample file.\n", stderr);
+      unlink ("test-fflush.txt");
+      return 1;
+    }
+
+  /* Test fflush.  */
+  f = fopen ("test-fflush.txt", "r");
+  fd = fileno (f);
+  if (!f || 0 > fd || fread (buffer, 1, 5, f) != 5)
+    {
+      fputs ("Failed initial read of sample file.\n", stderr);
+      fclose (f);
+      unlink ("test-fflush.txt");
+      return 1;
+    }
+  /* For deterministic results, ensure f read a bigger buffer.  */
+  if (lseek (fd, 0, SEEK_CUR) == 5)
+    {
+      fputs ("Sample file was not buffered.\n", stderr);
+      fclose (f);
+      unlink ("test-fflush.txt");
+      return 1;
+    }
+  /* POSIX requires fflush-fseek to set file offset of fd.  */
+  if (fflush (f) != 0 || fseek (f, 0, SEEK_CUR) != 0)
+    {
+      fputs ("Failed to flush sample file.\n", stderr);
+      fclose (f);
+      unlink ("test-fflush.txt");
+      return 1;
+    }
+  /* Check that offset is correct.  */
+  if (lseek (fd, 0, SEEK_CUR) != 5)
+    {
+      fputs ("File offset is wrong.\n", stderr);
+      fclose (f);
+      unlink ("test-fflush.txt");
+      return 1;
+    }
+  /* Check that file reading resumes at correct location.  */
+  if (fgetc (f) != '6')
+    {
+      fputs ("Failed to read next byte of file.\n", stderr);
+      fclose (f);
+      unlink ("test-fflush.txt");
+      return 1;
+    }
+  fclose (f);
+  unlink ("test-fflush.txt");
+  return 0;
+}