Date: Mon, 2 Apr 2007 08:21:12 -0400 From: John Baldwin <jhb@freebsd.org> To: Alexander Leidinger <Alexander@leidinger.net> Cc: Greg 'groggy' Lehey <grog@freebsd.org>, 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: <200704020821.15298.jhb@freebsd.org> In-Reply-To: <20070402093238.dmw2rypu40sksc0o@webmail.leidinger.net> References: <200703282125.l2SLPuR9058727@repoman.freebsd.org> <20070402042600.GB19923@wantadilla.lemis.com> <20070402093238.dmw2rypu40sksc0o@webmail.leidinger.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Monday 02 April 2007 03:32:38 am Alexander Leidinger wrote: > 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. There is previous history of casting a function's return value to (void) to please lint(1). Just look for '(void)printf' :) Coverity at least is smarter than lint as it doesn't warn about printf not being checked. -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200704020821.15298.jhb>