Hi! Le mardi 26 décembre 2023, 10:46:48 CET Simon Glass a écrit : > Hi, > > On Fri, Dec 22, 2023 at 9:23 PM Tom Rini <tr...@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 <s...@chromium.org> > > > > Signed-off-by: Francis Laniel <francis.lan...@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 leads > > > > > > bootflow_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() propagates > > > > > > CMD_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!
You are welcome! > > - Simon Best regards and have a nice end of the year!