Ingo Molnar wrote:
* Tim Bird <[EMAIL PROTECTED]> wrote:

Add support for a built-in command line for x86 architectures. The Kconfig help gives the major rationale for this addition.

i have actually used a local hack quite similar to this to inject boot options into bzImages via randconfig - so i would find this feature rather useful.

a small observation:

+       /* append boot loader cmdline to builtin, unless builtin overrides it */
+       if (builtin_cmdline[0] != '!') {
+               strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE);
+               strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE);
+               strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
+       } else {
+               strlcpy(boot_command_line, &builtin_cmdline[1],
+                       COMMAND_LINE_SIZE);
+       }

the default branch changes existing command lines slightly: it appends a space to them. This could break scripts that rely on the precise contents of /proc/cmdline output. (i have some - they are arguably dodgy)

Best would be to make it really apparent in the code that nothing changes if this config option is not set. Preferably there should be no extra code at all in that case.


I would like to see this:

#ifdef CONFIG_BUILTIN_CMDLINE
# ifdef CONFIG_BUILTIN_CMDLINE_OVERRIDE
        strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
# else
        if (boot_command_line) {
                strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE);
                strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE);
        }
# endif
#endif

        -hpa

                
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to