From owner-cvs-all@FreeBSD.ORG Mon Apr 2 14:28:57 2007 Return-Path: X-Original-To: cvs-all@FreeBSD.org Delivered-To: cvs-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 2C0B216A408; Mon, 2 Apr 2007 14:28:57 +0000 (UTC) (envelope-from wes@opensail.org) Received: from softweyr.homeunix.net (cpe-24-161-160-202.san.res.rr.com [24.161.160.202]) by mx1.freebsd.org (Postfix) with ESMTP id DFF1B13C44B; Mon, 2 Apr 2007 14:28:56 +0000 (UTC) (envelope-from wes@opensail.org) Received: from [204.68.178.179] ([204.68.178.179]) (authenticated bits=0) by softweyr.homeunix.net (8.13.8/8.13.8) with ESMTP id l32EBFkB084752; Mon, 2 Apr 2007 07:11:15 -0700 (PDT) (envelope-from wes@opensail.org) In-Reply-To: <200704020821.15298.jhb@freebsd.org> References: <200703282125.l2SLPuR9058727@repoman.freebsd.org> <20070402042600.GB19923@wantadilla.lemis.com> <20070402093238.dmw2rypu40sksc0o@webmail.leidinger.net> <200704020821.15298.jhb@freebsd.org> Mime-Version: 1.0 (Apple Message framework v752.3) Content-Type: text/plain; charset=US-ASCII; delsp=yes; format=flowed Message-Id: <3B10268D-C554-4EC2-8F16-388A49982AF3@opensail.org> Content-Transfer-Encoding: 7bit From: Wes Peters Date: Mon, 2 Apr 2007 07:11:10 -0700 To: John Baldwin X-Mailer: Apple Mail (2.752.3) Cc: Greg 'groggy' Lehey , Alexander Leidinger , src-committers@FreeBSD.org, cvs-all@FreeBSD.org, cvs-src@FreeBSD.org Subject: Re: Giving in to Coverity (was: cvs commit: src/sys/netgraph/bluetooth/l2cap ng_l2cap_cmds.c) X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 Apr 2007 14:28:57 -0000 On Apr 2, 2007, at 5:21 AM, John Baldwin wrote: > On Monday 02 April 2007 03:32:38 am Alexander Leidinger wrote: >> Quoting Greg 'groggy' Lehey (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 (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. Void casts are a valuable tool, they tell the next poor dumb programmer who comes along "I know this returns a value but I'm too lazy or stupid to check it." I find I often get bitten by these, in my own code and in others. I mean, it's not like printf can overflow your buffer or anything, is it? -- Where am I, and what am I doing in this handbasket? Wes Peters wes@softweyr.com