diff mbox series

[v8,2/5] x86: Add cmdline_prepare() helper

Message ID b81fa524589ff21002a501f0b4cddf41b53f640f.1668082601.git.baskov@ispras.ru
State New
Headers
Series Parse CONFIG_CMDLINE in compressed kernel |

Commit Message

Evgeniy Baskov Nov. 10, 2022, 1:09 p.m. UTC 
  Command line needs to be combined in both compressed and uncompressed
kernel from built-in and boot command line strings, which requires
non-trivial logic depending on CONFIG_CMDLINE_BOOL and
CONFIG_CMDLINE_OVERRIDE.

Add a helper function to avoid code duplication.

Signed-off-by: Evgeniy Baskov <baskov@ispras.ru>
---
 arch/x86/include/asm/shared/cmdline.h | 35 +++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 arch/x86/include/asm/shared/cmdline.h

Comments

Borislav Petkov Nov. 14, 2022, 2:28 p.m. UTC | #1 
On Thu, Nov 10, 2022 at 04:09:30PM +0300, Evgeniy Baskov wrote:
> Command line needs to be combined in both compressed and uncompressed
> kernel from built-in and boot command line strings, which requires
> non-trivial logic depending on CONFIG_CMDLINE_BOOL and
> CONFIG_CMDLINE_OVERRIDE.
> 
> Add a helper function to avoid code duplication.
> 
> Signed-off-by: Evgeniy Baskov <baskov@ispras.ru>
> ---
>  arch/x86/include/asm/shared/cmdline.h | 35 +++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100644 arch/x86/include/asm/shared/cmdline.h
> 
> diff --git a/arch/x86/include/asm/shared/cmdline.h b/arch/x86/include/asm/shared/cmdline.h
> new file mode 100644
> index 000000000000..01736d66028d
> --- /dev/null
> +++ b/arch/x86/include/asm/shared/cmdline.h
> @@ -0,0 +1,35 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _ASM_X86_SETUP_CMDLINE_H
> +#define _ASM_X86_SETUP_CMDLINE_H
> +
> +#define _SETUP
> +#include <asm/setup.h> /* For COMMAND_LINE_SIZE */
> +#undef _SETUP
> +
> +#include <linux/string.h>
> +
> +#ifdef CONFIG_CMDLINE_BOOL
> +#define BUILTIN_COMMAND_LINE CONFIG_CMDLINE
> +#else
> +#define BUILTIN_COMMAND_LINE ""
> +#endif
> +
> +static inline void cmdline_prepare(char *dst, const char *boot_command_line)
> +{
> +	if (!IS_ENABLED(CONFIG_CMDLINE_BOOL)) {
> +		strscpy(dst, boot_command_line, COMMAND_LINE_SIZE);
> +	} else {
> +		strscpy(dst, BUILTIN_COMMAND_LINE, COMMAND_LINE_SIZE);
> +		/*
> +		 * Append boot loader cmdline to builtin, if it exists
> +		 * and should not be overriden.
> +		 */
> +		if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE) && boot_command_line[0]) {
> +			strlcat(dst, " ", COMMAND_LINE_SIZE);
> +			strlcat(dst, boot_command_line, COMMAND_LINE_SIZE);
> +		}

You keep changing what I'm suggesting and the next patch has a strscpy()
outside of the function.

When I say it should be all concentrated in one function, I really mean
it.

So now it is my turn: I'll do it how I think it should be done and you
can review it.
Borislav Petkov Nov. 15, 2022, 6:58 p.m. UTC | #2 
On Mon, Nov 14, 2022 at 03:28:55PM +0100, Borislav Petkov wrote:
> So now it is my turn: I'll do it how I think it should be done and you
> can review it.

Ok, here are two patches as a reply to this message.

I was able to test them as much as I can in a VM here but I'd need more
details/testing in your configuration with earlyprintk as a builtin
cmdline.

cmdline_prepare() has grown a bit hairy in the end but I've tried hard
to comment what happens there so that it is clear for the future. The
main goal being to concentrate all command line strings processing in
that function and not have it spread around the tree. And yes, there are
more cleanups possible.

In the compressed stage I'm using the cmdline which is in boot_params as
source and destination to basically add only the builtin cmdline.

In kernel proper the boot_command_line comes from generic code and that
is a whole another way of crazy in itself when I look at init/main.c

And as previously stated - the goal is to have everything in one place
and documented as good as possible so that trying to figure out how
command line parsing is done doesn't send you on grepping spree around
the tree.

Suggestions how to simplify this even more are always welcome, ofc.

Thx.
diff mbox series

Patch

diff --git a/arch/x86/include/asm/shared/cmdline.h b/arch/x86/include/asm/shared/cmdline.h
new file mode 100644
index 000000000000..01736d66028d
--- /dev/null
+++ b/arch/x86/include/asm/shared/cmdline.h
@@ -0,0 +1,35 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _ASM_X86_SETUP_CMDLINE_H
+#define _ASM_X86_SETUP_CMDLINE_H
+
+#define _SETUP
+#include <asm/setup.h> /* For COMMAND_LINE_SIZE */
+#undef _SETUP
+
+#include <linux/string.h>
+
+#ifdef CONFIG_CMDLINE_BOOL
+#define BUILTIN_COMMAND_LINE CONFIG_CMDLINE
+#else
+#define BUILTIN_COMMAND_LINE ""
+#endif
+
+static inline void cmdline_prepare(char *dst, const char *boot_command_line)
+{
+	if (!IS_ENABLED(CONFIG_CMDLINE_BOOL)) {
+		strscpy(dst, boot_command_line, COMMAND_LINE_SIZE);
+	} else {
+		strscpy(dst, BUILTIN_COMMAND_LINE, COMMAND_LINE_SIZE);
+		/*
+		 * Append boot loader cmdline to builtin, if it exists
+		 * and should not be overriden.
+		 */
+		if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE) && boot_command_line[0]) {
+			strlcat(dst, " ", COMMAND_LINE_SIZE);
+			strlcat(dst, boot_command_line, COMMAND_LINE_SIZE);
+		}
+	}
+}
+
+#endif /* _ASM_X86_SETUP_CMDLINE_H */