This is the mail archive of the newlib@sourceware.org mailing list for the newlib project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On 27/10/09 03:49 PM, Corinna Vinschen wrote:
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.


Glibc adds to this under info lseek:


    `ESPIPE'
          The FILEDES corresponds to an object that cannot be
          positioned, such as a pipe, FIFO or terminal device.
          (POSIX.1 specifies this error only for pipes and FIFOs, but
          in the GNU system, you always get `ESPIPE' if the object is
          not seekable.)

I interpret this that Linux always return ESPIPE for unseekable devices.

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.


I have no issue with removing the check for position equality. I get a slightly different result on Fedora Linux with /dev/urandom. I get back -1 and errno set to 22 (Invalid argument) for the seek of -65528.


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


Your change allows -1 to be returned if no errno is set. None of the above tests exhibit this, but you may know differently. If -1 and errno-not-set is possible, then the earlier check in fflush where -1 returned from seek causes failure unless ESPIPE is set, might require similar checking to what you are proposing.


There might be an issue if any libgloss implementation does not properly set errno on an lseek failure, but I know none off-hand and have only checked a handful of platforms.

If an unseekable device returns -1 and sets ESPIPE, shouldn't you ignore this situation as is done above in the same code? (i.e. there is no requirement to do a seek once the read buffer is cleared).

So one possible solution would be to change the added check to be (rc != -1 || ptr->errno == ESPIPE || ptr->errno == 0), with the last check for errno being removed if it is not feasible that -1 will be returned with errno 0.

-- Jeff J.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]