2.6.7-bk: asm/setup.h and linux/init.h

2.6.7-bk: asm/setup.h and linux/init.h

[Russell King]

Hi,

I notice that recent changes resulted in asm/setup.h being included by
linux/init.h, for the saved_command_line change:

@@ -66,6 +67,9 @@

 extern initcall_t __con_initcall_start, __con_initcall_end;
 extern initcall_t __security_initcall_start, __security_initcall_end;
+
+/* Defined in init/main.c */
+extern char saved_command_line[COMMAND_LINE_SIZE];
 #endif

 #ifndef MODULE

Unfortunately, this causes problems on ARM, because asm/setup.h on
ARM needs things like linux/types.h, which I'd rather not include
here since:

1. it means that we'll be at odds with what other architectures include
   (and therefore will hide a missing linux/types.h include when
   developing on ARM.)
2. linux/init.h is included by the majority of the kernel, and I'd
   rather not have asm/setup.h added to the dependency of every single
   kernel source file.

Is there a reason why we can't delete asm/setup.h from linux/init.h
and change that declaration to:

+extern char saved_command_line[];

?

IOW, like this (which works fine on ARM):

===== include/linux/init.h 1.32 vs edited =====
--- 1.32/include/linux/init.h	Thu Jun 24 09:55:46 2004
+++ edited/include/linux/init.h	Sat Jun 26 12:50:09 2004
@@ -3,7 +3,6 @@
 
 #include <linux/config.h>
 #include <linux/compiler.h>
-#include <asm/setup.h>
 
 /* These macros are used to mark some functions or 
  * initialized data (doesn't apply to uninitialized data)
@@ -69,7 +68,7 @@
 extern initcall_t __security_initcall_start, __security_initcall_end;
 
 /* Defined in init/main.c */
-extern char saved_command_line[COMMAND_LINE_SIZE];
+extern char saved_command_line[];
 #endif
   
 #ifndef MODULE
===== init/main.c 1.148 vs edited =====
--- 1.148/init/main.c	Thu Jun 24 09:55:46 2004
+++ edited/init/main.c	Sat Jun 26 12:51:27 2004
@@ -47,6 +47,7 @@
 
 #include <asm/io.h>
 #include <asm/bugs.h>
+#include <asm/setup.h>
 
 /*
  * This is one of the first .c files built. Error out early


-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: 2.6.7-bk: asm/setup.h and linux/init.h

[Linus Torvalds]

On Sat, 26 Jun 2004, Russell King wrote:
> 
> Is there a reason why we can't delete asm/setup.h from linux/init.h
> and change that declaration to:
> 
> +extern char saved_command_line[];

Yes. A number of achitectures use something like

	strlcpy(saved_command_line, cmd_line, sizeof(saved_command_line));

and that "sizeof()" would require the full declaration.

However, I don't see any reason why we couldn't make that sizeof be
COMMAND_LINE_SIZE instead, if somebody is willing to grep and do the
conversion and make sure everybody who now uses it has <asm/setup.h>
included.

Hmm?

		Linus

Re: 2.6.7-bk: asm/setup.h and linux/init.h

[Russell King]

On Sat, Jun 26, 2004 at 09:05:57AM -0700, Linus Torvalds wrote:
> On Sat, 26 Jun 2004, Russell King wrote:
> > Is there a reason why we can't delete asm/setup.h from linux/init.h
> > and change that declaration to:
> > 
> > +extern char saved_command_line[];
> 
> Yes. A number of achitectures use something like
> 
> 	strlcpy(saved_command_line, cmd_line, sizeof(saved_command_line));
> 
> and that "sizeof()" would require the full declaration.
> 
> However, I don't see any reason why we couldn't make that sizeof be
> COMMAND_LINE_SIZE instead, if somebody is willing to grep and do the
> conversion and make sure everybody who now uses it has <asm/setup.h>
> included.

Ok, grepping around for sizeof.*saved_command_line and COMMAND_LINE_SIZE
resulted in this patch:

===== arch/alpha/kernel/setup.c 1.43 vs edited =====
--- 1.43/arch/alpha/kernel/setup.c	Thu Jun 24 09:55:53 2004
+++ edited/arch/alpha/kernel/setup.c	Sat Jun 26 17:35:52 2004
@@ -39,6 +39,7 @@
 #include <linux/reboot.h>
 #endif
 #include <linux/notifier.h>
+#include <asm/setup.h>
 #include <asm/io.h>
 
 extern struct notifier_block *panic_notifier_list;
===== arch/cris/kernel/setup.c 1.15 vs edited =====
--- 1.15/arch/cris/kernel/setup.c	Thu Jun 24 09:55:46 2004
+++ edited/arch/cris/kernel/setup.c	Sat Jun 26 17:36:04 2004
@@ -18,6 +18,8 @@
 #include <linux/seq_file.h>
 #include <linux/tty.h>
 
+#include <asm/setup.h>
+
 /*
  * Setup options
  */
===== arch/h8300/kernel/setup.c 1.8 vs edited =====
--- 1.8/arch/h8300/kernel/setup.c	Thu Jun 24 09:55:46 2004
+++ edited/arch/h8300/kernel/setup.c	Sat Jun 26 17:47:10 2004
@@ -155,8 +155,8 @@
 #endif
 	/* Keep a copy of command line */
 	*cmdline_p = &command_line[0];
-	memcpy(saved_command_line, command_line, sizeof(saved_command_line));
-	saved_command_line[sizeof(saved_command_line)-1] = 0;
+	memcpy(saved_command_line, command_line, COMMAND_LINE_SIZE);
+	saved_command_line[COMMAND_LINE_SIZE-1] = 0;
 
 #ifdef DEBUG
 	if (strlen(*cmdline_p)) 
===== arch/ia64/kernel/setup.c 1.75 vs edited =====
--- 1.75/arch/ia64/kernel/setup.c	Thu Jun 24 09:55:46 2004
+++ edited/arch/ia64/kernel/setup.c	Sat Jun 26 17:47:10 2004
@@ -47,6 +47,7 @@
 #include <asm/sal.h>
 #include <asm/sections.h>
 #include <asm/serial.h>
+#include <asm/setup.h>
 #include <asm/smp.h>
 #include <asm/system.h>
 #include <asm/unistd.h>
@@ -284,7 +285,7 @@
 	ia64_patch_vtop((u64) __start___vtop_patchlist, (u64) __end___vtop_patchlist);
 
 	*cmdline_p = __va(ia64_boot_param->command_line);
-	strlcpy(saved_command_line, *cmdline_p, sizeof(saved_command_line));
+	strlcpy(saved_command_line, *cmdline_p, COMMAND_LINE_SIZE);
 
 	efi_init();
 	io_port_init();
===== arch/m68knommu/kernel/setup.c 1.8 vs edited =====
--- 1.8/arch/m68knommu/kernel/setup.c	Thu Jun 24 09:55:46 2004
+++ edited/arch/m68knommu/kernel/setup.c	Sat Jun 26 17:47:10 2004
@@ -216,8 +216,8 @@
 
 	/* Keep a copy of command line */
 	*cmdline_p = &command_line[0];
-	memcpy(saved_command_line, command_line, sizeof(saved_command_line));
-	saved_command_line[sizeof(saved_command_line)-1] = 0;
+	memcpy(saved_command_line, command_line, COMMAND_LINE_SIZE);
+	saved_command_line[COMMAND_LINE_SIZE-1] = 0;
 
 #ifdef DEBUG
 	if (strlen(*cmdline_p)) 
===== arch/mips/kernel/setup.c 1.16 vs edited =====
--- 1.16/arch/mips/kernel/setup.c	Thu Jun 24 09:55:59 2004
+++ edited/arch/mips/kernel/setup.c	Sat Jun 26 17:47:10 2004
@@ -38,6 +38,7 @@
 #include <asm/bootinfo.h>
 #include <asm/cpu.h>
 #include <asm/sections.h>
+#include <asm/setup.h>
 #include <asm/system.h>
 
 struct cpuinfo_mips cpu_data[NR_CPUS];
@@ -489,7 +490,7 @@
 	do_earlyinitcalls();
 
 	strlcpy(command_line, arcs_cmdline, sizeof(command_line));
-	strlcpy(saved_command_line, command_line, sizeof(saved_command_line));
+	strlcpy(saved_command_line, command_line, COMMAND_LINE_SIZE);
 
 	*cmdline_p = command_line;
 
===== arch/parisc/kernel/setup.c 1.11 vs edited =====
--- 1.11/arch/parisc/kernel/setup.c	Thu Jun 24 09:55:46 2004
+++ edited/arch/parisc/kernel/setup.c	Sat Jun 26 17:36:13 2004
@@ -44,6 +44,7 @@
 #include <asm/machdep.h>	/* for pa7300lc_init() proto */
 #include <asm/pdc_chassis.h>
 #include <asm/io.h>
+#include <asm/setup.h>
 
 char	command_line[COMMAND_LINE_SIZE];
 
===== arch/ppc/kernel/setup.c 1.55 vs edited =====
--- 1.55/arch/ppc/kernel/setup.c	Thu Jun 24 09:55:46 2004
+++ edited/arch/ppc/kernel/setup.c	Sat Jun 26 17:47:10 2004
@@ -676,7 +676,7 @@
 	init_mm.brk = (unsigned long) klimit;
 
 	/* Save unparsed command line copy for /proc/cmdline */
-	strlcpy(saved_command_line, cmd_line, sizeof(saved_command_line));
+	strlcpy(saved_command_line, cmd_line, COMMAND_LINE_SIZE);
 	*cmdline_p = cmd_line;
 
 	/* set up the bootmem stuff with available memory */
===== arch/ppc64/kernel/setup.c 1.59 vs edited =====
--- 1.59/arch/ppc64/kernel/setup.c	Fri May 28 05:02:20 2004
+++ edited/arch/ppc64/kernel/setup.c	Sat Jun 26 17:47:10 2004
@@ -44,6 +44,7 @@
 #include <asm/sections.h>
 #include <asm/btext.h>
 #include <asm/nvram.h>
+#include <asm/setup.h>
 #include <asm/system.h>
 
 extern unsigned long klimit;
@@ -629,7 +630,7 @@
 	init_mm.brk = klimit;
 	
 	/* Save unparsed command line copy for /proc/cmdline */
-	strlcpy(saved_command_line, cmd_line, sizeof(saved_command_line));
+	strlcpy(saved_command_line, cmd_line, COMMAND_LINE_SIZE);
 	*cmdline_p = cmd_line;
 
 	irqstack_early_init();
===== arch/sh/kernel/setup.c 1.19 vs edited =====
--- 1.19/arch/sh/kernel/setup.c	Thu Jun 24 09:56:13 2004
+++ edited/arch/sh/kernel/setup.c	Sat Jun 26 17:36:21 2004
@@ -25,6 +25,7 @@
 #include <asm/io_generic.h>
 #include <asm/sections.h>
 #include <asm/irq.h>
+#include <asm/setup.h>
 
 #ifdef CONFIG_SH_KGDB
 #include <asm/kgdb.h>
===== arch/sparc/kernel/setup.c 1.29 vs edited =====
--- 1.29/arch/sparc/kernel/setup.c	Thu Jun 24 09:55:46 2004
+++ edited/arch/sparc/kernel/setup.c	Sat Jun 26 17:36:31 2004
@@ -47,6 +47,7 @@
 #include <asm/hardirq.h>
 #include <asm/machines.h>
 #include <asm/cpudata.h>
+#include <asm/setup.h>
 
 struct screen_info screen_info = {
 	0, 0,			/* orig-x, orig-y */
===== arch/sparc64/kernel/setup.c 1.53 vs edited =====
--- 1.53/arch/sparc64/kernel/setup.c	Thu Jun 24 09:55:46 2004
+++ edited/arch/sparc64/kernel/setup.c	Sat Jun 26 17:36:40 2004
@@ -47,6 +47,7 @@
 #include <asm/mmu_context.h>
 #include <asm/timer.h>
 #include <asm/sections.h>
+#include <asm/setup.h>
 
 #ifdef CONFIG_IP_PNP
 #include <net/ipconfig.h>
===== arch/v850/kernel/setup.c 1.6 vs edited =====
--- 1.6/arch/v850/kernel/setup.c	Thu Jun 24 09:55:46 2004
+++ edited/arch/v850/kernel/setup.c	Sat Jun 26 17:47:10 2004
@@ -23,6 +23,7 @@
 #include <linux/init.h>
 
 #include <asm/irq.h>
+#include <asm/setup.h>
 
 #include "mach.h"
 
@@ -63,8 +64,8 @@
 {
 	/* Keep a copy of command line */
 	*cmdline = command_line;
-	memcpy (saved_command_line, command_line, sizeof saved_command_line);
-	saved_command_line[sizeof saved_command_line - 1] = '\0';
+	memcpy (saved_command_line, command_line, COMMAND_LINE_SIZE);
+	saved_command_line[COMMAND_LINE_SIZE - 1] = '\0';
 
 	console_verbose ();
 
===== arch/x86_64/kernel/head64.c 1.9 vs edited =====
--- 1.9/arch/x86_64/kernel/head64.c	Thu May 27 17:34:10 2004
+++ edited/arch/x86_64/kernel/head64.c	Sat Jun 26 17:36:55 2004
@@ -16,6 +16,7 @@
 #include <asm/proto.h>
 #include <asm/smp.h>
 #include <asm/bootsetup.h>
+#include <asm/setup.h>
 
 /* Don't add a printk in there. printk relies on the PDA which is not initialized 
    yet. */
===== arch/x86_64/kernel/setup.c 1.43 vs edited =====
--- 1.43/arch/x86_64/kernel/setup.c	Thu Jun 24 09:55:46 2004
+++ edited/arch/x86_64/kernel/setup.c	Sat Jun 26 17:37:03 2004
@@ -55,6 +55,7 @@
 #include <asm/bootsetup.h>
 #include <asm/smp.h>
 #include <asm/proto.h>
+#include <asm/setup.h>
 
 /*
  * Machine setup..
===== include/linux/init.h 1.32 vs edited =====
--- 1.32/include/linux/init.h	Thu Jun 24 09:55:46 2004
+++ edited/include/linux/init.h	Sat Jun 26 12:50:09 2004
@@ -3,7 +3,6 @@
 
 #include <linux/config.h>
 #include <linux/compiler.h>
-#include <asm/setup.h>
 
 /* These macros are used to mark some functions or 
  * initialized data (doesn't apply to uninitialized data)
@@ -69,7 +68,7 @@
 extern initcall_t __security_initcall_start, __security_initcall_end;
 
 /* Defined in init/main.c */
-extern char saved_command_line[COMMAND_LINE_SIZE];
+extern char saved_command_line[];
 #endif
   
 #ifndef MODULE
===== init/main.c 1.148 vs edited =====
--- 1.148/init/main.c	Thu Jun 24 09:55:46 2004
+++ edited/init/main.c	Sat Jun 26 12:51:27 2004
@@ -47,6 +47,7 @@
 
 #include <asm/io.h>
 #include <asm/bugs.h>
+#include <asm/setup.h>
 
 /*
  * This is one of the first .c files built. Error out early

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: 2.6.7-bk: asm/setup.h and linux/init.h

[Paul Jackson]

Could someone speak a few words on why a patch such as this is
desirable?

Apparently, there is more value to avoiding non-essential include's than
I realize.  I'd have preferred doing a memcpy of "sizeof(some_struct)"
over doing it for SOME_STRUCT_SIZE constant, and I'd have thought it
appropriate to include whatever header files were needed to do that.

The simple "wc -l" size of the kernel source goes up a couple of lines,
the code complexity is unchanged, and the generated code is unchanged. 
So obviously none of those factors motivate this change.

-- 
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <pj@sgi.com> 1.650.933.1373

Re: 2.6.7-bk: asm/setup.h and linux/init.h

[Rusty Russell]

On Sun, 2004-06-27 at 02:49, Russell King wrote:
> Ok, grepping around for sizeof.*saved_command_line and COMMAND_LINE_SIZE
> resulted in this patch:

Looks fine to me.  Sorry for the breakage.

Rusty.
-- 
Anyone who quotes me in their signature is an idiot -- Rusty Russell

Re: 2.6.7-bk: asm/setup.h and linux/init.h

[Rusty Russell]

On Sun, 2004-06-27 at 09:24, Paul Jackson wrote:
> Could someone speak a few words on why a patch such as this is
> desirable?

Read Russell's original post; including asm-arm/setup.h from
linux/init.h enters #include hell.  It's purely a practical matter, and
while I agree with you in general on sizeof(), we need the constant
somewhere anyway to declare the array.

Hope that clarifies,
Rusty.
-- 
Anyone who quotes me in their signature is an idiot -- Rusty Russell

Re: 2.6.7-bk: asm/setup.h and linux/init.h

[Paul Jackson]

>  Russell's original post ... enters #include hell ...

Ah - ok.  Thanks.  Make sense.

-- 
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <pj@sgi.com> 1.650.933.1373