Re: [patch 1/22] Add __early_param for all arches

[Tom Rini]

On Wed, Mar 31, 2004 at 02:23:05PM +1000, Rusty Russell wrote:

> On Thu, 2004-03-25 at 10:57, trini@xxxxxxxxxxxxxxxxxxx wrote: 
> > +void __init parse_early_options(char **cmdline_p)
> 
> Please, don't do it this way.
> 
> __setup() has icky semantics which are only used in three places (ide,
> ppc64's eeh, and um's eth).  The string is a prefix and the rest of the
> parameter is handed to the fn as an arg.  Meaning misspellings aren't
> usually caught, and accidental name reuse is hard to catch.
> 
> Secondly, we already have a parser in the kernel.
> 
> I like the idea of cleaning up saved_command_line crap: ideally
> the archs would just assign to a global "command_line" var, and
> anyone wanting to write to it would make their own copies.
> 
> How's this version, instead?  If you agree, I'll produce a merged
> version.

My first concern is can parse_args & co really be run so very early on ?
Also:
> +/* Arch code calls this early on. */
> +void __init parse_early_options(const char *saved_command_line)
> +{
> +	static char __initdata command_line[COMMAND_LINE_SIZE];
> +	strcpy(command_line, saved_command_line);

Really should be:
/* i386 goes right to saved_command_line */
if (*cmdline_p != saved_command_line)
	memcpy(saved_command_line, *cmdline_p, COMMAND_LINE_SIZE);
/* ensure NUL terminated. */
saved_command_line[COMMAND_LINE_SIZE - 1] = '\0';

-- 
Tom Rini
http://gate.crashing.org/~trini/
-
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/