Date: Mon, 02 Apr 2007 09:32:38 +0200 From: Alexander Leidinger <Alexander@Leidinger.net> To: "Greg 'groggy' Lehey" <grog@FreeBSD.org> Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: Giving in to Coverity (was: cvs commit: src/sys/netgraph/bluetooth/l2cap ng_l2cap_cmds.c) Message-ID: <20070402093238.dmw2rypu40sksc0o@webmail.leidinger.net> In-Reply-To: <20070402042600.GB19923@wantadilla.lemis.com> References: <200703282125.l2SLPuR9058727@repoman.freebsd.org> <20070329012834.GC79742@wantadilla.lemis.com> <20070329015212.GA97061@heff.fud.org.nz> <20070329133631.e0xqnpftccgc4cow@webmail.leidinger.net> <20070402042600.GB19923@wantadilla.lemis.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Quoting Greg 'groggy' Lehey <grog@FreeBSD.org> (from Mon, 2 Apr 2007 =20 13:56:00 +0930): > On Thursday, 29 March 2007 at 13:36:31 +0200, Alexander Leidinger wrote: >> Quoting Andrew Thompson <thompsa@freebsd.org> (from Thu, 29 Mar 2007 >> 13:52:12 +1200): >> >>> On Thu, Mar 29, 2007 at 10:58:34AM +0930, Greg 'groggy' Lehey wrote: >>>> On Wednesday, 28 March 2007 at 21:25:56 +0000, Maksim Yevmenkin wrote: >>>>> emax 2007-03-28 21:25:56 UTC >>>>> >>>>> FreeBSD src repository >>>>> >>>>> Modified files: >>>>> sys/netgraph/bluetooth/l2cap ng_l2cap_cmds.c >>>>> Log: >>>>> Try to silence Coverity by adding (void) in front of function call. >>>>> Also add a comment, explaining why return value is not being checked= . >>>> >>>> I hope Coverity isn't going to force us to add unnecessary casts to >>>> function calls. >>> >>> Well no, you can always silence Coverity by just marking it as a false >>> bug. >> >> Maxim and me discussed this briefly before this commit. >> >> ... >> >> The cast does not obfuscate the code, doesn't make it harder to read ... > > I've dropped the rest of your argumentation, because I don't disagree > with it, but I do think that unnecessary casts cause (minor) > obfuscation and make it (fractionally) more difficult to read. > > My concern is that we shouldn't compromise our style because of bugs > in program checkers. I understand that there are alternatives, like > flagging it for Coverity as "OK", and I'd expect that to be the > preferable solution. But I'm not the guardian of style, so I'll let > others decide on this if they care. There are several cases where Coverity gets something wrong (e.g. the =20 use of TAILQ). I did mark those as invalid in Coverity (until either =20 we get a new version of Coverity which understands this, or someone =20 writes a model of the TAILQ stuff for Coverity, or until someone tells =20 me to mark them as false positives). I did this because I don't know =20 how to fix this in our code _and_ I see no benefit in fixing this in =20 our code just to make Coverity not moan. For the void cast we are =20 talking about I see a benefit. Coverity can count this as "the return =20 value of this function is checked". As such a report is only generated =20 if a specific percentage of the use of a function is handled this way, =20 it is important if we want to get reports for this. And we want to get =20 reports for functions where the return value typically has to be =20 checked. Bye, Alexander. --=20 Marriage is learning about women the hard way. http://www.Leidinger.net Alexander @ Leidinger.net: PGP ID =3D B0063FE7 http://www.FreeBSD.org netchild @ FreeBSD.org : PGP ID =3D 72077137
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070402093238.dmw2rypu40sksc0o>