Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 11 Jan 2010 00:06:18 +0100
From:      Jilles Tjoelker <jilles@stack.nl>
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:  <20100110230618.GA6756@stack.nl>
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, Jan 10, 2010 at 02:18:58PM -0800, Colin Percival wrote:
> Andrey Chernov wrote:
> > On Sun, Jan 10, 2010 at 02:30:30PM +0000, Colin Percival wrote:
> >> +	if (((count | size) > 0xFFFF) &&
> >> +	    (count > SIZE_MAX / size)) {
> >> +		errno = EINVAL;
> >> +		fp->_flags |= __SERR;
> >> +		return (0);
> >> +	}

> > 2) fp->_flags |= __SERR;
> > This flag is for errors in the file stream, not for errors in 
> > the arguments. Please back that line out.

> 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
> 	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.

Agreed, this error should set the error indicator.

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

> 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.

I think the fact that you can only read(2)/write(2) INT_MAX at a time is
a bug, so basing additional code on it seems inappropriate.

Even then, fread()/fwrite() will not generate such large
read(2)/write(2) requests as they always go through the buffer which is
filled using smaller requests.

Also, EOVERFLOW will give a more understandable error message for users.

-- 
Jilles Tjoelker



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100110230618.GA6756>