linux-kernel - Re: [PATCH 1/2] x86/bootflag: Change some static functions to bool

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: <CAFULd4bTYudfNap1trVyjqA0xv5cQQeWxSZ8numv_uHqxz1Afw@mail.gmail.com>
Date: Mon, 24 Feb 2025 08:39:20 +0100
From: Uros Bizjak <ubizjak@...il.com>
To: Jiri Slaby <jirislaby@...nel.org>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org, 
	Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...nel.org>, Borislav Petkov <bp@...en8.de>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, "H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH 1/2] x86/bootflag: Change some static functions to bool

On Mon, Feb 24, 2025 at 8:27 AM Jiri Slaby <jirislaby@...nel.org> wrote:
>
> On 24. 02. 25, 8:24, Uros Bizjak wrote:
> > On Mon, Feb 24, 2025 at 8:18 AM Jiri Slaby <jirislaby@...nel.org> wrote:
> >>
> >> On 29. 01. 25, 16:47, Uros Bizjak wrote:
> >>> The return values of some functions are of boolean type. Change the
> >>> type of these function to bool and adjust their return values.
> >>>
> >>> No functional change intended.
> >>>
> >>> Signed-off-by: Uros Bizjak <ubizjak@...il.com>
> >>> Cc: Thomas Gleixner <tglx@...utronix.de>
> >>> Cc: Ingo Molnar <mingo@...nel.org>
> >>> Cc: Borislav Petkov <bp@...en8.de>
> >>> Cc: Dave Hansen <dave.hansen@...ux.intel.com>
> >>> Cc: "H. Peter Anvin" <hpa@...or.com>
> >>> ---
> >>>    arch/x86/kernel/bootflag.c | 12 ++++++------
> >>>    1 file changed, 6 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/arch/x86/kernel/bootflag.c b/arch/x86/kernel/bootflag.c
> >>> index 3fed7ae58b60..4d89a2d80d0f 100644
> >>> --- a/arch/x86/kernel/bootflag.c
> >>> +++ b/arch/x86/kernel/bootflag.c
> >>> @@ -20,7 +20,7 @@
> >>>
> >>>    int sbf_port __initdata = -1;       /* set via acpi_boot_init() */
> >>>
> >>> -static int __init parity(u8 v)
> >>> +static bool __init parity(u8 v)
> >>>    {
> >>>        int x = 0;
> >>>        int i;
> >>> @@ -30,7 +30,7 @@ static int __init parity(u8 v)
> >>>                v >>= 1;
> >>>        }
> >>>
> >>> -     return x;
> >>> +     return !!x;
> >>
> >> This "!!" is unnecessary and only obfuscates the code, right?
> >
> > Not really, this idiom is used in place of (x != 0) to change the type
> > to the return type of the function in a pedantic way.
>
> Care to explain what exactly it changes?

The internal compiler representation of the following testcase:

_Bool foo (int x) { return x; }

is:

--cut here--
_Bool foo (int x)
{
 _Bool _2;

 <bb 2> [local count: 1073741824]:
 _2 = x_1(D) != 0;
 return _2;
}
--cut here--

For me, !!x in the source means that the change of types was
intentional. Surely, the compiler can do it by itself, so at the end
of the day, it is just a matter of personal taste.

FYI, the whole function will soon be removed and replaced with a
generic parity8() function.

Uros.