linux-kernel - Re: [PATCH] x86/fpu: fix boolreturn.cocci warnings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170307090144.GB6946@gmail.com>
Date:   Tue, 7 Mar 2017 10:01:44 +0100
From:   Ingo Molnar <mingo@...nel.org>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     kbuild test robot <fengguang.wu@...el.com>,
        Rik van Riel <riel@...hat.com>, kbuild-all@...org,
        LKML <linux-kernel@...r.kernel.org>, tipbuild@...or.com,
        "H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Yu-cheng Yu <yu-cheng.yu@...el.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Borislav Petkov <bp@...e.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] x86/fpu: fix boolreturn.cocci warnings

(Linus and Andrew Cc:-ed)

* Thomas Gleixner <tglx@...utronix.de> wrote:

> On Tue, 7 Mar 2017, Ingo Molnar wrote:
> > 
> > * kbuild test robot <fengguang.wu@...el.com> wrote:
> > 
> > > arch/x86/kernel/fpu/xstate.c:931:9-10: WARNING: return of 0/1 in function 'xfeatures_mxcsr_quirk' with return type bool
> > > 
> > >  Return statements in functions returning bool should use
> > >  true/false instead of 1/0.
> > 
> > Note that this is a totally bogus warning. I personally find a 0/1 return more 
> > readable than a textual 'true/false', even if bools are used, and nowhere does the 
> > kernel mandate the use of 0/1.
> 
> I disagree.
> 
> The fact that booleans have been brought retroactively into the C-Standard
> does and for compability reasons C still follows the approach "Boolean
> values are just integers" does not make it any better.
> 
> We had stupid bugs, where people returned -EINVAL from a boolean function
> and introduced silly and hard to understand bugs.

But this function is not using -EINVAL, it's using 0 and 1 which is both correct 
and unambiguous!

I mean, if the Cocci script warned about -EINVAL then it would have found a clear 
bug. Now it's warning about the use of 0/1 literals with bool types which is 
perfectly legal, readable, clear C code!

> The canonical values assigned to booleans are 'true' and 'false' and not 
> whatever people prefer. Can we please be consistent on that?

I think that's backwards, because 1/0 is just as canonical for true/false, and to 
me personally it's in fact easier to read as well.

I would really like higher level buy-in for that principle (I've Cc:-ed Linus and 
Andrew), and if indeed the consensus is that '0/1' cannot be used with 'bool' then 
I'll remove all uses of 'bool' from my patches and from code I care about and use 
'int' instead. Please update Documentation/CodingStyle accordingly as well.

To me a lexical 'true/false' instead of '1/0' is a step backwards in readability 
in many cases - using the slightly wider 'int' type is the lesser evil.

Thanks,

	Ingo