Merge patch series "Modernize U-Boot shell"

Francis Laniel <[email protected]> says:

During 2021 summer, Sean Anderson wrote a contribution to add a new shell, based
on LIL, to U-Boot [1, 2].
While one of the goals of this contribution was to address the fact actual
U-Boot shell, which is based on Busybox hush, is old there was a discussion
about adding a new shell versus updating the actual one [3, 4].

So, in this series, with Harald Seiler, we updated the actual U-Boot shell to
reflect what is currently in Busybox source code.
Basically, this contribution is about taking a snapshot of Busybox shell/hush.c
file (as it exists in commit 37460f5da) and adapt it to suit U-Boot needs.

This contribution was written to be as backward-compatible as possible to avoid
breaking the existing.
So, the modern hush flavor offers the same as the actual, that is to say:
1. Variable expansion.
2. Instruction lists (;, && and ||).
3. If, then and else.
4. Loops (for, while and until).
No new features offered by Busybox hush were implemented (e.g. functions).

It is possible to change the parser at runtime using the "cli" command:
=> cli print
old
=> cli set modern
=> cli print
modern
=> cli set old
The default parser is the old one.
Note that to use both parser, you would need to set both
CONFIG_HUSH_MODERN_PARSER and CONFIG_HUSH_OLD_PARSER.

In terms of testing, new unit tests were added to ut to ensure the new behavior
is the same as the old one and it does not add regression.
Nonetheless, if old behavior was buggy and fixed upstream, the fix is then added
to U-Boot [5].
In sandbox, all of these tests pass smoothly:
=> printenv board
board=sandbox
=> ut hush
Running 20 hush tests
...
Failures: 0
=> cli set modern
=> ut hush
Running 20 hush tests
...
Failures: 0

Thanks to the effort of Harald Seiler, I was successful booting a board:
=> printenv fdtfile
fdtfile=amlogic/meson-gxl-s905x-libretech-cc.dtb
=> cli get
old
=> boot
...
root@lepotato:~#
root@lepotato:~# reboot
...
=> cli set modern
=> cli get
modern
=> printenv fdtfile
fdtfile=amlogic/meson-gxl-s905x-libretech-cc.dtb
=> boot
...
root@lepotato:~#

This contribution indeed adds a lot of code and there were concern about its
size [6, 7].
With regard to the amount of code added, the cli_hush_upstream.c is 13030 lines
long but it seems a smaller subset is really used:
gcc -D__U_BOOT__ -E common/cli_hush_upstream.c | wc -l
2870
Despite this, it is better to still have the whole upstream code for the sake of
easing maintenance.
With regard to memory size, I conducted some experiments for version 8 of this
series and for a subset of arm64 boards and found the worst case to be 4K [8].
Tom Rini conducted more research on this and also found the increase to be
acceptable [9].

If you want to review it - your review will really be appreciated - here are
some information regarding the commits:
* commits marked as "test:" deal with unit tests.
* commit "cli: Add Busybox upstream hush.c file." copies Busybox shell/hush.c
into U-Boot tree, this explain why this commit contains around 12000 additions.
* commit "cli: Port Busybox 2021 hush to U-Boot." modifies previously added file
to permit us to use this as new shell.
The really good idea of #include'ing Busybox code into a wrapper file to define
some particular functions while minimizing modifications to upstream code comes
from Harald Seiler.
* commit "cmd: Add new parser command" adds a new command which permits
selecting parser at runtime.
I am not really satisfied with the fact it calls cli_init() and cli_loop() each
time the parser is set, so your reviews would be welcomed.
* Other commits focus on enabling features we need (e.g. if).

11 files changed, 830 insertions, 199 deletions

diff --git a/test/Makefile b/test/Makefile
index 6b8a1506f54..9aeef02f9ee 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -17,6 +17,9 @@ obj-$(CONFIG_FUZZ) += fuzz/
 ifndef CONFIG_SANDBOX_VPL
 obj-$(CONFIG_UNIT_TEST) += lib/
 endif
+ifneq ($(CONFIG_HUSH_PARSER),)
+obj-$(CONFIG_$(SPL_)CMDLINE) += hush/
+endif
 obj-$(CONFIG_$(SPL_)CMDLINE) += print_ut.o
 obj-$(CONFIG_$(SPL_)CMDLINE) += str_ut.o
 obj-$(CONFIG_UT_TIME) += time_ut.o
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index a9b555c7794..104f49deef2 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)");
+	}
 	ut_assertnonnull(std->cur_bootflow);
 	ut_assert_console_end();
 
diff --git a/test/cmd_ut.c b/test/cmd_ut.c
index 1b934b23295..0677ce0cd17 100644
--- a/test/cmd_ut.c
+++ b/test/cmd_ut.c
@@ -121,6 +121,9 @@ static struct cmd_tbl cmd_ut_sub[] = {
 #ifdef CONFIG_CMD_ADDRMAP
 	U_BOOT_CMD_MKENT(addrmap, CONFIG_SYS_MAXARGS, 1, do_ut_addrmap, "", ""),
 #endif
+#if CONFIG_IS_ENABLED(HUSH_PARSER)
+	U_BOOT_CMD_MKENT(hush, CONFIG_SYS_MAXARGS, 1, do_ut_hush, "", ""),
+#endif
 #ifdef CONFIG_CMD_LOADM
 	U_BOOT_CMD_MKENT(loadm, CONFIG_SYS_MAXARGS, 1, do_ut_loadm, "", ""),
 #endif
@@ -216,6 +219,9 @@ U_BOOT_LONGHELP(ut,
 #ifdef CONFIG_CONSOLE_TRUETYPE
 	"\nfont - font command"
 #endif
+#if CONFIG_IS_ENABLED(HUSH_PARSER)
+	"\nhush - Test hush behavior"
+#endif
 #ifdef CONFIG_CMD_LOADM
 	"\nloadm - loadm command parameters and loading memory blob"
 #endif
diff --git a/test/hush/Makefile b/test/hush/Makefile
new file mode 100644
index 00000000000..a2d98815e50
--- /dev/null
+++ b/test/hush/Makefile
@@ -0,0 +1,10 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# (C) Copyright 2021
+# Francis Laniel, Amarula Solutions, [email protected]
+
+obj-y += cmd_ut_hush.o
+obj-y += if.o
+obj-y += dollar.o
+obj-y += list.o
+obj-y += loop.o
diff --git a/test/hush/cmd_ut_hush.c b/test/hush/cmd_ut_hush.c
new file mode 100644
index 00000000000..abad44f3216
--- /dev/null
+++ b/test/hush/cmd_ut_hush.c
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * (C) Copyright 2021
+ * Francis Laniel, Amarula Solutions, [email protected]
+ */
+
+#include <command.h>
+#include <test/hush.h>
+#include <test/suites.h>
+#include <test/ut.h>
+
+int do_ut_hush(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
+{
+	struct unit_test *tests = UNIT_TEST_SUITE_START(hush_test);
+	const int n_ents = UNIT_TEST_SUITE_COUNT(hush_test);
+
+	return cmd_ut_category("hush", "hush_test_",
+			       tests, n_ents, argc, argv);
+}
diff --git a/test/hush/dollar.c b/test/hush/dollar.c
new file mode 100644
index 00000000000..4caa07c192a
--- /dev/null
+++ b/test/hush/dollar.c
@@ -0,0 +1,225 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * (C) Copyright 2021
+ * Francis Laniel, Amarula Solutions, [email protected]
+ */
+
+#include <command.h>
+#include <env_attr.h>
+#include <test/hush.h>
+#include <test/ut.h>
+#include <asm/global_data.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static int hush_test_simple_dollar(struct unit_test_state *uts)
+{
+	console_record_reset_enable();
+	ut_assertok(run_command("echo $dollar_foo", 0));
+	ut_assert_nextline_empty();
+	ut_assert_console_end();
+
+	ut_assertok(run_command("echo ${dollar_foo}", 0));
+	ut_assert_nextline_empty();
+	ut_assert_console_end();
+
+	ut_assertok(run_command("dollar_foo=bar", 0));
+
+	ut_assertok(run_command("echo $dollar_foo", 0));
+	ut_assert_nextline("bar");
+	ut_assert_console_end();
+
+	ut_assertok(run_command("echo ${dollar_foo}", 0));
+	ut_assert_nextline("bar");
+	ut_assert_console_end();
+
+	ut_assertok(run_command("dollar_foo=\\$bar", 0));
+
+	ut_assertok(run_command("echo $dollar_foo", 0));
+	ut_assert_nextline("$bar");
+	ut_assert_console_end();
+
+	ut_assertok(run_command("dollar_foo='$bar'", 0));
+
+	ut_assertok(run_command("echo $dollar_foo", 0));
+	ut_assert_nextline("$bar");
+	ut_assert_console_end();
+
+	ut_asserteq(1, run_command("dollar_foo=bar quux", 0));
+	/* Next line contains error message */
+	ut_assert_skipline();
+	ut_assert_console_end();
+
+	ut_asserteq(1, run_command("dollar_foo='bar quux", 0));
+	/* Next line contains error message */
+	ut_assert_skipline();
+
+	if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+		/*
+		 * For some strange reasons, the console is not empty after
+		 * running above command.
+		 * So, we reset it to not have side effects for other tests.
+		 */
+		console_record_reset_enable();
+	} else if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+		ut_assert_console_end();
+	}
+
+	ut_asserteq(1, run_command("dollar_foo=bar quux\"", 0));
+	/* Two next lines contain error message */
+	ut_assert_skipline();
+	ut_assert_skipline();
+
+	if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+		/* See above comments. */
+		console_record_reset_enable();
+	} else if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+		ut_assert_console_end();
+	}
+
+	ut_assertok(run_command("dollar_foo='bar \"quux'", 0));
+
+	ut_assertok(run_command("echo $dollar_foo", 0));
+	/*
+	 * This one is buggy.
+	 * ut_assert_nextline("bar \"quux");
+	 * ut_assert_console_end();
+	 *
+	 * So, let's reset output:
+	 */
+	console_record_reset_enable();
+
+	if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+		/*
+		 * Old parser returns an error because it waits for closing
+		 * '\'', but this behavior is wrong as the '\'' is surrounded by
+		 * '"', so no need to wait for a closing one.
+		 */
+		ut_assertok(run_command("dollar_foo=\"bar 'quux\"", 0));
+
+		ut_assertok(run_command("echo $dollar_foo", 0));
+		ut_assert_nextline("bar 'quux");
+		ut_assert_console_end();
+	} else if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+		ut_asserteq(1, run_command("dollar_foo=\"bar 'quux\"", 0));
+		/* Next line contains error message */
+		ut_assert_skipline();
+		ut_assert_console_end();
+	}
+
+	ut_assertok(run_command("dollar_foo='bar quux'", 0));
+	ut_assertok(run_command("echo $dollar_foo", 0));
+	ut_assert_nextline("bar quux");
+	ut_assert_console_end();
+
+	if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+		/* Reset local variable. */
+		ut_assertok(run_command("dollar_foo=", 0));
+	} else if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+		puts("Beware: this test set local variable dollar_foo and it cannot be unset!");
+	}
+
+	return 0;
+}
+HUSH_TEST(hush_test_simple_dollar, 0);
+
+static int hush_test_env_dollar(struct unit_test_state *uts)
+{
+	env_set("env_foo", "bar");
+	console_record_reset_enable();
+
+	ut_assertok(run_command("echo $env_foo", 0));
+	ut_assert_nextline("bar");
+	ut_assert_console_end();
+
+	ut_assertok(run_command("echo ${env_foo}", 0));
+	ut_assert_nextline("bar");
+	ut_assert_console_end();
+
+	/* Environment variables have priority over local variable */
+	ut_assertok(run_command("env_foo=quux", 0));
+	ut_assertok(run_command("echo ${env_foo}", 0));
+	ut_assert_nextline("bar");
+	ut_assert_console_end();
+
+	/* Clean up setting the variable */
+	env_set("env_foo", NULL);
+
+	if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+		/* Reset local variable. */
+		ut_assertok(run_command("env_foo=", 0));
+	} else if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+		puts("Beware: this test set local variable env_foo and it cannot be unset!");
+	}
+
+	return 0;
+}
+HUSH_TEST(hush_test_env_dollar, 0);
+
+static int hush_test_command_dollar(struct unit_test_state *uts)
+{
+	console_record_reset_enable();
+
+	ut_assertok(run_command("dollar_bar=\"echo bar\"", 0));
+
+	ut_assertok(run_command("$dollar_bar", 0));
+	ut_assert_nextline("bar");
+	ut_assert_console_end();
+
+	ut_assertok(run_command("${dollar_bar}", 0));
+	ut_assert_nextline("bar");
+	ut_assert_console_end();
+
+	ut_assertok(run_command("dollar_bar=\"echo\nbar\"", 0));
+
+	ut_assertok(run_command("$dollar_bar", 0));
+	ut_assert_nextline("bar");
+	ut_assert_console_end();
+
+	ut_assertok(run_command("dollar_bar='echo bar\n'", 0));
+
+	ut_assertok(run_command("$dollar_bar", 0));
+	ut_assert_nextline("bar");
+	ut_assert_console_end();
+
+	ut_assertok(run_command("dollar_bar='echo bar\\n'", 0));
+
+	ut_assertok(run_command("$dollar_bar", 0));
+
+	if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+		/*
+		 * This difference seems to come from a bug solved in Busybox
+		 * hush.
+		 * Behavior of hush 2021 is coherent with bash and other shells.
+		 */
+		ut_assert_nextline("bar\\n");
+	} else if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+		ut_assert_nextline("barn");
+	}
+
+	ut_assert_console_end();
+
+	ut_assertok(run_command("dollar_bar='echo $bar'", 0));
+
+	ut_assertok(run_command("$dollar_bar", 0));
+	ut_assert_nextline("$bar");
+	ut_assert_console_end();
+
+	ut_assertok(run_command("dollar_quux=quux", 0));
+	ut_assertok(run_command("dollar_bar=\"echo $dollar_quux\"", 0));
+
+	ut_assertok(run_command("$dollar_bar", 0));
+	ut_assert_nextline("quux");
+	ut_assert_console_end();
+
+	if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+		/* Reset local variables. */
+		ut_assertok(run_command("dollar_bar=", 0));
+		ut_assertok(run_command("dollar_quux=", 0));
+	} else if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+		puts("Beware: this test sets local variable dollar_bar and dollar_quux and they cannot be unset!");
+	}
+
+	return 0;
+}
+HUSH_TEST(hush_test_command_dollar, 0);
diff --git a/test/hush/if.c b/test/hush/if.c
new file mode 100644
index 00000000000..8939b7a6c86
--- /dev/null
+++ b/test/hush/if.c
@@ -0,0 +1,316 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * (C) Copyright 2021
+ * Francis Laniel, Amarula Solutions, [email protected]
+ */
+
+#include <command.h>
+#include <env_attr.h>
+#include <vsprintf.h>
+#include <test/hush.h>
+#include <test/ut.h>
+
+/*
+ * All tests will execute the following:
+ * if condition_to_test; then
+ *   true
+ * else
+ *   false
+ * fi
+ * If condition is true, command returns 1, 0 otherwise.
+ */
+const char *if_format = "if %s; then true; else false; fi";
+
+static int hush_test_if_base(struct unit_test_state *uts)
+{
+	char if_formatted[128];
+
+	sprintf(if_formatted, if_format, "true");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "false");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	return 0;
+}
+HUSH_TEST(hush_test_if_base, 0);
+
+static int hush_test_if_basic_operators(struct unit_test_state *uts)
+{
+	char if_formatted[128];
+
+	sprintf(if_formatted, if_format, "test aaa = aaa");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test aaa = bbb");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test aaa != bbb");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test aaa != aaa");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test aaa < bbb");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test bbb < aaa");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test bbb > aaa");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test aaa > bbb");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 123 -eq 123");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 123 -eq 456");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 123 -ne 456");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 123 -ne 123");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 123 -lt 456");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 123 -lt 123");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 456 -lt 123");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 123 -le 456");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 123 -le 123");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 456 -le 123");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 456 -gt 123");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 123 -gt 123");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 123 -gt 456");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 456 -ge 123");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 123 -ge 123");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 123 -ge 456");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	return 0;
+}
+HUSH_TEST(hush_test_if_basic_operators, 0);
+
+static int hush_test_if_octal(struct unit_test_state *uts)
+{
+	char if_formatted[128];
+
+	sprintf(if_formatted, if_format, "test 010 -eq 010");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 010 -eq 011");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 010 -ne 011");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 010 -ne 010");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	return 0;
+}
+HUSH_TEST(hush_test_if_octal, 0);
+
+static int hush_test_if_hexadecimal(struct unit_test_state *uts)
+{
+	char if_formatted[128];
+
+	sprintf(if_formatted, if_format, "test 0x2000000 -gt 0x2000001");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 0x2000000 -gt 0x2000000");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 0x2000000 -gt 0x1ffffff");
+	ut_assertok(run_command(if_formatted, 0));
+
+	return 0;
+}
+HUSH_TEST(hush_test_if_hexadecimal, 0);
+
+static int hush_test_if_mixed(struct unit_test_state *uts)
+{
+	char if_formatted[128];
+
+	sprintf(if_formatted, if_format, "test 010 -eq 10");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 010 -ne 10");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 0xa -eq 10");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 0xa -eq 012");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 2000000 -gt 0x1ffffff");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 0x2000000 -gt 1ffffff");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 0x2000000 -lt 1ffffff");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 0x2000000 -eq 2000000");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 0x2000000 -ne 2000000");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test -z \"\"");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test -z \"aaa\"");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test -n \"aaa\"");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test -n \"\"");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	return 0;
+}
+HUSH_TEST(hush_test_if_mixed, 0);
+
+static int hush_test_if_inverted(struct unit_test_state *uts)
+{
+	char if_formatted[128];
+
+	sprintf(if_formatted, if_format, "test ! aaa = aaa");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test ! aaa = bbb");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test ! ! aaa = aaa");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test ! ! aaa = bbb");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	return 0;
+}
+HUSH_TEST(hush_test_if_inverted, 0);
+
+static int hush_test_if_binary(struct unit_test_state *uts)
+{
+	char if_formatted[128];
+
+	sprintf(if_formatted, if_format, "test aaa != aaa -o bbb != bbb");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test aaa != aaa -o bbb = bbb");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test aaa = aaa -o bbb != bbb");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test aaa = aaa -o bbb = bbb");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test aaa != aaa -a bbb != bbb");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test aaa != aaa -a bbb = bbb");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test aaa = aaa -a bbb != bbb");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test aaa = aaa -a bbb = bbb");
+	ut_assertok(run_command(if_formatted, 0));
+
+	return 0;
+}
+HUSH_TEST(hush_test_if_binary, 0);
+
+static int hush_test_if_inverted_binary(struct unit_test_state *uts)
+{
+	char if_formatted[128];
+
+	sprintf(if_formatted, if_format, "test ! aaa != aaa -o ! bbb != bbb");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test ! aaa != aaa -o ! bbb = bbb");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test ! aaa = aaa -o ! bbb != bbb");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test ! aaa = aaa -o ! bbb = bbb");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format,
+		"test ! ! aaa != aaa -o ! ! bbb != bbb");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format,
+		"test ! ! aaa != aaa -o ! ! bbb = bbb");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format,
+		"test ! ! aaa = aaa -o ! ! bbb != bbb");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test ! ! aaa = aaa -o ! ! bbb = bbb");
+	ut_assertok(run_command(if_formatted, 0));
+
+	return 0;
+}
+HUSH_TEST(hush_test_if_inverted_binary, 0);
+
+static int hush_test_if_z_operator(struct unit_test_state *uts)
+{
+	char if_formatted[128];
+
+	/* Deal with environment variable used during test. */
+	env_set("ut_var_nonexistent", NULL);
+	env_set("ut_var_exists", "1");
+	env_set("ut_var_unset", "1");
+
+	sprintf(if_formatted, if_format, "test -z \"$ut_var_nonexistent\"");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test -z \"$ut_var_exists\"");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test -z \"$ut_var_unset\"");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	env_set("ut_var_unset", NULL);
+	sprintf(if_formatted, if_format, "test -z \"$ut_var_unset\"");
+	ut_assertok(run_command(if_formatted, 0));
+
+	/* Clear the set environment variable. */
+	env_set("ut_var_exists", NULL);
+
+	return 0;
+}
+HUSH_TEST(hush_test_if_z_operator, 0);
diff --git a/test/hush/list.c b/test/hush/list.c
new file mode 100644
index 00000000000..210823db2f5
--- /dev/null
+++ b/test/hush/list.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * (C) Copyright 2021
+ * Francis Laniel, Amarula Solutions, [email protected]
+ */
+
+#include <command.h>
+#include <env_attr.h>
+#include <test/hush.h>
+#include <test/ut.h>
+#include <asm/global_data.h>
+
+static int hush_test_semicolon(struct unit_test_state *uts)
+{
+	/* A; B = B truth table. */
+	ut_asserteq(1, run_command("false; false", 0));
+	ut_assertok(run_command("false; true", 0));
+	ut_assertok(run_command("true; true", 0));
+	ut_asserteq(1, run_command("true; false", 0));
+
+	return 0;
+}
+HUSH_TEST(hush_test_semicolon, 0);
+
+static int hush_test_and(struct unit_test_state *uts)
+{
+	/* A && B truth table. */
+	ut_asserteq(1, run_command("false && false", 0));
+	ut_asserteq(1, run_command("false && true", 0));
+	ut_assertok(run_command("true && true", 0));
+	ut_asserteq(1, run_command("true && false", 0));
+
+	return 0;
+}
+HUSH_TEST(hush_test_and, 0);
+
+static int hush_test_or(struct unit_test_state *uts)
+{
+	/* A || B truth table. */
+	ut_asserteq(1, run_command("false || false", 0));
+	ut_assertok(run_command("false || true", 0));
+	ut_assertok(run_command("true || true", 0));
+	ut_assertok(run_command("true || false", 0));
+
+	return 0;
+}
+HUSH_TEST(hush_test_or, 0);
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static int hush_test_and_or(struct unit_test_state *uts)
+{
+	/* A && B || C truth table. */
+	ut_asserteq(1, run_command("false && false || false", 0));
+
+	if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+		ut_asserteq(1, run_command("false && false || true", 0));
+	} else if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+		/*
+		 * This difference seems to come from a bug solved in Busybox
+		 * hush.
+		 *
+		 * Indeed, the following expression can be seen like this:
+		 * (false && false) || true
+		 * So, (false && false) returns 1, the second false is not
+		 * executed, and true is executed because of ||.
+		 */
+		ut_assertok(run_command("false && false || true", 0));
+	}
+
+	if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+		ut_asserteq(1, run_command("false && true || true", 0));
+	} else if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+		/*
+		 * This difference seems to come from a bug solved in Busybox
+		 * hush.
+		 *
+		 * Indeed, the following expression can be seen like this:
+		 * (false && true) || true
+		 * So, (false && true) returns 1, the true is not executed, and
+		 * true is executed because of ||.
+		 */
+		ut_assertok(run_command("false && true || true", 0));
+	}
+
+	ut_asserteq(1, run_command("false && true || false", 0));
+	ut_assertok(run_command("true && true || false", 0));
+	ut_asserteq(1, run_command("true && false || false", 0));
+	ut_assertok(run_command("true && false || true", 0));
+	ut_assertok(run_command("true && true || true", 0));
+
+	return 0;
+}
+HUSH_TEST(hush_test_and_or, 0);
+
+static int hush_test_or_and(struct unit_test_state *uts)
+{
+	/* A || B && C truth table. */
+	ut_asserteq(1, run_command("false || false && false", 0));
+	ut_asserteq(1, run_command("false || false && true", 0));
+	ut_assertok(run_command("false || true && true", 0));
+	ut_asserteq(1, run_command("false || true && false", 0));
+
+	if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+		ut_assertok(run_command("true || true && false", 0));
+	} else if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+		/*
+		 * This difference seems to come from a bug solved in Busybox
+		 * hush.
+		 *
+		 * Indeed, the following expression can be seen like this:
+		 * (true || true) && false
+		 * So, (true || true) returns 0, the second true is not
+		 * executed, and then false is executed because of &&.
+		 */
+		ut_asserteq(1, run_command("true || true && false", 0));
+	}
+
+	if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+		ut_assertok(run_command("true || false && false", 0));
+	} else if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+		/*
+		 * This difference seems to come from a bug solved in Busybox
+		 * hush.
+		 *
+		 * Indeed, the following expression can be seen like this:
+		 * (true || false) && false
+		 * So, (true || false) returns 0, the false is not executed, and
+		 * then false is executed because of &&.
+		 */
+		ut_asserteq(1, run_command("true || false && false", 0));
+	}
+
+	ut_assertok(run_command("true || false && true", 0));
+	ut_assertok(run_command("true || true && true", 0));
+
+	return 0;
+}
+HUSH_TEST(hush_test_or_and, 0);
diff --git a/test/hush/loop.c b/test/hush/loop.c
new file mode 100644
index 00000000000..d734abf136d
--- /dev/null
+++ b/test/hush/loop.c
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * (C) Copyright 2021
+ * Francis Laniel, Amarula Solutions, [email protected]
+ */
+
+#include <command.h>
+#include <env_attr.h>
+#include <test/hush.h>
+#include <test/ut.h>
+#include <asm/global_data.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static int hush_test_for(struct unit_test_state *uts)
+{
+	console_record_reset_enable();
+
+	ut_assertok(run_command("for loop_i in foo bar quux quux; do echo $loop_i; done", 0));
+	ut_assert_nextline("foo");
+	ut_assert_nextline("bar");
+	ut_assert_nextline("quux");
+	ut_assert_nextline("quux");
+	ut_assert_console_end();
+
+	if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+		/* Reset local variable. */
+		ut_assertok(run_command("loop_i=", 0));
+	} else if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+		puts("Beware: this test set local variable loop_i and it cannot be unset!");
+	}
+
+	return 0;
+}
+HUSH_TEST(hush_test_for, 0);
+
+static int hush_test_while(struct unit_test_state *uts)
+{
+	console_record_reset_enable();
+
+	if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+		/*
+		 * Hush 2021 always returns 0 from while loop...
+		 * You can see code snippet near this line to have a better
+		 * understanding:
+		 * debug_printf_exec(": while expr is false: breaking (exitcode:EXIT_SUCCESS)\n");
+		 */
+		ut_assertok(run_command("while test -z \"$loop_foo\"; do echo bar; loop_foo=quux; done", 0));
+	} else if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+		/*
+		 * Exit status is that of test, so 1 since test is false to quit
+		 * the loop.
+		 */
+		ut_asserteq(1, run_command("while test -z \"$loop_foo\"; do echo bar; loop_foo=quux; done", 0));
+	}
+	ut_assert_nextline("bar");
+	ut_assert_console_end();
+
+	if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+		/* Reset local variable. */
+		ut_assertok(run_command("loop_foo=", 0));
+	} else if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+		puts("Beware: this test set local variable loop_foo and it cannot be unset!");
+	}
+
+	return 0;
+}
+HUSH_TEST(hush_test_while, 0);
+
+static int hush_test_until(struct unit_test_state *uts)
+{
+	console_record_reset_enable();
+	env_set("loop_bar", "bar");
+
+	/*
+	 * WARNING We have to use environment variable because it is not possible
+	 * resetting local variable.
+	 */
+	ut_assertok(run_command("until test -z \"$loop_bar\"; do echo quux; setenv loop_bar; done", 0));
+	ut_assert_nextline("quux");
+	ut_assert_console_end();
+
+	/*
+	 * Loop normally resets foo environment variable, but we reset it here in
+	 * case the test failed.
+	 */
+	env_set("loop_bar", NULL);
+	return 0;
+}
+HUSH_TEST(hush_test_until, 0);
diff --git a/test/py/tests/test_hush_if_test.py b/test/py/tests/test_hush_if_test.py
deleted file mode 100644
index 3b4b6fcaf40..00000000000
--- a/test/py/tests/test_hush_if_test.py
+++ /dev/null
@@ -1,197 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-# Copyright (c) 2015-2016, NVIDIA CORPORATION. All rights reserved.
-
-# Test operation of the "if" shell command.
-
-import os
-import os.path
-import pytest
-
-# TODO: These tests should be converted to a C test.
-# For more information please take a look at the thread
-# https://lists.denx.de/pipermail/u-boot/2019-October/388732.html
-
-pytestmark = pytest.mark.buildconfigspec('hush_parser')
-
-# The list of "if test" conditions to test.
-subtests = (
-    # Base if functionality.
-
-    ('true', True),
-    ('false', False),
-
-    # Basic operators.
-
-    ('test aaa = aaa', True),
-    ('test aaa = bbb', False),
-
-    ('test aaa != bbb', True),
-    ('test aaa != aaa', False),
-
-    ('test aaa < bbb', True),
-    ('test bbb < aaa', False),
-
-    ('test bbb > aaa', True),
-    ('test aaa > bbb', False),
-
-    ('test 123 -eq 123', True),
-    ('test 123 -eq 456', False),
-
-    ('test 123 -ne 456', True),
-    ('test 123 -ne 123', False),
-
-    ('test 123 -lt 456', True),
-    ('test 123 -lt 123', False),
-    ('test 456 -lt 123', False),
-
-    ('test 123 -le 456', True),
-    ('test 123 -le 123', True),
-    ('test 456 -le 123', False),
-
-    ('test 456 -gt 123', True),
-    ('test 123 -gt 123', False),
-    ('test 123 -gt 456', False),
-
-    ('test 456 -ge 123', True),
-    ('test 123 -ge 123', True),
-    ('test 123 -ge 456', False),
-
-    # Octal tests
-
-    ('test 010 -eq 010', True),
-    ('test 010 -eq 011', False),
-
-    ('test 010 -ne 011', True),
-    ('test 010 -ne 010', False),
-
-    # Hexadecimal tests
-
-    ('test 0x2000000 -gt 0x2000001', False),
-    ('test 0x2000000 -gt 0x2000000', False),
-    ('test 0x2000000 -gt 0x1ffffff', True),
-
-    # Mixed tests
-
-    ('test 010 -eq 10', False),
-    ('test 010 -ne 10', True),
-    ('test 0xa -eq 10', True),
-    ('test 0xa -eq 012', True),
-
-    ('test 2000000 -gt 0x1ffffff', False),
-    ('test 0x2000000 -gt 1ffffff', True),
-    ('test 0x2000000 -lt 1ffffff', False),
-    ('test 0x2000000 -eq 2000000', False),
-    ('test 0x2000000 -ne 2000000', True),
-
-    ('test -z ""', True),
-    ('test -z "aaa"', False),
-
-    ('test -n "aaa"', True),
-    ('test -n ""', False),
-
-    # Inversion of simple tests.
-
-    ('test ! aaa = aaa', False),
-    ('test ! aaa = bbb', True),
-    ('test ! ! aaa = aaa', True),
-    ('test ! ! aaa = bbb', False),
-
-    # Binary operators.
-
-    ('test aaa != aaa -o bbb != bbb', False),
-    ('test aaa != aaa -o bbb = bbb', True),
-    ('test aaa = aaa -o bbb != bbb', True),
-    ('test aaa = aaa -o bbb = bbb', True),
-
-    ('test aaa != aaa -a bbb != bbb', False),
-    ('test aaa != aaa -a bbb = bbb', False),
-    ('test aaa = aaa -a bbb != bbb', False),
-    ('test aaa = aaa -a bbb = bbb', True),
-
-    # Inversion within binary operators.
-
-    ('test ! aaa != aaa -o ! bbb != bbb', True),
-    ('test ! aaa != aaa -o ! bbb = bbb', True),
-    ('test ! aaa = aaa -o ! bbb != bbb', True),
-    ('test ! aaa = aaa -o ! bbb = bbb', False),
-
-    ('test ! ! aaa != aaa -o ! ! bbb != bbb', False),
-    ('test ! ! aaa != aaa -o ! ! bbb = bbb', True),
-    ('test ! ! aaa = aaa -o ! ! bbb != bbb', True),
-    ('test ! ! aaa = aaa -o ! ! bbb = bbb', True),
-)
-
-def exec_hush_if(u_boot_console, expr, result):
-    """Execute a shell "if" command, and validate its result."""
-
-    config = u_boot_console.config.buildconfig
-    maxargs = int(config.get('config_sys_maxargs', '0'))
-    args = len(expr.split(' ')) - 1
-    if args > maxargs:
-        u_boot_console.log.warning('CONFIG_SYS_MAXARGS too low; need ' +
-            str(args))
-        pytest.skip()
-
-    cmd = 'if ' + expr + '; then echo true; else echo false; fi'
-    response = u_boot_console.run_command(cmd)
-    assert response.strip() == str(result).lower()
-
[email protected]('cmd_echo')
[email protected]('expr,result', subtests)
-def test_hush_if_test(u_boot_console, expr, result):
-    """Test a single "if test" condition."""
-
-    exec_hush_if(u_boot_console, expr, result)
-
-def test_hush_z(u_boot_console):
-    """Test the -z operator"""
-    u_boot_console.run_command('setenv ut_var_nonexistent')
-    u_boot_console.run_command('setenv ut_var_exists 1')
-    exec_hush_if(u_boot_console, 'test -z "$ut_var_nonexistent"', True)
-    exec_hush_if(u_boot_console, 'test -z "$ut_var_exists"', False)
-    u_boot_console.run_command('setenv ut_var_exists')
-
-# We might test this on real filesystems via UMS, DFU, 'save', etc.
-# Of those, only UMS currently allows file removal though.
[email protected]('cmd_echo')
[email protected]('sandbox')
-def test_hush_if_test_host_file_exists(u_boot_console):
-    """Test the "if test -e" shell command."""
-
-    test_file = u_boot_console.config.result_dir + \
-        '/creating_this_file_breaks_u_boot_tests'
-
-    try:
-        os.unlink(test_file)
-    except:
-        pass
-    assert not os.path.exists(test_file)
-
-    expr = 'test -e hostfs - ' + test_file
-    exec_hush_if(u_boot_console, expr, False)
-
-    try:
-        with open(test_file, 'wb'):
-            pass
-        assert os.path.exists(test_file)
-
-        expr = 'test -e hostfs - ' + test_file
-        exec_hush_if(u_boot_console, expr, True)
-    finally:
-        os.unlink(test_file)
-
-    expr = 'test -e hostfs - ' + test_file
-    exec_hush_if(u_boot_console, expr, False)
-
-def test_hush_var(u_boot_console):
-    """Test the set and unset of variables"""
-    u_boot_console.run_command('ut_var_nonexistent=')
-    u_boot_console.run_command('ut_var_exists=1')
-    u_boot_console.run_command('ut_var_unset=1')
-    exec_hush_if(u_boot_console, 'test -z "$ut_var_nonexistent"', True)
-    exec_hush_if(u_boot_console, 'test -z "$ut_var_exists"', False)
-    exec_hush_if(u_boot_console, 'test -z "$ut_var_unset"', False)
-    exec_hush_if(u_boot_console, 'ut_var_unset=', True)
-    exec_hush_if(u_boot_console, 'test -z "$ut_var_unset"', True)
-    u_boot_console.run_command('ut_var_exists=')
-    u_boot_console.run_command('ut_var_unset=')
diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py
index 0e3a48e73f6..c169c835e38 100644
--- a/test/py/tests/test_ut.py
+++ b/test/py/tests/test_ut.py
@@ -500,5 +500,11 @@ def test_ut(u_boot_console, ut_subtest):
             execute command 'ut foo bar'
     """
 
-    output = u_boot_console.run_command('ut ' + ut_subtest)
+    if ut_subtest == 'hush hush_test_simple_dollar':
+        # ut hush hush_test_simple_dollar prints "Unknown command" on purpose.
+        with u_boot_console.disable_check('unknown_command'):
+            output = u_boot_console.run_command('ut ' + ut_subtest)
+        assert('Unknown command \'quux\' - try \'help\'' in output)
+    else:
+        output = u_boot_console.run_command('ut ' + ut_subtest)
     assert output.endswith('Failures: 0')