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

Alexander Leidinger Alexander at Leidinger.net
Thu Mar 29 11:36:39 UTC 2007


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 return value of the function is checked most of the time (8 out of  
9 times in this case). The cast tells that the return value of the  
function is ignored by intend. It is not necessary for humans to  
understand the code, but it allows humans to gain more insight than  
without it. The comment tells even more, so a human would not need the  
cast, but it should allow to give statistical checkers like Coverity  
Prevent more hints. I don't know if this version of Coverity Prevent  
understands this hint or not (and if it isn't, I will mark this CID up  
as IGNORE). But in the light of such "useless" comments as "/*  
FALLTHROUGH */" to "please lint" I don't think such a comment about  
unnecessary casts is appropriate. I don't object to add hints for  
lint, they provide some additional info to a human being too.

The cast does not obfuscate the code, doesn't make it harder to read  
or understand and doesn't cause deeper indenting or nesting or  
whatever. It also allows statistical checkers to count this as a  
checked return value, so next time the return value of the same  
function at another place is not checked, it knows about it (at least  
theoretically, I don't know if this is the case for the version of  
Coverity Prevent we use).

If you still think this cast is a bad idea, feel free to explain why  
this is the case in your opinion.

Bye,
Alexander.

-- 
Someone is speaking well of you.
How unusual!

http://www.Leidinger.net    Alexander @ Leidinger.net: PGP ID = B0063FE7
http://www.FreeBSD.org       netchild @ FreeBSD.org  : PGP ID = 72077137


More information about the cvs-src mailing list