Re: [patch] x86_64: fix earlyprintk=...,keep regression

[Linus Torvalds]

On Tue, 28 Nov 2006, Ingo Molnar wrote:
> 
> the following cleanup patch:
> 
>  commit 2c8c0e6b8d7700a990da8d24eff767f9ca223b96
>  Author: Andi Kleen <ak@xxxxxxx>
>  Date:   Tue Sep 26 10:52:32 2006 +0200
> 
>      [PATCH] Convert x86-64 to early param
> 
>      Instead of hackish manual parsing
> 
> broke the earlyprintk=...,keep feature. This patch restores that 
> functionality. Tested on x86_64. Must-have for v2.6.19, no risk.

Hmm.

> -	if (!strcmp(buf,"keep"))
> +	if (strstr(buf, "keep"))
>  		keep_early = 1;
>  
>  	if (!strncmp(buf, "serial", 6)) {

This is pretty ugly. The whole reason for the bug in the first place was 
that "keep" was tested differently from all the other cases, and now you 
just perpetuate that.

All the other strings are tested with "!strncmp(..)", so I think this one 
should too. No?

Something like the appended (where I made the patch a bit bigger by just 
making it visually look the same too and adding the "} else if .." thing.

Or is there some reason you really _want_ "keep" to be different? If so, 
it should probably be commented on.

		Linus
---
diff --git a/arch/x86_64/kernel/early_printk.c b/arch/x86_64/kernel/early_printk.c
index e22ecd5..51e6912 100644
--- a/arch/x86_64/kernel/early_printk.c
+++ b/arch/x86_64/kernel/early_printk.c
@@ -224,10 +224,9 @@ static int __init setup_early_printk(char *buf)
 		return 0;
 	early_console_initialized = 1;
 
-	if (!strcmp(buf,"keep"))
+	if (!strncmp(buf,"keep", 4)) {
 		keep_early = 1;
-
-	if (!strncmp(buf, "serial", 6)) {
+	} else if (!strncmp(buf, "serial", 6)) {
 		early_serial_init(buf + 6);
 		early_console = &early_serial_console;
 	} else if (!strncmp(buf, "ttyS", 4)) {
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/