[PATCH v13 15/24] cli: add modern hush as parser for run_command*()

[Tom Rini]

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 at chromium.org>
> > Signed-off-by: Francis Laniel <francis.laniel at 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!

-- 
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/20231222/2d5c9789/attachment.sig>