Hi,
On Fri, Dec 22, 2023 at 9:23 PM Tom Rini trini@konsulko.com wrote:
On Fri, Dec 22, 2023 at 10:10:42PM +0100, Francis Laniel wrote:
Hi!
Le vendredi 22 décembre 2023, 22:02:35 CET Francis Laniel a écrit :
Enables using, in code, modern hush as parser for run_command function family. It also enables the command run to be used by CLI user of modern hush.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Francis Laniel francis.laniel@amarulasolutions.com
[snip]
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c index a9b555c779..104f49deef 100644 --- a/test/boot/bootflow.c +++ b/test/boot/bootflow.c @@ -710,7 +710,21 @@ static int bootflow_scan_menu_boot(struct unit_test_state *uts) ut_assert_skip_to_line("(2 bootflows, 2 valid)");
ut_assert_nextline("Selected: Armbian");
- ut_assert_skip_to_line("Boot failed (err=-14)");
- if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
/** With old hush, despite booti failing to boot, i.e. returning* CMD_RET_FAILURE, run_command() returns 0 which leadsbootflow_boot(),
as + * we are using bootmeth_script here, to return -EFAULT.
*/ut_assert_skip_to_line("Boot failed (err=-14)");- } else if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
/** While with modern one, run_command() propagatesCMD_RET_FAILURE
returned + * by booti, so we get 1 here.
*/ut_assert_skip_to_line("Boot failed (err=1)");- }
I would like to give a bit of context here. With the following patch: diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py index c169c835e3..cc5adda0a3 100644 --- a/test/py/tests/test_ut.py +++ b/test/py/tests/test_ut.py @@ -173,7 +173,7 @@ else fi fi booti ${kernel_addr_r} ${ramdisk_addr_r} ${fdt_addr_r}
+echo $? # Recompile with: # mkimage -C none -A arm -T script -d /boot/boot.cmd /boot/boot.scr ''' % (mmc_dev) We can easily see that booti is failing while running the test: $ ./test/py/test.py -o log_cli=true -s --build -v -k 'test_ut[ut_bootstd_bootflow_scan_menu_boot' ... Aborting! Failed to load '/boot/dtb/rockchip/overlay/-fixup.scr' 1
The problem with old hush, is that the 1 returned here, which corresponds to CMD_RET_FAILURE, is not propagated as the return value of run_command(). So, this lead the -EFAULT branch here to be taken: int bootflow_boot(struct bootflow *bflow) { /* ... */
ret = bootmeth_boot(bflow->method, bflow); if (ret) return log_msg_ret("boot", ret); /* * internal error, should not get here since we should have booted * something or returned an error */ return log_msg_ret("end", -EFAULT);}
With modern hush, CMD_RET_FAILURE is propagated as the return value of run_command(). As a consequence, we return with log_mst_ret("boot", 1), which leaded to this test to fail. The above modification consists in adapting the expected output to the current shell flavor. I think this is the good thing to do, as I find modern hush behavior better than the old one, i.e. it propagates CMD_RET_FAILURE as return of run_command().
Oh very nice, thanks for digging in to this and explaining!
Yes, thank you from me too!
- Simon