Re: fflush after ungetc

[Bruno Haible]

Eric Blake wrote:
> Newlib has two bugs - first, fflush is failing to discard ungetc data when
> changing the underlying fd offset.

This bug is common to all BSD and AT&T Unix derived implementations. For BSD
systems, I'm committing this fix. For AT&T Unix derived implementations, I
don't see an easy fix.


2008-03-06  Bruno Haible  <address@hidden>

        Make fflush after ungetc work on BSD platforms.
        * lib/fflush.c (rpl_fflush): Discard ungetc buffer if possible.
        * tests/test-fflush2.c: New file.
        * tests/test-fflush2.sh: New file.
        * modules/fflush-tests (Files): Add tests/test-fflush2.sh,
        tests/test-fflush2.c.
        (Makefile.am): Build test-fflush2 and run test-fflush2.sh.
        * doc/posix-functions/fflush.texi: Document fflush after ungetc bug.

*** lib/fflush.c.orig   2008-03-07 03:24:45.000000000 +0100
--- lib/fflush.c        2008-03-07 02:44:45.000000000 +0100
***************
*** 1,5 ****
  /* 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
--- 1,5 ----
  /* fflush.c -- allow flushing input streams
!    Copyright (C) 2007-2008 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
***************
*** 59,64 ****
--- 59,104 ----
    if (stream == NULL || ! freading (stream))
      return fflush (stream);
  
+   /* Clear the ungetc buffer.
+ 
+      This is needed before fetching the file-position indicator, because
+      1) The file position indicator is incremented by fgetc() and decremented
+         by ungetc():
+         <http://www.opengroup.org/susv3/functions/fgetc.html>
+           "The file-position indicator is decremented by each successful
+            call to ungetc()..."
+         <http://www.opengroup.org/susv3/functions/ungetc.html>
+           "... the fgetc() function shall ... advance the associated file
+         position indicator for the stream ..."
+      2) <http://www.opengroup.org/susv3/functions/ungetc.html> says:
+           "The value of the file-position indicator for the stream after
+            reading or discarding all pushed-back bytes shall be the same
+            as it was before the bytes were pushed back."
+      3) Here we are discarding all pushed-back bytes.
+ 
+      Unfortunately it is impossible to implement this on platforms with
+      _IOERR, because an ungetc() on this platform prepends the pushed-back
+      bytes to the buffer without an indication of the limit between the
+      pushed-back bytes and the read-ahead bytes.  */
+ #if defined __sferror               /* FreeBSD, NetBSD, OpenBSD, MacOS X, 
Cygwin */
+   {
+ # if defined __NetBSD__ || defined __OpenBSD__
+     struct __sfileext
+       {
+       struct  __sbuf _ub; /* ungetc buffer */
+       /* More fields, not relevant here.  */
+       };
+     if (((struct __sfileext *) stream->_ext._base)->_ub._base != NULL)
+ # else
+     if (stream->_ub._base != NULL)
+ # endif
+       {
+       stream->_p += stream->_r;
+       stream->_r = 0;
+       }
+   }
+ #endif
+ 
    /* POSIX does not specify fflush behavior for non-seekable input
       streams.  Some implementations purge unread data, some return
       EBADF, some do nothing.  */
======================= tests/test-fflush2.c ============================
/* Test of POSIX compatible fflush() function.
   Copyright (C) 2008 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 <stdio.h>

#include <stdlib.h>

/* This test can only be made to work on specific platforms.  */
#if defined _IO_ferror_unlocked || defined __sferror /* GNU libc, BeOS; 
FreeBSD, NetBSD, OpenBSD, MacOS X, Cygwin */
# define ASSERT(expr) \
  do                                                                         \
    {                                                                        \
      if (!(expr))                                                           \
        {                                                                    \
          fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \
          abort ();                                                          \
        }                                                                    \
    }                                                                        \
  while (0)
#else
# define ASSERT(expr) \
  do                                                                         \
    {                                                                        \
      if (!(expr))                                                           \
        {                                                                    \
          printf ("Skipping test: expected failure on this platform\n");     \
          exit (77);                                                         \
        }                                                                    \
    }                                                                        \
  while (0)
#endif

int
main (int argc, char **argv)
{
  /* Check that fflush after a non-backup ungetc() call discards the ungetc
     buffer.  This is mandated by POSIX
     <http://www.opengroup.org/susv3/functions/ungetc.html>:
       "The value of the file-position indicator for the stream after
        reading or discarding all pushed-back bytes shall be the same
        as it was before the bytes were pushed back."  */
  int c;

  c = fgetc (stdin);
  ASSERT (c == '#');

  c = fgetc (stdin);
  ASSERT (c == '!');

  /* Here the file-position indicator must be 2.  */

  c = ungetc ('@', stdin);
  ASSERT (c == '@');

  fflush (stdin);

  /* Here the file-position indicator must be 2 again.  */

  c = fgetc (stdin);
  ASSERT (c == '/');

  return 0;
}
=============================== tests/test-fflush2.sh =========================
#!/bin/sh

# Execute the test only with seekable input stream.
# The behaviour of fflush() on a non-seekable input stream is undefined.
./test-fflush2${EXEEXT} < "$srcdir/test-fflush2.sh" || exit $?
#cat "$srcdir/test-fflush2.sh" | ./test-fflush2${EXEEXT} || exit $?

exit 0
===============================================================================
*** modules/fflush-tests.orig   2008-03-07 03:24:45.000000000 +0100
--- modules/fflush-tests        2008-03-06 22:59:12.000000000 +0100
***************
*** 1,5 ****
--- 1,7 ----
  Files:
  tests/test-fflush.c
+ tests/test-fflush2.sh
+ tests/test-fflush2.c
  
  Depends-on:
  fseeko
***************
*** 7,12 ****
  configure.ac:
  
  Makefile.am:
! TESTS += test-fflush
! check_PROGRAMS += test-fflush
  MOSTLYCLEANFILES += test-fflush.txt
--- 9,15 ----
  configure.ac:
  
  Makefile.am:
! TESTS += test-fflush test-fflush2.sh
! TESTS_ENVIRONMENT += EXEEXT='@EXEEXT@' srcdir='$(srcdir)'
! check_PROGRAMS += test-fflush test-fflush2
  MOSTLYCLEANFILES += test-fflush.txt
*** doc/posix-functions/fflush.texi.orig        2008-03-07 03:24:45.000000000 
+0100
--- doc/posix-functions/fflush.texi     2008-03-07 03:05:55.000000000 +0100
***************
*** 15,20 ****
--- 15,25 ----
  @item
  @code{fflush} on an input stream changes the position of the stream to the
  end of the previous buffer, on some platforms: mingw.
+ @item
+ @code{fflush} on an input stream right after @code{ungetc} does not discard
+ the @code{ungetc} buffer, on some platforms:
+ MacOS X 10.3, FreeBSD 6.0, NetBSD 3.0, OpenBSD 3.8, Cygwin.
+ Cygwin.
  @end itemize
  
  Portability problems not fixed by Gnulib:
***************
*** 26,29 ****
--- 31,38 ----
  @item
  On Windows platforms (excluding Cygwin), this function does not set 
@code{errno}
  upon failure.
+ @item
+ @code{fflush} on an input stream right after @code{ungetc} does not discard
+ the @code{ungetc} buffer, on some platforms:
+ AIX 5.1, HP-UX 11, IRIX 6.5, OSF/1 5.1, Solaris 10, mingw.
  @end itemize