Date: Mon, 11 Jan 2010 14:29:03 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Colin Percival <cperciva@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, Andrey Chernov <ache@nagual.pp.ru>, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r201999 - head/lib/libc/stdio Message-ID: <20100111134429.F1302@besplex.bde.org> In-Reply-To: <4B4A5252.9070205@freebsd.org> References: <201001101430.o0AEUURS051917@svn.freebsd.org> <20100110212548.GA47331@nagual.pp.ru> <4B4A5252.9070205@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100111134429.F1302>