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

[Corinna Vinschen <vinschen at redhat dot com>]

Jeff,

could you please have a look into this patch?


Thanks,
Corinna

On Oct 23 15:08, 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.
> 
> Below is a patch which tries to fix this problem.  The idea is that only
> a return code of -1 with errno set to some value != 0 is really an
> error.  To accomplish this, the code stores the current errno, calls
> fp->seek, and then checks for a return value of -1 and the errno.  If no
> error has happened from the point of view of the underlying device, it
> treats the result as success, sets the new offset, and restores the old
> errno.  Otherwise, an error occured and the new errno is returned to the
> calling code.
> 
> Is that ok?
> 
> 
> Corinna
> 
> 
> 	* libc/stdio/fflush.c (_fflush_r): Don't use new position
> 	after seek as error condition, rather check for return value of
> 	-1 and errno.
> 
> 
> Index: libc/stdio/fflush.c
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/stdio/fflush.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 fflush.c
> --- libc/stdio/fflush.c	22 Jul 2009 02:17:12 -0000	1.14
> +++ libc/stdio/fflush.c	23 Oct 2009 13:05:23 -0000
> @@ -115,7 +115,7 @@ _DEFUN(_fflush_r, (ptr, fp),
>           to miss a code scenario.  */
>        if ((fp->_r > 0 || fp->_ur > 0) && fp->_seek != NULL)
>  	{
> -	  int tmp;
> +	  int tmp_errno;
>  #ifdef __LARGE64_FILES
>  	  _fpos64_t curoff;
>  #else
> @@ -155,13 +155,15 @@ _DEFUN(_fflush_r, (ptr, fp),
>                  curoff -= fp->_ur;
>              }
>  	  /* Now physically seek to after byte last read.  */
> +	  tmp_errno = ptr->_errno;
> +	  ptr->_errno = 0;
>  #ifdef __LARGE64_FILES
>  	  if (fp->_flags & __SL64)
> -	    tmp = (fp->_seek64 (ptr, fp->_cookie, curoff, SEEK_SET) == curoff);
> +	    curoff = fp->_seek64 (ptr, fp->_cookie, curoff, SEEK_SET);
>  	  else
>  #endif
> -	    tmp = (fp->_seek (ptr, fp->_cookie, curoff, SEEK_SET) == curoff);
> -	  if (tmp)
> +	    curoff = fp->_seek (ptr, fp->_cookie, curoff, SEEK_SET);
> +	  if (curoff != -1 || ptr->_errno == 0)
>  	    {
>  	      /* Seek successful.  We can clear read buffer now.  */
>  	      fp->_flags &= ~__SNPT;
> @@ -169,6 +171,7 @@ _DEFUN(_fflush_r, (ptr, fp),
>  	      fp->_p = fp->_bf._base;
>  	      if (fp->_flags & __SOFF)
>  		fp->_offset = curoff;
> +	      ptr->_errno = tmp_errno;
>  	      if (HASUB (fp))
>  		FREEUB (ptr, fp);
>  	    }
> 
> 
> 
> -- 
> Corinna Vinschen
> Cygwin Project Co-Leader
> Red Hat

-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat