svn commit: r201999 - head/lib/libc/stdio

[Bruce Evans]

On Sun, 10 Jan 2010, Colin Percival wrote:

> Andrey Chernov wrote:
>> On Sun, Jan 10, 2010 at 02:30:30PM +0000, Colin Percival wrote:
> ...
>> 2) fp->_flags |= __SERR;
>> This flag is for errors in the file stream, not for errors in
>> the arguments. Please back that line out.

I agree.

> Quoting fread(3):
>     The function fread() does not distinguish between end-of-file and error,
>     and callers must use feof(3) and ferror(3) to determine which occurred.
> This would seem to imply that for any failed request, either feof or ferror
> should return a non-zero value.  Removing this line would break the common

It would seem to mean a physical i/o error.

> 	while (fread(ptr, reclen, 1, f) == 1) {
> 		/* Do stuff here. */
> 	}
> 	if (ferror(f)) {
> 		/* Die due to read error */
> 	}
> 	/* No error?  Ok, we must have hit EOF */
> idiom.

Hmm.  This case doesn't overflow (reclen * 1 == reclen :-).  fread()
could be finessed by returning a short read (of 1 with no error) for
the silly case.  Then only buggy code that doesn't understand short
reads would fail.

fwrite() hopefully knows that any write of 0 records is an error.

But how about the folling from C99 (old draft n869.txt):

%%%
        [#3] The fwrite function  returns  the  number  of  elements
        successfully  written, which will be less than nmemb only if
        a write error is encountered.
%%%

This doesn't allow any error if there was no write error, and again a
write error would seem to mean a physical one (if it meant any error
it would have said "a fwrite error").  This is unimplementable (except
by not returning when nmemb is more than possible).

>> 3) errno should be EOVERFLOW, see other owerflow checks in the stdio.

There are none.  I (we?) intentionally didn't set errno when the correct
infinite-precision result would overflow in the printf family.  C99
applications wouldn't know to check errno, and buggy ones might be
surprised by unexpected (though permitted) clobbering of errno.  POSIX
didn't have EOVERFLOW when printf was fixed in FreeBSD, and no one has
changed printf to set it, and this change risks breaking C99 applications.

> I picked EINVAL because this is the code used by read(2) and write(2) if they
> are passed nbytes > INT_MAX.  It would seem odd to use one error code for a
> number of bytes between INT_MAX and SIZE_MAX and then switch to a different
> error code for > SIZE_MAX bytes.

A bug in FreeBSD read and write (the result is implementation defined,
but this misbehaviour isn't a result, so it probably isn't allowed when
SSIZE_T == INT_MAX, and it certainly isn't defined when SSIZE_T > INT_MAX
(64-bit arches).

calloc() has the same overflow bug, if any.  Standards seem to require
fread and calloc to work even if the multiplication would occur, though
they cannot work in most cases where the multiplication would occur,
even if the overflow is avoided.  Hmm, fread and fwrite can work, though
calloc cannot:
- fread: when the multiplication would overflow, reduce nmemb to the
          maximum that prevents overflow.  Any more would give an invalid
 	 object size.  Let the read proceed.  If the file is smaller
 	 than the actual object size, then the fread will work, the same
 	 as if nmemb was reasonable to begin with.  Otherwise, undefined
 	 behaviour occurs when the buffer overruns, the same as if
 	 nmemb is just 1 too large to begin with.  There is no reason
 	 to guard against overrun for the silly case when it is impossible
 	 to guard against overrun for a common off-by-1 case.
- fwrite: when the multiplication would overflow, the result of an
          infinitely precise multiplication is an invalid object size.
 	 Writing that many is sure to overrun the buffer.  The behaviour
 	 is undefined.  Reasonable behaviour is an i/o error with EFAULT,
 	 just like would happen for attempting to write beyond the end
 	 of the buffer, if you are lucky enough for the overrun to reach
 	 a page boundary and the next page is unmapped.  You might also
 	 get SIGSEGV if the buffer is accessed in userland.
- calloc: like fwrite.  The object size is invalid so the behaviour is
          undefined.  Could EFAULT/SIGSEGV or just return NULL.

I sent private mail about parts of this and more.

Bruce