On Fri,  9 Nov 2018 09:34:31 -0800 Daniel Walker <danie...@cisco.com> wrote:

> This code allows architectures to use a generic builtin command line.
> The state of the builtin command line options across architecture is
> diverse. On x86 and mips they have pretty much the same code and the
> code prepends the builtin command line onto the boot loader provided
> one. On powerpc there is only a builtin override and nothing else.
> 
> The code in this commit unifies the mips and x86 code into a generic
> header file under the CONFIG_GENERIC_CMDLINE option. When this
> option is enabled the architecture can call the cmdline_add_builtin()
> to add the builtin command line.

I'm not sure what's happened to this and I haven't seen the other two
patches but...

> [maksym.kok...@globallogic.com: fix cmdline_add_builtin() macro]
> Cc: Daniel Walker <dwal...@fifo99.com>
> Cc: Daniel Walker <danie...@cisco.com>
> Cc: xe-linux-exter...@cisco.com
> Signed-off-by: Daniel Walker <danie...@cisco.com>
> Signed-off-by: Maksym Kokhan <maksym.kok...@globallogic.com>

Two SOB's is nice, but some reviews and acks would be nicer.

> --- /dev/null
> +++ b/include/linux/cmdline.h
> @@ -0,0 +1,79 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_CMDLINE_H
> +#define _LINUX_CMDLINE_H
> +
> +/*
> + *
> + * Copyright (C) 2015. Cisco Systems, Inc.
> + *
> + * Generic Append/Prepend cmdline support.
> + */
> +
> +#if defined(CONFIG_GENERIC_CMDLINE) && defined(CONFIG_CMDLINE_BOOL)
> +
> +#ifndef CONFIG_CMDLINE_OVERRIDE
> +/*
> + * This function will append or prepend a builtin command line to the command
> + * line provided by the bootloader. Kconfig options can be used to alter
> + * the behavior of this builtin command line.
> + * @dest: The destination of the final appended/prepended string
> + * @src: The starting string or NULL if there isn't one.
> + * @tmp: temporary space used for prepending
> + * @length: the maximum length of the strings above.
> + */
> +static inline void
> +_cmdline_add_builtin(char *dest, char *src, char *tmp, unsigned long length)
> +{
> +     if (src != dest && src != NULL) {
> +             strlcpy(dest, " ", length);
> +             strlcat(dest, src, length);
> +     }
> +
> +     strlcat(dest, " ", length);
> +
> +     if (sizeof(CONFIG_CMDLINE_APPEND) > 1)
> +             strlcat(dest, CONFIG_CMDLINE_APPEND, length);
> +
> +     /*
> +      * You need to convert you old style CONFIG_CMDLINE to use

"your"

> +      * the prepend, or append defines. Some architectures use one

"one and"

> +      * some use the other. You need to figure out which ones is

"one"

> +      * right for your situation. I would recommend prepending
> +      * because it's the safest (i.e. CONFIG_CMDLINE_PREPEND).
> +      */
> +     BUILD_BUG_ON(sizeof(CONFIG_CMDLINE) != 1);
> +
> +     if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {
> +             strlcpy(tmp, CONFIG_CMDLINE_PREPEND, length);
> +             strlcat(tmp, " ", length);
> +             strlcat(tmp, dest, length);
> +             strlcpy(dest, tmp, length);
> +     }
> +}

And... holy cow.  Does this monster really need to be inlined?

> +#define cmdline_add_builtin(dest, src, length)                               
>     \
> +{                                                                        \
> +     if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {                           \
> +             static char cmdline_tmp_space[length] __initdata;           \
> +             _cmdline_add_builtin(dest, src, cmdline_tmp_space, length); \
> +     } else {                                                            \
> +             _cmdline_add_builtin(dest, src, NULL, length);              \
> +     }                                                                   \
> +}

And this will generate __initdata storage at each invocation site.  Can
it be redone in real, non-inlined C?

> +#else
> +#define cmdline_add_builtin(dest, src, length)                               
>    \
> +{                                                                       \
> +     strlcpy(dest, CONFIG_CMDLINE_PREPEND " " CONFIG_CMDLINE_APPEND,    \
> +             length);                                                   \
> +}
> +#endif /* !CONFIG_CMDLINE_OVERRIDE */
> +
> +#else
> +#define cmdline_add_builtin(dest, src, length) { \
> +     if (src != NULL)                                                   \
> +             strlcpy(dest, src, length);                                \
> +}
> +#endif /* CONFIG_GENERIC_CMDLINE */
> +
> +
> +#endif /* _LINUX_CMDLINE_H */

Reply via email to