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

From: Joe Perches
Date: Sat Oct 27 2018 - 18:55:26 EST


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.