Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 11 Jan 2010 00:25:48 +0300
From:      Andrey Chernov <ache@nagual.pp.ru>
To:        Colin Percival <cperciva@FreeBSD.ORG>
Cc:        svn-src-head@FreeBSD.ORG, svn-src-all@FreeBSD.ORG, src-committers@FreeBSD.ORG
Subject:   Re: svn commit: r201999 - head/lib/libc/stdio
Message-ID:  <20100110212548.GA47331@nagual.pp.ru>
In-Reply-To: <201001101430.o0AEUURS051917@svn.freebsd.org>
References:  <201001101430.o0AEUURS051917@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Jan 10, 2010 at 02:30:30PM +0000, Colin Percival wrote:
> +	 * Check for integer overflow.  As an optimization, first check that
> +	 * at least one of {count, size} is at least 2^16, since if both
> +	 * values are less than that, their product can't possible overflow
> +	 * (size_t is always at least 32 bits on FreeBSD).
> +	 */
> +	if (((count | size) > 0xFFFF) &&
> +	    (count > SIZE_MAX / size)) {
> +		errno = EINVAL;
> +		fp->_flags |= __SERR;
> +		return (0);
> +	}

1) I don't think that this is good place of exact constants like 0xFFFF, 
usually we don't use such things in overflow checks (see all other ones).
fread/fwrite are already slow as designed, so optimizing one time argument 
check looks strange.

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

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

-- 
http://ache.pp.ru/



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