Giving in to Coverity (was: cvs commit:
jhb at freebsd.org
Mon Apr 2 12:30:52 UTC 2007
On Monday 02 April 2007 03:32:38 am Alexander Leidinger wrote:
> Quoting Greg 'groggy' Lehey <grog at 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 at 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
> >>>> 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
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.
More information about the cvs-src