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

Piotr Kubik piotr.kubik at iopsys.eu
Wed Oct 18 11:24:17 CEST 2023


Hi Simon, Tom
Thanks for your feedback

> On Tue, 17 Oct 2023 at 10:35, Tom Rini <trini at konsulko.com> wrote:
>>
>> Checkpatch will complain that this should be:
>> In commit 51bb33846ad2 ("bootm: Support string substitution in
>> bootargs")

Actually it did not complain, but I'll fix.

> On 18.10.2023 05:32, Simon Glass wrote:
> Oh wow that is a terrible bug. We have lots of test coverage of
> bootm_process_cmdline_env() and below, but bootm itself is still quite
> spotty.
>
> I wonder if we could add something to run_fit_test to check this. We
> have lots of tests for bootm_process_cmdline_env() but not much of
> bootm itself. It might be possible to add just a few lines there, e.g.
> to set the bootargs to something and check what ends up in bootargs?

That's a good idea, I'll check

>> +		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.

Well, I followed previous way of coding this part and it was:
ret = bootm_process_cmdline_env(images->os.os == IH_OS_LINUX ?
						BOOTM_CL_SILENT : 0);

But sure, I can update to make it more readable. 

Regards,
Piotr


More information about the U-Boot mailing list