Date: Tue, 5 Jun 2018 16:07:16 -0500 From: Eric van Gyzen <vangyzen@FreeBSD.org> To: Ian Lepore <ian@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r334669 - head/sys/sys Message-ID: <a80e0090-614d-2753-97cc-d94cad6ecbdd@FreeBSD.org> In-Reply-To: <1528232004.63685.25.camel@freebsd.org> References: <201806052034.w55KYBsb096418@repo.freebsd.org> <1528232004.63685.25.camel@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?a80e0090-614d-2753-97cc-d94cad6ecbdd>