Date: Mon, 4 Apr 2016 04:58:13 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Pedro Giffuni <pfg@freebsd.org> Cc: Kevin Lo <kevlo@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r297526 - head/sys/geom/sched Message-ID: <20160404043525.P816@besplex.bde.org> In-Reply-To: <5701595A.9070605@FreeBSD.org> References: <201604031625.u33GPpnK088911@repo.freebsd.org> <20160403165827.GA24850@ns.kevlo.org> <5701595A.9070605@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160404043525.P816>