From owner-svn-src-all@freebsd.org Sun Apr 3 18:58:20 2016 Return-Path: Delivered-To: svn-src-all@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 D913AB0101F; Sun, 3 Apr 2016 18:58:20 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail104.syd.optusnet.com.au (mail104.syd.optusnet.com.au [211.29.132.246]) by mx1.freebsd.org (Postfix) with ESMTP id A4E3B1A08; Sun, 3 Apr 2016 18:58:20 +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 mail104.syd.optusnet.com.au (Postfix) with ESMTPS id ED5E94228C9; Mon, 4 Apr 2016 04:58:14 +1000 (AEST) Date: Mon, 4 Apr 2016 04:58:13 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Pedro Giffuni cc: Kevin Lo , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r297526 - head/sys/geom/sched In-Reply-To: <5701595A.9070605@FreeBSD.org> Message-ID: <20160404043525.P816@besplex.bde.org> References: <201604031625.u33GPpnK088911@repo.freebsd.org> <20160403165827.GA24850@ns.kevlo.org> <5701595A.9070605@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=EfU1O6SC c=1 sm=1 tr=0 a=73JWPhLeruqQCjN69UNZtQ==:117 a=L9H7d07YOLsA:10 a=9cW_t1CCXrUA:10 a=s5jvgZ67dGcA:10 a=kj9zAlcOel0A:10 a=QsJJmg0kW5a1Qme-mrcA:9 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.21 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, 03 Apr 2016 18:58:21 -0000 On Sun, 3 Apr 2016, Pedro Giffuni wrote: > On 04/03/16 11:58, Kevin Lo wrote: >> On Sun, Apr 03, 2016 at 04:25:51PM +0000, Pedro F. Giffuni wrote: >>> Log: >>> g_sched_destroy(): prevent return of uninitialized scalar variable. >>> >>> For the !gsp case there some chance of returning an uninitialized >>> return value. Prevent that from happening by initializing the >>> error value. >> >> Hmm, wouldn't it be better to initialize 'error' before use? > > No. The if case initializes error on line 1278, the only problem > is the else case. Indeed. >> Index: sys/geom/sched/g_sched.c >> =================================================================== >> --- sys/geom/sched/g_sched.c (revision 297527) >> +++ sys/geom/sched/g_sched.c (working copy) >> @@ -1236,7 +1236,7 @@ g_sched_destroy(struct g_geom *gp, boolean_t force >> struct g_provider *pp, *oldpp = NULL; >> struct g_sched_softc *sc; >> struct g_gsched *gsp; >> - int error; >> + int error = 0; >> >> g_topology_assert(); >> sc = gp->softc; > > Even when this is frequent, it is against style(9). style(9) itself was broken to not FORBID initializing variables in declarations. The style bug is frequent for "fixing" -Wuninitialised warnings -- the code had an obscure flow of control that is too hard for compilers and humans to understand, and the "fix" is to make it even harder to understand by initializing the variable to a value that is probably wrong if it is ever used. It would be better to do the reverse and initialize variables immediately before use and kill them after their use so that their lifetime is clear without a deep analysis, but the kill operations would take large code and isn't in C. You can approximate it using neted declarations, but I don't like that and style(9) forbids it. You can approximate it by a macro kill() that might expand to assigning a NaN to the variables. Compilers would probably object to this macro if it actually assigns a NaN. Bruce