From owner-svn-src-head@freebsd.org Wed Mar 30 07:42:15 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 1D708AE2F72; Wed, 30 Mar 2016 07:42:15 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail109.syd.optusnet.com.au (mail109.syd.optusnet.com.au [211.29.132.80]) by mx1.freebsd.org (Postfix) with ESMTP id DB4EB1A62; Wed, 30 Mar 2016 07:42:14 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from c110-21-41-193.carlnfd1.nsw.optusnet.com.au (c110-21-41-193.carlnfd1.nsw.optusnet.com.au [110.21.41.193]) by mail109.syd.optusnet.com.au (Postfix) with ESMTPS id A4CE4D663B1; Wed, 30 Mar 2016 18:41:59 +1100 (AEDT) Date: Wed, 30 Mar 2016 18:41:58 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: "Pedro F. Giffuni" cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r297408 - head/lib/libc/stdio In-Reply-To: <201603300613.u2U6DwGv026746@repo.freebsd.org> Message-ID: <20160330180123.V3090@besplex.bde.org> References: <201603300613.u2U6DwGv026746@repo.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=c+ZWOkJl c=1 sm=1 tr=0 a=73JWPhLeruqQCjN69UNZtQ==:117 a=L9H7d07YOLsA:10 a=9cW_t1CCXrUA:10 a=s5jvgZ67dGcA:10 a=kj9zAlcOel0A:10 a=7aRgpVi7GxIzNs7i03cA:9 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 30 Mar 2016 07:42:15 -0000 On Wed, 30 Mar 2016, Pedro F. Giffuni wrote: > Log: > freopen(3): prevent uninitialized errno. > > Revert r297407 and redo it cleanly. This might need a few more commits to do it correctly. > Modified: head/lib/libc/stdio/freopen.c > ============================================================================== > --- head/lib/libc/stdio/freopen.c Wed Mar 30 01:32:08 2016 (r297407) > +++ head/lib/libc/stdio/freopen.c Wed Mar 30 06:13:58 2016 (r297408) > @@ -66,8 +66,7 @@ freopen(const char * __restrict file, co > (void) fclose(fp); > errno = sverrno; > return (NULL); > - } else > - sverrno = 0; > + } This uses the one value that is clearly wrong. C Standard library functions are not permitted to set errno to 0, but that is what happens if this setting of sverrno is actually used. Most C Standard library functions are permitted to set errno to any nonzero value even if they succeed. For this particular function, errno has an unspecified value in Standard C after the function returns, but POSIX extends this and specifies some settings. This extension is permitted because, unlike for some other functions, Standard C doesn't specify anything for errno. It would not be wrong to set sverrno to errno here, of course without the style bug, but any use of the value would be dubious. > > FLOCKFILE(fp); > > @@ -79,6 +78,7 @@ freopen(const char * __restrict file, co > * re-open the same file with a different mode. We allow this only > * if the modes are compatible. > */ > + sverrno = 0; > if (file == NULL) { > /* See comment below regarding freopen() of closed files. */ > if (fp->_flags == 0) { Setting it to 0 here is equivalent to setting it above, except it doesn't have the style bug. Actually saving errno in it is not quite eqivalent to saving errno above, since there are functions that might set errno in between. It should be set above, or better not at all. You need to understand the function better to place it exactly correctly. The function only restores from sverrno after the "finish" label in the f < 0 case. I think it intends to restore the errno that was set for one of the 2 'f = _open(...)' expressions, so if it actually uses the above setting then that is a bug. A complete fix would reorganise the function to avoid "goto finish" so that it is clear to humans and coverities that the above setting is never used. Bruce