Giving in to Coverity (was: cvs commit: src/sys/netgraph/bluetooth/l2cap ng_l2cap_cmds.c)

[John Baldwin]

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 
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