From owner-svn-src-head@FreeBSD.ORG  Sun Jan 10 21:25:51 2010
Return-Path: <owner-svn-src-head@FreeBSD.ORG>
Delivered-To: svn-src-head@FreeBSD.ORG
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34])
	by hub.freebsd.org (Postfix) with ESMTP id 649B3106566B;
	Sun, 10 Jan 2010 21:25:51 +0000 (UTC)
	(envelope-from ache@nagual.pp.ru)
Received: from nagual.pp.ru (nagual.pp.ru [194.87.13.69])
	by mx1.freebsd.org (Postfix) with ESMTP id 96EB38FC17;
	Sun, 10 Jan 2010 21:25:50 +0000 (UTC)
Received: from nagual.pp.ru (ache@localhost [127.0.0.1])
	by nagual.pp.ru (8.14.3/8.14.3) with ESMTP id o0ALPnPB047475;
	Mon, 11 Jan 2010 00:25:49 +0300 (MSK)
	(envelope-from ache@nagual.pp.ru)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=nagual.pp.ru;
	s=default; t=1263158749;
	bh=AMUw04gd551RrKYslYeylXGMNVf2rYs9i8bLHzoqd7k=; l=984;
	h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version:
	Content-Type:In-Reply-To;
	b=F4e8VIqUgug0cUbTpBV87k86owrjSfJzg5WlQjtfU2oiEJUcK/lDQ080AYmpjOcaY
	J1wyRnn/GfbnJvMjbXSHFNVkX1zcJ9pcLQKp19I8oAUdhfH47tWLQmWgc9rAyV47tu
	4G5AmMoNVDmzBzSEhvsyIWfd4sS4fSyTXm9aXOO8=
Received: (from ache@localhost)
	by nagual.pp.ru (8.14.3/8.14.3/Submit) id o0ALPmbB047474;
	Mon, 11 Jan 2010 00:25:49 +0300 (MSK) (envelope-from ache)
Date: Mon, 11 Jan 2010 00:25:48 +0300
From: Andrey Chernov <ache@nagual.pp.ru>
To: Colin Percival <cperciva@FreeBSD.ORG>
Message-ID: <20100110212548.GA47331@nagual.pp.ru>
Mail-Followup-To: Andrey Chernov <ache@nagual.pp.ru>,
	Colin Percival <cperciva@FreeBSD.ORG>, src-committers@FreeBSD.ORG,
	svn-src-all@FreeBSD.ORG, svn-src-head@FreeBSD.ORG
References: <201001101430.o0AEUURS051917@svn.freebsd.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <201001101430.o0AEUURS051917@svn.freebsd.org>
User-Agent: Mutt/1.5.20 (2009-06-14)
Cc: svn-src-head@FreeBSD.ORG, svn-src-all@FreeBSD.ORG,
	src-committers@FreeBSD.ORG
Subject: Re: svn commit: r201999 - head/lib/libc/stdio
X-BeenThere: svn-src-head@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: SVN commit messages for the src tree for head/-current
	<svn-src-head.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/svn-src-head>,
	<mailto:svn-src-head-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/svn-src-head>
List-Post: <mailto:svn-src-head@freebsd.org>
List-Help: <mailto:svn-src-head-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/svn-src-head>,
	<mailto:svn-src-head-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Sun, 10 Jan 2010 21:25:51 -0000

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/