On Tue, Oct 17, 2023 at 12:53:05PM +0200, Piotr Kubik wrote: > Commit <51bb33846ad2> introduced a feature of bootargs
Checkpatch will complain that this should be:
In commit 51bb33846ad2 ("bootm: Support string substitution in
bootargs")
And this is the kind of thing I would fixup on applying if there was no
other feedback. However:
> string substitution and changed a flag used in
> bootm_process_cmdline_env() call to be either true or false.
> With this flag value, condition in bootm_process_cmdline()
> `if (flags & BOOTM_CL_SUBST)` is never true
> and process_subst() is never called.
>
> Signed-off-by: Piotr Kubik <piotr.ku...@iopsys.eu>
> ---
> boot/bootm.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/boot/bootm.c b/boot/bootm.c
> index 8f96a80d42..e96489e549 100644
> --- a/boot/bootm.c
> +++ b/boot/bootm.c
> @@ -778,7 +778,8 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int
> argc,
> if (!ret && (states & BOOTM_STATE_OS_BD_T))
> ret = boot_fn(BOOTM_STATE_OS_BD_T, argc, argv, images);
> if (!ret && (states & BOOTM_STATE_OS_PREP)) {
> - ret = bootm_process_cmdline_env(images->os.os == IH_OS_LINUX);
> + ret = bootm_process_cmdline_env(images->os.os == IH_OS_LINUX ?
> +
> BOOTM_CL_ALL : 0);
This gets hard to read. I would prefer a comment and something like:
int flags = 0;
if (images->os.os == IH_OS_LINUX)
flags = BOOTM_CL_ALL;
ret = bootm_process_cmdline_env(flags);
As the compiler should be just as smart, and that'll be clear about
what's going on. Thanks.
--
Tom
signature.asc
Description: PGP signature
