Date: Mon, 2 Apr 2007 07:11:10 -0700 From: Wes Peters <wes@opensail.org> To: John Baldwin <jhb@FreeBSD.org> Cc: Greg 'groggy' Lehey <grog@FreeBSD.org>, Alexander Leidinger <Alexander@leidinger.net>, 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) Message-ID: <3B10268D-C554-4EC2-8F16-388A49982AF3@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>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <grog@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@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.
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3B10268D-C554-4EC2-8F16-388A49982AF3>