arch/x86/boot/memory.c: Touched up coding-style issues
PATCH 1/1 arch/x86/boot/memory.c: Touched up coding-style issues
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.
--- linux-4.19/arch/x86/boot/memory.c | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-)
linux-4.19/arch/x86/boot/memory.c +7 -17
@@ -26,7 +26,7 @@ static int detect_memory_e820(void)initregs (&ireg); ireg.ax =0xe820 ; - ireg.cx =sizeof buf; + ireg.cx =sizeof (buf); ireg.edx =SMAP ; ireg.di = (size_t )&buf;
@@ -88,11 +88,11 @@ static int detect_memory_e801(void) oreg.bx = oreg.dx; } -if (oreg.ax >15 *1024 ) { +if (oreg.ax >15 *1024 )return -1 ;/* Bogus! */ - }else if (oreg.ax ==15 *1024 ) { +else if (oreg.ax ==15 *1024 ) boot_params.alt_mem_k = (oreg.bx <<6 ) + oreg.ax; - }else { +else /* * This ignores memory above 16MB if we have a memory * hole there. If someone actually finds a machine
@@ -101,7 +101,6 @@ static int detect_memory_e801(void)* map. */ boot_params.alt_mem_k = oreg.ax; - }return 0 ; }
@@ -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 ; }
Signature · 1 line
2.11.0
Quoted 8 lines · Jordan Borgner
Added missing parentheses to
sizeof()function indetect_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.
Quoted 10 lines
linux-4.19/arch/x86/boot/memory.c +0
-6@@ -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 ())
linux-4.19/arch/x86/boot/memory.c static int detect_memory_88(void) · line 126
Quoted 10 lines
linux-4.19/arch/x86/boot/memory.c +3
-6- 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.
Quoted 19 lines · Jordan Borgner
Added missing parentheses to
sizeof()function indetect_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.
Signed-off-by Jordan Borgner <[email protected]>--- linux-4.19/arch/x86/boot/memory.c | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-)linux-4.19/arch/x86/boot/memory.c +0
-0
linux-4.19/arch/x86/boot/memory.c static int detect_memory_e820(void) · line 31
Quoted 7 lines
linux-4.19/arch/x86/boot/memory.c +1
-1@@ -26,7 +26,7 @@ static int detect_memory_e820(void)initregs (&ireg); ireg.ax =0xe820 ; - ireg.cx =sizeof buf; + ireg.cx =sizeof (buf); ireg.edx =SMAP ; ireg.di = (size_t )&buf;
That's legit - could you do a single patch that only changes all the 'sizeof x' patterns in arch/x86/ to 'sizeof(x)']?
Quoted 17 lines
linux-4.19/arch/x86/boot/memory.c +3
-3@@ -88,11 +88,11 @@ static int detect_memory_e801(void) oreg.bx = oreg.dx; } -if (oreg.ax >15 *1024 ) { +if (oreg.ax >15 *1024 )return -1 ;/* Bogus! */ - }else if (oreg.ax ==15 *1024 ) { +else if (oreg.ax ==15 *1024 ) boot_params.alt_mem_k = (oreg.bx <<6 ) + oreg.ax; - }else { +else /* * This ignores memory above 16MB if we have a memory * hole there. If someone actually finds a machine
linux-4.19/arch/x86/boot/memory.c static int detect_memory_e801(void) · line 104
Quoted 4 lines
linux-4.19/arch/x86/boot/memory.c +0
-1@@ -101,7 +101,6 @@ static int detect_memory_e801(void)* map. */ boot_params.alt_mem_k = oreg.ax; - }
The original code was better - multi-screen-line statements require curly braces in general.
Quoted 12 lines
linux-4.19/arch/x86/boot/memory.c +0
-5return 0 ; }@@ -121,16 +120,7 @@ static int detect_memory_88(void)int detect_memory (void ) { -int err =-1 ; - -if (detect_memory_e820 () >0 ) - err =0 ; -
linux-4.19/arch/x86/boot/memory.c static int detect_memory_88(void) · line 125
Quoted 10 lines
linux-4.19/arch/x86/boot/memory.c +3
-7-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 ;
Here too I think the original flow of logic was easier to read - more compact is not always better.
Also, please investigate whether the return value is actually *used*, and if not then please send a separate patch that simplifies the code.
Thanks,
Ingo