init: Fix printk format strings
PATCH 1/1 init: Fix printk format strings
This makes sure printk format strings are string literals containing no more than a single line.
--- init/calibrate.c | 4 +++- init/do_mounts_initrd.c | 5 ++++- init/main.c | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-)
init/calibrate.c +3 -1
@@ -98,7 +98,9 @@ static unsigned long __devinit calibrate_delay_direct(void)return (good_tsc_sum/good_tsc_count);printk (KERN_WARNING "calibrate_delay_direct() failed to get a good " -"estimate for loops_per_jiffy.\nProbably due to long platform interrupts. Consider using \"lpj=\" boot option.\n" ); +"estimate for loops_per_jiffy.\n" ); +printk (KERN_WARNING "Probably due to long platform interrupts. " +"Consider using \"lpj=\" boot option.\n" ); return 0; }#else
init/do_mounts_initrd.c +4 -1
@@ -98,7 +98,10 @@ static void __init handle_initrd(void) error =sys_ioctl (fd,BLKFLSBUF ,0 );sys_close (fd); } -printk (!error ?"okay\n" :"failed\n" ); +if (error) +printk ("failed\n" ); +else +printk ("okay\n" ); } }
init/main.c +1 -1
@@ -537,7 +537,7 @@ asmlinkage void __init start_kernel(void)boot_cpu_init ();page_address_init ();printk (KERN_NOTICE ); -printk (linux_banner); +printk ("%s" , linux_banner);setup_arch (&command_line);setup_command_line (command_line);unwind_setup ();
Signature · 1 line
1.5.2.4
Quoted 2 lines · On Thu, 11 Oct 2007 08:17:02 +0200 Vegard Nossum wrote:
This makes sure printk format strings are string literals containing no more than a single line.
Each patch needs justification (unless it is blatantly obvious).
Quoted 12 lines
Signed-off-by Vegard Nossum <[email protected]>--- init/calibrate.c | 4 +++- init/do_mounts_initrd.c | 5 ++++- init/main.c | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-)init/do_mounts_initrd.c +0
-0
init/do_mounts_initrd.c static void __init handle_initrd(void) · line 106
Quoted 10 lines
init/do_mounts_initrd.c +4
-1@@ -98,7 +98,10 @@ static void __init handle_initrd(void) error =sys_ioctl (fd,BLKFLSBUF ,0 );sys_close (fd); } -printk (!error ?"okay\n" :"failed\n" ); +if (error) +printk ("failed\n" ); +else +printk ("okay\n" ); } }
Why this one?
and if it must change, use:
if (error)
but I see little need for the change.
---
~Randy
Quoted 27 lines · Randy Dunlap
Quoted 2 lines · On Thu, 11 Oct 2007 08:17:02 +0200 Vegard Nossum wrote:
This makes sure printk format strings are string literals containing no more than a single line.
Each patch needs justification (unless it is blatantly obvious).
Quoted 19 lines
Signed-off-by Vegard Nossum <[email protected]>--- init/calibrate.c | 4 +++- init/do_mounts_initrd.c | 5 ++++- init/main.c | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-)init/do_mounts_initrd.c +3
-1@@ -98,7 +98,10 @@ static void __init handle_initrd(void) error =sys_ioctl (fd,BLKFLSBUF ,0 );sys_close (fd); } -printk (!error ?"okay\n" :"failed\n" ); +if (error) +printk ("failed\n" ); +else
Quoted 8 lines
init/do_mounts_initrd.c static void __init handle_initrd(void) · line 106
Quoted 3 lines
init/do_mounts_initrd.c +1
-0+printk ("okay\n" ); } }Why this one?
and if it must change, use:
if (error)
but I see little need for the change.
Oops about the space. And it's to make the format string a constant literal. If it isn't, you are going to run into trouble with options to remove printks at compile-time based on log-level token. I realize it's not really an issue at this point in time, but I assume that it will eventually make its way into the kernel in some way or another.
Vegard
Quoted 37 lines · On Thu, 11 Oct 2007 17:55:16 +0200 Vegard Nossum wrote:
Quoted 27 lines · Randy Dunlap
Quoted 2 lines · On Thu, 11 Oct 2007 08:17:02 +0200 Vegard Nossum wrote:
This makes sure printk format strings are string literals containing no more than a single line.
Each patch needs justification (unless it is blatantly obvious).
Quoted 19 lines
Signed-off-by Vegard Nossum <[email protected]>--- init/calibrate.c | 4 +++- init/do_mounts_initrd.c | 5 ++++- init/main.c | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-)init/do_mounts_initrd.c +3
-1@@ -98,7 +98,10 @@ static void __init handle_initrd(void) error =sys_ioctl (fd,BLKFLSBUF ,0 );sys_close (fd); } -printk (!error ?"okay\n" :"failed\n" ); +if (error) +printk ("failed\n" ); +else Quoted 8 lines
init/do_mounts_initrd.c static void __init handle_initrd(void) · line 106
Quoted 3 lines
init/do_mounts_initrd.c +1
-0+printk ("okay\n" ); } }Why this one?
and if it must change, use:
if (error)
but I see little need for the change.
Quoted 5 lines
Oops about the space. And it's to make the format string a constant literal. If it isn't, you are going to run into trouble with options to remove printks at compile-time based on log-level token. I realize it's not really an issue at this point in time, but I assume that it will eventually make its way into the kernel in some way or another.
so would this be OK?
printk("%s\n", error ? "failed" : "okay");---
~Randy
Quoted 45 lines · Randy Dunlap
Quoted 37 lines · On Thu, 11 Oct 2007 17:55:16 +0200 Vegard Nossum wrote:
Quoted 27 lines · Randy Dunlap
Quoted 2 lines · On Thu, 11 Oct 2007 08:17:02 +0200 Vegard Nossum wrote:
This makes sure printk format strings are string literals containing no more than a single line.
Each patch needs justification (unless it is blatantly obvious).
Quoted 19 lines
Signed-off-by Vegard Nossum <[email protected]>--- init/calibrate.c | 4 +++- init/do_mounts_initrd.c | 5 ++++- init/main.c | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-)init/do_mounts_initrd.c +3
-1@@ -98,7 +98,10 @@ static void __init handle_initrd(void) error =sys_ioctl (fd,BLKFLSBUF ,0 );sys_close (fd); } -printk (!error ?"okay\n" :"failed\n" ); +if (error) +printk ("failed\n" ); +else Quoted 8 lines
init/do_mounts_initrd.c static void __init handle_initrd(void) · line 106
Quoted 3 lines
init/do_mounts_initrd.c +1
-0+printk ("okay\n" ); } }Why this one?
and if it must change, use:
if (error)
but I see little need for the change.Quoted 5 lines
Oops about the space. And it's to make the format string a constant literal. If it isn't, you are going to run into trouble with options to remove printks at compile-time based on log-level token. I realize it's not really an issue at this point in time, but I assume that it will eventually make its way into the kernel in some way or another.
Quoted 3 lines
so would this be OK?
printk("%s\n", error ? "failed" : "okay");
It helps the filtering, but it doesn't help localisation very much, I think. Since for that, you'd only translate the format string, which in this case isn't much to translate. (I do realize that this is a very, very tiny part of the kernel and probably hundreds of other call-sites have much worse problems -- but every little helps, and now that you're at it... :-))
Vegard
Quoted 48 lines · On Thu, 11 Oct 2007 18:09:47 +0200 Vegard Nossum wrote:
Quoted 47 lines · Randy Dunlap
Quoted 37 lines · On Thu, 11 Oct 2007 17:55:16 +0200 Vegard Nossum wrote:
Quoted 27 lines · Randy Dunlap
Quoted 2 lines · On Thu, 11 Oct 2007 08:17:02 +0200 Vegard Nossum wrote:
This makes sure printk format strings are string literals containing no more than a single line.
Each patch needs justification (unless it is blatantly obvious).
Quoted 19 lines
Signed-off-by Vegard Nossum <[email protected]>--- init/calibrate.c | 4 +++- init/do_mounts_initrd.c | 5 ++++- init/main.c | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-)init/do_mounts_initrd.c +3
-1@@ -98,7 +98,10 @@ static void __init handle_initrd(void) error =sys_ioctl (fd,BLKFLSBUF ,0 );sys_close (fd); } -printk (!error ?"okay\n" :"failed\n" ); +if (error) +printk ("failed\n" ); +else Quoted 8 lines
init/do_mounts_initrd.c static void __init handle_initrd(void) · line 106
Quoted 3 lines
init/do_mounts_initrd.c +1
-0+printk ("okay\n" ); } }Why this one?
and if it must change, use:
if (error)
but I see little need for the change.Quoted 5 lines
Oops about the space. And it's to make the format string a constant literal. If it isn't, you are going to run into trouble with options to remove printks at compile-time based on log-level token. I realize it's not really an issue at this point in time, but I assume that it will eventually make its way into the kernel in some way or another.
so would this be OK?
Quoted 8 lines
Quoted 1 line
printk("%s\n", error ? "failed" : "okay");It helps the filtering, but it doesn't help localisation very much, I think. Since for that, you'd only translate the format string, which in this case isn't much to translate. (I do realize that this is a very, very tiny part of the kernel and probably hundreds of other call-sites have much worse problems -- but every little helps, and now that you're at it... :-))
Oh well.
I find the proposed changes too restrictive, practically changing a stream-oriented output into a variable-length block-oriented output.
---
~Randy
Hi,
Quoted 2 lines · Vegard Nossum
This makes sure printk format strings are string literals containing no more than a single line.
Perhaps you should write _why_ one-line printk()s are even needed, with profound reasons instead of constantly talking about mysterious later changes that you will propose any time soon.
Hannes
Quoted 7 lines · Johannes Weiner
Quoted 2 lines · Vegard Nossum
This makes sure printk format strings are string literals containing no more than a single line.
Perhaps you should write _why_ one-line printk()s are even needed, with profound reasons instead of constantly talking about mysterious later changes that you will propose any time soon.
In short, the reason for disallowing multiple lines per call is that printk() will be less complex, while not really changing the complexity of the callers (a change in the code, yes, but the changes are trivial, and, actually, not that many).
I think I see now, though, that my changes will not gain support if I do not lift this restriction.
You can find my current "mysterious later changes" as patches, with descriptions, at this address: http://folk.uio.no/vegardno/xprintf/
Thanks for the tip, though. I am quite new to the kernel business, so bear with me :)
Kind regards,
Vegard Nossum