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>
index | next in thread | previous in thread | raw e-mail
Quoting Greg 'groggy' Lehey <grog@FreeBSD.org> (from Mon, 2 Apr 2007 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 use of TAILQ). I did mark those as invalid in Coverity (until either we get a new version of Coverity which understands this, or someone writes a model of the TAILQ stuff for Coverity, or until someone tells me to mark them as false positives). I did this because I don't know how to fix this in our code _and_ I see no benefit in fixing this in our code just to make Coverity not moan. For the void cast we are talking about I see a benefit. Coverity can count this as "the return value of this function is checked". As such a report is only generated if a specific percentage of the use of a function is handled this way, it is important if we want to get reports for this. And we want to get reports for functions where the return value typically has to be checked. Bye, Alexander. -- Marriage is learning about women the hard way. http://www.Leidinger.net Alexander @ Leidinger.net: PGP ID = B0063FE7 http://www.FreeBSD.org netchild @ FreeBSD.org : PGP ID = 72077137help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070402093238.dmw2rypu40sksc0o>