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

[Joe Perches <joe@perches.com>]

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.