linux-kernel - Re: [PATCH] arch/x86/boot/memory.c: Touched up coding-style issues

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: <989035104a2af28816a3c1b9aeb40d54ee88644a.camel@perches.com>
Date:   Sat, 27 Oct 2018 15:55:19 -0700
From:   Joe Perches <joe@...ches.com>
To:     Jordan Borgner <mail@...dan-borgner.de>, tglx@...utronix.de,
        mingo@...hat.com, bp@...en8.de, x86@...nel.org
Cc:     linux-kernel@...r.kernel.org, hpa@...or.com
Subject: Re: [PATCH] arch/x86/boot/memory.c: Touched up coding-style issues

On Sat, 2018-10-27 at 23:32 +0100, Jordan Borgner wrote:
> Added missing parentheses to sizeof() function in detect_memory_e820().
> 
> Removed unnecessary braces in detect_memory_e801().
> 
> Replaced three if-statements with a ternary if-statement and 
> removed an unnecessary integer variable in detect_memory().
> 
> This is my first patch I hope it is okay.

Hello Jordan.

While the first bit is generally OK, given some individual
maintainer preferences, it may not also be applied.

Whitespace only changes without logical fixes/upgrades are
generally discouraged outside of drivers/staging.

This bit below though changes behavior.

> @@ -121,16 +120,7 @@ static int detect_memory_88(void)
>  
>  int detect_memory(void)
>  {
> -       int err = -1;
> -
> -       if (detect_memory_e820() > 0)
> -               err = 0;
> -
> -       if (!detect_memory_e801())
> -               err = 0;
> -
> -       if (!detect_memory_88())
> -               err = 0;
> -
> -       return err;
> +       return (detect_memory_e820() > 0 ||
> +               !detect_memory_e801()    ||
> +               !detect_memory_88()) ? 0 : -1;
>  }

For instance:

If the first detect_memory_e820 > 0 is true,
the other detect_memory_<e801|88> calls will
not be done.

The original will always perform all three tests.

Regardless, please try to make your first patches
to the linux kernel somewhere in drivers/staging
so get comfortable with the general mechanisms and
styles of kernel patching.