Reading a write-only file doesn't set error condition (was Re: Cygwin fread on Write-Only File Descriptor returns undefined state)

Jeff Johnston jjohnstn@redhat.com
Thu Jun 15 12:33:00 GMT 2006


Corinna Vinschen wrote:
> Thanks for the testcase.  This looks like a small flaw in newlib.
> I redirected to the newlib list.
> 
> On Jun 13 22:29, Linda Walsh wrote:
> 
>>I think I've run into a problem involving fread.
>>I open a test file (fopen) with Write-Only access ("w"),
>>immediately after that, I do an fread on the file handle
>>of some number of bytes. 
>>
>>I expect fread to return an error.  But it doesn't. 
>>It returns 0 as the number of bytes read, and the
>>error value remains unset when querying with ferror.
>>
>>In addition to fread not setting the error value, a value
>>of zero is returned.  Zero is to be returned, *only* on
>>end-of-file or error.  However, in the test case, neither
> 
> 
> That's not correct.  Any value less than size*nitems indicates either
> EOF or an error.  The programmer is responsible to test with feof() or
> ferror() to distinguish between these two cases.  See
> 
> http://www.opengroup.org/onlinepubs/009695399/functions/fread.html
> 
> 
>>value is returned.  Theoretically, this shouldn't happen
>>(i.e. it's an undefined "set" of return values).
>>
>>test program follows:
>>-------------------
>>#include <stdio.h>
>>main (int argc, char * argv[]) {
>>
>>   FILE * output_handle=fopen("/tmp/tmpfile","w");
>>   if (!output_handle) {
>>       printf ("can't open output file /tmp/tmpfile\n");
>>       exit (1);
>>   }  
>>
>>   char mybuff[4];
>>   int retval=fread(mybuff, sizeof(char), 4, output_handle);
>>   if (!retval) {
>>       int eof=feof(output_handle);
>>       int err=ferror(output_handle);
>>       printf ("(retval,eof,err) = (%d,%d,%d)\n",retval,eof,err);
>>       if (!eof && ! err) 
>>           printf ("Undefined error: fread returned zero, but there is 
>>no eof or error\n");
>>       exit(2);
>>   } else
>>       printf ("Unexpected success in fread: Read %d chars\n",retval);
>>  
>>   exit(0);
>>}
> 
> 
> I debugged your testcase and the problem appears to be in __srefill(),
> defined in newlib/libc/stdio/refill.c:
> 
>   /* if not already reading, have to be reading and writing */
>   if ((fp->_flags & __SRD) == 0)
>     {
>       if ((fp->_flags & __SRW) == 0)
>         return EOF;
> 
> So, what happens is that EOF is returned if the file is not readable.
> Errno isn't set and the error condition on the file pointer isn't set
> either.
> 
> Testing the same situation on Linux, errno is set to EBADF and the
> error indicator is set on the file pointer, while the EOF condition
> stays clear.
> 
> So, I'd like to propose the below patch.  I assume a similar patch
> should be ok for __sfvwrite, too, isn't it?  There's a call to
> cantwrite() which only returns EOF but which probably should also
> set the error condition and errno.
> 

This opens a can of worms.  You can't just use _REENT to set errno.  One 
has to provide _r versions of the read/write functions.  I have just 
made a patch and am verifying it builds for x86-linux.  I will post the 
patch when I am ready to check it in.

-- Jeff J.

> 
> 	* libc/stdio/refill.c (__srefill): Return error condition when
> 	trying to read a file not opened for reading.
> 
> Index: libc/stdio/refill.c
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/stdio/refill.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 refill.c
> --- libc/stdio/refill.c	8 Feb 2005 01:33:17 -0000	1.6
> +++ libc/stdio/refill.c	14 Jun 2006 08:19:28 -0000
> @@ -19,6 +19,7 @@
>  #include <_ansi.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> +#include <sys/errno.h>
>  #include "local.h"
>  
>  static int
> @@ -55,7 +56,11 @@ _DEFUN(__srefill, (fp),
>    if ((fp->_flags & __SRD) == 0)
>      {
>        if ((fp->_flags & __SRW) == 0)
> -	return EOF;
> +	{
> +	  _REENT->_errno = EBADF;
> +	  fp->_flags |= __SERR;
> +	  return EOF;
> +	}
>        /* switch to reading */
>        if (fp->_flags & __SWR)
>  	{
> 
> 
> Corinna
> 



More information about the Newlib mailing list