From owner-svn-src-all@FreeBSD.ORG Sun Jan 10 23:06:20 2010 Return-Path: Delivered-To: svn-src-all@FreeBSD.ORG Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 497F2106566B; Sun, 10 Jan 2010 23:06:20 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mx1.stack.nl (relay02.stack.nl [IPv6:2001:610:1108:5010::104]) by mx1.freebsd.org (Postfix) with ESMTP id 0EA848FC14; Sun, 10 Jan 2010 23:06:20 +0000 (UTC) Received: from toad.stack.nl (toad.stack.nl [IPv6:2001:610:1108:5010::135]) by mx1.stack.nl (Postfix) with ESMTP id EC08B359958; Mon, 11 Jan 2010 00:06:18 +0100 (CET) Received: by toad.stack.nl (Postfix, from userid 1677) id E150973F9D; Mon, 11 Jan 2010 00:06:18 +0100 (CET) Date: Mon, 11 Jan 2010 00:06:18 +0100 From: Jilles Tjoelker To: Colin Percival Message-ID: <20100110230618.GA6756@stack.nl> References: <201001101430.o0AEUURS051917@svn.freebsd.org> <20100110212548.GA47331@nagual.pp.ru> <4B4A5252.9070205@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4B4A5252.9070205@freebsd.org> User-Agent: Mutt/1.5.18 (2008-05-17) Cc: svn-src-head@FreeBSD.ORG, Andrey Chernov , svn-src-all@FreeBSD.ORG, src-committers@FreeBSD.ORG Subject: Re: svn commit: r201999 - head/lib/libc/stdio X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 10 Jan 2010 23:06:20 -0000 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