[PATCH] bootm: Fix flags used for bootargs string substitution

Tom Rini trini at konsulko.com
Tue Oct 17 18:35:47 CEST 2023


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.kubik at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20231017/ee57ee7f/attachment.sig>


More information about the U-Boot mailing list