Date: Thu, 29 Mar 2007 13:36:31 +0200 From: Alexander Leidinger <Alexander@Leidinger.net> To: "Greg 'groggy' Lehey" <grog@FreeBSD.org> Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, Maksim Yevmenkin <emax@FreeBSD.org>, cvs-all@FreeBSD.org, Andrew Thompson <thompsa@FreeBSD.org> Subject: Re: Giving in to Coverity (was: cvs commit: src/sys/netgraph/bluetooth/l2cap ng_l2cap_cmds.c) Message-ID: <20070329133631.e0xqnpftccgc4cow@webmail.leidinger.net> In-Reply-To: <20070329015212.GA97061@heff.fud.org.nz> References: <200703282125.l2SLPuR9058727@repoman.freebsd.org> <20070329012834.GC79742@wantadilla.lemis.com> <20070329015212.GA97061@heff.fud.org.nz>
next in thread | previous in thread | raw e-mail | index | archive | help
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 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070329133631.e0xqnpftccgc4cow>