Re: new module 'freadseek'

[Eric Blake]

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

According to Eric Blake on 3/1/2008 1:36 PM:
| On further investigation, I think this is a true bug in newlib's stdio,
| and not in your code.

Newlib has two bugs - first, fflush is failing to discard ungetc data when
changing the underlying fd offset.  Second, ftell is improperly calling
fflush under the hood, which when coupled with the first bug, results in a
confused file offset, but even when the first bug is fixed, loses data
contrary to POSIX.  (BSD's ftell probably also calls fflush under the
hood, but since BSD's fflush disobeys POSIX by treating fflush on a read
stream as a no-op rather than discarding unread data, these bugs are not
visible there).

$ cat foo.c
#include <stdio.h>
#include <stdlib.h>

int
main (int argc, char* argv[])
{
~  int i1, i2;
~  i1 = fgetc (stdin);
~  printf ("i1=%c\n", i1);
~  i1 = ungetc ('@', stdin);
~  printf ("i1=%c\n", i1);
~  if (argc > 1)
~    {
~      i2 = ftell (stdin);
~      printf ("i2=%d\n", i2);
~    }
~  i1 = fgetc (stdin);
~  printf ("i1=%c\n", i1);
~  return 0;
}
$ ./foo < foo.c
i1=#
i1=@
i1=@
$ ./foo a < foo.c
i1=#
i1=@
i2=-337
i1=i

On Linux, and after my patch, it properly does:
% ./foo < foo.c
i1=#
i1=@
i1=@
% ./foo a < foo.c
i1=#
i1=@
i2=0
i1=@

OK to apply this to newlib?  Patching gnulib to work around these newlib
bugs for older releases of cygwin seems daunting; it's hard to do anything
when you can't trust ftell or fflush after ungetc.

2008-03-01  Eric Blake  <address@hidden>

        Fix ftell bug after ungetc.
        * libc/stdio/ftell.c (_ftell_r): Don't flush ungetc buffer on
        ftell.
        * libc/stdio64/ftello64.c (_ftello64_r): Likewise.
        * libc/stdio/fflush.c (_fflush_r): Clear unget buffer when
        repositioning underlying fd offset.

- --
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

iD8DBQFHyec984KuGfSFAYARAoezAKDC6snVwkdf1ZcSS0NbgTZ5fRMLBACaAnSl
Cixn14d6TZ56/1PH6WgR10A=
=/Dr1
-----END PGP SIGNATURE-----

Index: libc/stdio/fflush.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio/fflush.c,v
retrieving revision 1.11
diff -u -p -r1.11 fflush.c
--- libc/stdio/fflush.c 13 Jul 2007 20:37:53 -0000      1.11
+++ libc/stdio/fflush.c 1 Mar 2008 22:41:26 -0000
@@ -164,6 +164,8 @@ _DEFUN(_fflush_r, (ptr, fp),
              fp->_p = fp->_bf._base;
              if (fp->_flags & __SOFF)
                fp->_offset = curoff;
+              if (HASUB (fp))
+               FREEUB (ptr, fp);
            }
          else
            {
Index: libc/stdio/ftell.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio/ftell.c,v
retrieving revision 1.12
diff -u -p -r1.12 ftell.c
--- libc/stdio/ftell.c  13 Jul 2007 20:37:53 -0000      1.12
+++ libc/stdio/ftell.c  1 Mar 2008 22:41:26 -0000
@@ -118,9 +118,12 @@ _DEFUN(_ftell_r, (ptr, fp),
       return -1L;
     }
 
-  /* Find offset of underlying I/O object, then
-     adjust for buffered bytes.  */
-  _fflush_r (ptr, fp);           /* may adjust seek offset on append stream */
+  /* Find offset of underlying I/O object, then adjust for buffered
+     bytes.  Flush a write stream, since the offset may be altered if
+     the stream is appending.  Do not flush a read stream, since we
+     must not lose the ungetc buffer.  */
+  if (fp->_flags & __SWR)
+    _fflush_r (ptr, fp);
   if (fp->_flags & __SOFF)
     pos = fp->_offset;
   else
Index: libc/stdio64/ftello64.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio64/ftello64.c,v
retrieving revision 1.8
diff -u -p -r1.8 ftello64.c
--- libc/stdio64/ftello64.c     13 Jul 2007 20:37:53 -0000      1.8
+++ libc/stdio64/ftello64.c     1 Mar 2008 22:41:26 -0000
@@ -108,9 +108,12 @@ _DEFUN (_ftello64_r, (ptr, fp),
       return -1L;
     }
 
-  /* Find offset of underlying I/O object, then
-     adjust for buffered bytes.  */
-  _fflush_r (ptr, fp);           /* may adjust seek offset on append stream */
+  /* Find offset of underlying I/O object, then adjust for buffered
+     bytes.  Flush a write stream, since the offset may be altered if
+     the stream is appending.  Do not flush a read stream, since we
+     must not lose the ungetc buffer.  */
+  if (fp->_flags & __SWR)
+    _fflush_r (ptr, fp);
   if (fp->_flags & __SOFF)
     pos = fp->_offset;
   else