svn commit: r334669 - head/sys/sys

[Eric van Gyzen]

On 06/05/2018 15:53, Ian Lepore wrote:
> On Tue, 2018-06-05 at 20:34 +0000, Eric van Gyzen wrote:
>> Author: vangyzen
>> Date: Tue Jun  5 20:34:11 2018
>> New Revision: 334669
>> URL: https://svnweb.freebsd.org/changeset/base/334669
>>
>> Log:
>>   Make Coverity more happy with r334545
>>   
>>   Coverity complains about:
>>   
>>   	if (((flags) & M_WAITOK) || _malloc_item != NULL)
>>   
>>   saying:
>>   
>>   	The expression
>>   		1 /* (2 | 0x100) & 2 */ || _malloc_item != NULL
>>   	is suspicious because it performs a Boolean operation
>>   	on a constant other than 0 or 1.
>>   
>>   Although the code is correct, add "!= 0" to make it slightly
>>   more legible and to silence hundreds(?) of Coverity warnings.
>>   
> 
> This is a sad sad thing. Treating (bits & flagconstants) as boolean has
> a long long history in C. Surely there are literally thousand of
> occurrances in freebsd code already, so why did this one get flagged?

I agree, and I tend to avoid adding "!= 0" unnecessarily, but I don't
feel very strongly about it.  This macro is expanded in many locations,
so the number of Coverity warnings increased by hundreds in the most
recent run.  I care about that more than avoiding "!= 0".  I don't
sprinkle crap all over the code just to appease Coverity, but this one
seemed perfectly reasonable.  It makes the code slightly more clear and
legible for some readers, and I imagine it doesn't hurt the others.

Yes, there are probably many old occurrences of this, and there might be
many old corresponding warnings, but I tend to focus on the recently
added ones, just because they're more likely relevant.

/me opens the static analysis can of worms...again

Eric