On Fri, Jul 14, 2017 at 2:05 PM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> Changing:
>
> - if (!frob()) {
> + if (frob() == 0) {
>
> is a totally pointless change. They're both bad, because they're doing
> success testing instead of failure testing, but probably the second one
> is slightly worse.
>
> This warning seems dumb. I can't imagine it has even a 10% success rate
> at finding real bugs. Just disable it.
>
> Changing the code to propagate error codes, is the right thing of course
> so long as it doesn't introduce bugs.
It found a two of bugs that I fixed earlier:
f0e8faa7a5e8 ("ARM: ux500: fix prcmu_is_cpu_in_wfi() calculation")
af15769ffab1 ("scsi: mvsas: fix command_active typo")
plus three patches from this series:
1. staging:iio:resolver:ad2s1210 fix negative IIO_ANGL_VEL read
2. isdn: isdnloop: suppress a gcc-7 warning (my patch is wrong,
as Joe pointed out there is a real bug)
3. drm/vmwgfx: avoid gcc-7 parentheses (here, Linus had a better
analysis of the problem, so we should consider that a bug as well)
I would estimate around 25% success rate here, which isn't that
bad for a new warning.
I agree that most of the false positives are really dumb though.
Arnd
