Le 09/03/2021 à 22:40, Daniel Walker a écrit :
On Tue, Mar 09, 2021 at 08:56:47AM +0100, Christophe Leroy wrote:

So we are referencing a function that doesn't exist (namely prom_strlcat).
But it works because cmdline_add_builtin_custom() looks like a function but
is in fact an obscure macro that doesn't use prom_strlcat() unless
GENERIC_CMDLINE_NEED_STRLCAT is defined.

IMHO that's awful for readability and code maintenance.

powerpc is a special case, there's no other users like this. The reason is
because of all the difficulty in this prom_init.c code. A lot of the generic
code has similar kind of changes to work across architectures.


I'd suggest the following (sorry if Thunderbird damages whitespaces, you'll get 
the idea anyway)



diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h
new file mode 100644
index 000000000000..30b9eefc802f
--- /dev/null
+++ b/include/linux/cmdline.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_CMDLINE_H
+#define _LINUX_CMDLINE_H
+
+#ifdef CONFIG_GENERIC_CMDLINE
+
+#ifndef cmdline_strlcpy
+#define cmdline_strlcpy strlcpy
+#endif
+#ifndef cmdline_strlcat
+#define cmdline_strlcat strlcat
+#endif
+
+static __always_inline void
+cmdline_add_builtin_custom(char *dest, const char *src, char *tmp, unsigned 
long length)
+{
+       if (WARN_ON(sizeof(CONFIG_CMDLINE_PREPEND) > 1 &&
+                   !IS_ENABLED(CONFIG_CMDLINE_OVERRIDE) &&
+                   !tmp && src == dest))
+               return;
+
+       if (sizeof(CONFIG_CMDLINE_PREPEND) > 1 &&
+           !IS_ENABLED(CONFIG_CMDLINE_OVERRIDE) && src == dest)
+               cmdline_strlcpy(tmp, src, length);
+       else
+               tmp = (char *)src;
+
+       cmdline_strlcpy(dest, CONFIG_CMDLINE_PREPEND " ", length);
+       if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE) && tmp)
+               cmdline_strlcat(dest, tmp, length);
+       cmdline_strlcat(dest, " " CONFIG_CMDLINE_APPEND, length);
+}
+
+#define cmdline_add_builtin(dest, src, length) do {                    \
+       static __init char cmdline_tmp[length];                         \
+                                                                       \
+       cmdline_add_builtin_custom(dest, src, cmdline_tmp, length);     \
+} while (0)
+
+#endif /* CONFIG_GENERIC_CMDLINE */
+
+#endif /* _LINUX_CMDLINE_H */
diff --git a/init/Kconfig b/init/Kconfig
index 22946fe5ded9..aeb134f0703b 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -2035,6 +2035,27 @@ config PROFILING
 config TRACEPOINTS
        bool

+config GENERIC_CMDLINE
+       bool
+
+if GENERIC_CMDLINE
+
+config CMDLINE_BOOL
+       bool "Built-in kernel command line"
+
+config CMDLINE_APPEND
+       string "Built-in kernel command string append" if CMDLINE_BOOL
+       default ""
+
+config CMDLINE_PREPEND
+       string "Built-in kernel command string prepend" if CMDLINE_BOOL
+       default ""
+
+config CMDLINE_OVERRIDE
+       bool "Built-in command line overrides boot loader arguments" if 
CMDLINE_BOOL
+
+endif
+
 endmenu                # General setup

 source "arch/Kconfig"
--

Then on powerpc you do:

diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 2c2f33155317..1649787c3953 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -152,7 +152,7 @@ static struct prom_t __prombss prom;
 static unsigned long __prombss prom_entry;

 static char __prombss of_stdout_device[256];
-static char __prombss prom_scratch[256];
+static char __prombss prom_scratch[COMMAND_LINE_SIZE];

 static unsigned long __prombss dt_header_start;
 static unsigned long __prombss dt_struct_start, dt_struct_end;
@@ -770,6 +770,12 @@ static unsigned long prom_memparse(const char *ptr, const 
char **retptr)
  * Early parsing of the command line passed to the kernel, used for
  * "mem=x" and the options that affect the iommu
  */
+
+#define cmdline_strlcpy prom_strlcpy
+#define cmdline_strlcat prom_strlcat
+
+#include <linux/cmdline.h>
+
 static void __init early_cmdline_parse(void)
 {
        const char *opt;
@@ -780,12 +786,11 @@ static void __init early_cmdline_parse(void)
        prom_cmd_line[0] = 0;
        p = prom_cmd_line;

-       if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && (long)prom.chosen > 0)
+       if ((long)prom.chosen > 0)
                l = prom_getprop(prom.chosen, "bootargs", p, 
COMMAND_LINE_SIZE-1);

-       if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || l <= 0 || p[0] == '\0')
-               prom_strlcat(prom_cmd_line, " " CONFIG_CMDLINE,
-                            sizeof(prom_cmd_line));
+       cmdline_add_builtin_custom(prom_cmd_line, (l > 0 ? p : NULL),
+                                  prom_scratch, sizeof(prom_cmd_line));

        prom_printf("command line: %s\n", prom_cmd_line);

--
2.25.0



Christophe

Reply via email to