Re: [PATCH/RFA] _fflush_r check seek result fix

[Corinna Vinschen <vinschen at redhat dot com>]

On Oct 27 15:14, Jeff Johnston wrote:
> On 23/10/09 09:08 AM, Corinna Vinschen wrote:
> >Hi,
> >
> >there was a short thread on the Cygwin list, in which it turned out that
> >fclose on certain Cygwin devices opened for readin could return an
> >error: http://cygwin.com/ml/cygwin/2009-10/msg00562.html
> >
> >I tracked it down to the _fflush_r function.  _fflush_r calls fp->seek,
> >basically like this:
> >
> >   curoff = fp->_seek (0, SEEK_CUR);
> >   curoff = -= fp->_r; // Take buffer position into account
> >   tmp = (fp->_seek (curoff, SEEK_SET) == curoff)
> >   if (tmp)
> >     // Success
> >   else
> >     // Failure
> >
> >This code ignores the possibility that the offset returned by seek does
> >not exactly match the desired position.  This result is no error
> >condition, especially when taking devices into account.  It's especially
> >no error if lseek returns a negative offset on character special
> >devices.
> >Please note that lseek on the Cygwin devices mentioned in the above
> >thread behave exactly like their Linux counterparts.  Thus, the same
> >_fseek_r would also treat the Linux behaviour as error.
> >[...]
> 
> So for these devices, you're saying seek is supported, but they
> cannot return their position (correct)?  Otherwise, if seek isn't
> supported, why don't they set ESPIPE?

Per POSIX, ESPIPE has to be returned if the descriptor references
a pipe, a fifo, or a socket.  POSIX does not require this for block
or character special devices.  Quote:

  The behavior of lseek() on devices which are incapable of seeking is
  implementation-defined. The value of the file offset associated with
  such a device is undefined.

Please note that Cygwin tries to emulate Linux behaviour in the first
place.  So the Cygwin devices behave just like the same Linux devices.

Try this code on Linux...

  #include <stdio.h>
  #include <sys/fcntl.h>
  #include <string.h>
  #include <errno.h>

  int
  main (int argc, char **argv)
  {
    int fd = open (argv[1], O_RDONLY);
    if (fd >= 0)
      {
	char buf[65536];
	off_t pos = lseek (fd, 0, SEEK_CUR);
	printf ("pos(0,CUR): %lld\n", (long long) pos);
	pos = read (fd, buf, 65536);
	printf ("pos(read): %lld\n", (long long) pos);
	pos = lseek (fd, 0, SEEK_CUR);
	printf ("pos(0, CUR): %lld\n", (long long) pos);
	pos = lseek (fd, -65528, SEEK_CUR);
	printf ("pos(-65528,CUR): %lld\n", (long long) pos);
	pos = lseek (fd, 0, SEEK_CUR);
	printf ("pos(0, CUR): %lld\n", (long long) pos);
	close (fd);
      }
    else
      printf ("open: %d <%s>\n", errno, strerror (errno));
    return 0;
  }

...with the following devices:

  $ gcc -o seektest seektest.c

  $ ./seektest /dev/urandom
  pos(0,CUR): 0
  pos(read): 65536
  pos(0, CUR): 0
  pos(-65528,CUR): -65528
  pos(0, CUR): -65528

  $ ./seektest /dev/zero
  pos(0,CUR): 0
  pos(read): 65536
  pos(0, CUR): 0
  pos(-65528,CUR): 0
  pos(0, CUR): 0

  $ ./seektest /dev/full
  pos(0,CUR): 0
  pos(read): 65536
  pos(0, CUR): 0
  pos(-65528,CUR): 0
  pos(0, CUR): 0

As you can see, lseek() on these devices does not return an error.
However, the position returned to the calling function doesn't match the
position you'd expect if you made the same operation on a file.

> Above the code you are changing is a check that either takes the
> file offset that has been tracked or tries to calculate it.  If it
> tries to calculate and -1 is returned, it returns 0 if ESPIPE,
> otherwise, it returns failure (it fails to reset errno in the case
> of the ignored ESPIPE).
> 
> Should that logic be changed to match yours or should your new logic
> do the same? (i.e. treat ESPIPE as ok and otherwise -1 is failure).

I'm not sure I can follow.  How are you going to change that code if
the curoff value doesn't reflect an error condition at all?!?


Corinna

-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat