Merge patch series "Modernize U-Boot shell"

Francis Laniel <francis.laniel@amarulasolutions.com> 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).

3 files changed, 156 insertions, 0 deletions

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 5a7678f0ac9..26aeeeed03b 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -22,6 +22,27 @@ config HUSH_PARSER
 	  If disabled, you get the old, much simpler behaviour with a somewhat
 	  smaller memory footprint.
 
+menu "Hush flavor to use"
+depends on HUSH_PARSER
+
+config HUSH_OLD_PARSER
+	bool "Use hush old parser"
+	help
+	  This option enables the old flavor of hush based on hush Busybox from
+	  2005.
+
+config HUSH_MODERN_PARSER
+	bool "Use hush modern parser"
+	default y
+	help
+	  This option enables the new flavor of hush based on hush upstream
+	  Busybox.
+
+config HUSH_SELECTABLE
+	bool
+	default y if HUSH_OLD_PARSER && HUSH_MODERN_PARSER
+endmenu
+
 config CMDLINE_EDITING
 	bool "Enable command line editing"
 	default y
diff --git a/cmd/Makefile b/cmd/Makefile
index 5ed0e4011d0..e2a2b16ab25 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -229,6 +229,8 @@ obj-$(CONFIG_CMD_AVB) += avb.o
 # Foundries.IO SCP03
 obj-$(CONFIG_CMD_SCP03) += scp03.o
 
+obj-$(CONFIG_HUSH_SELECTABLE) += cli.o
+
 obj-$(CONFIG_ARM) += arm/
 obj-$(CONFIG_RISCV) += riscv/
 obj-$(CONFIG_SANDBOX) += sandbox/
diff --git a/cmd/cli.c b/cmd/cli.c
new file mode 100644
index 00000000000..be3bf7dfe20
--- /dev/null
+++ b/cmd/cli.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <cli.h>
+#include <command.h>
+#include <string.h>
+#include <asm/global_data.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static const char *gd_flags_to_parser_name(void)
+{
+	if (gd->flags & GD_FLG_HUSH_OLD_PARSER)
+		return "old";
+	if (gd->flags & GD_FLG_HUSH_MODERN_PARSER)
+		return "modern";
+	return NULL;
+}
+
+static int do_cli_get(struct cmd_tbl *cmdtp, int flag, int argc,
+			 char *const argv[])
+{
+	const char *current = gd_flags_to_parser_name();
+
+	if (!current) {
+		printf("current cli value is not valid, this should not happen!\n");
+		return CMD_RET_FAILURE;
+	}
+
+	printf("%s\n", current);
+
+	return CMD_RET_SUCCESS;
+}
+
+static int parser_string_to_gd_flags(const char *parser)
+{
+	if (!strcmp(parser, "old"))
+		return GD_FLG_HUSH_OLD_PARSER;
+	if (!strcmp(parser, "modern"))
+		return GD_FLG_HUSH_MODERN_PARSER;
+	return -1;
+}
+
+static int gd_flags_to_parser_config(int flag)
+{
+	if (gd->flags & GD_FLG_HUSH_OLD_PARSER)
+		return CONFIG_VAL(HUSH_OLD_PARSER);
+	if (gd->flags & GD_FLG_HUSH_MODERN_PARSER)
+		return CONFIG_VAL(HUSH_MODERN_PARSER);
+	return -1;
+}
+
+static void reset_parser_gd_flags(void)
+{
+	gd->flags &= ~GD_FLG_HUSH_OLD_PARSER;
+	gd->flags &= ~GD_FLG_HUSH_MODERN_PARSER;
+}
+
+static int do_cli_set(struct cmd_tbl *cmdtp, int flag, int argc,
+		      char *const argv[])
+{
+	char *parser_name;
+	int parser_config;
+	int parser_flag;
+
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
+	parser_name = argv[1];
+
+	parser_flag = parser_string_to_gd_flags(parser_name);
+	if (parser_flag == -1) {
+		printf("Bad value for parser name: %s\n", parser_name);
+		return CMD_RET_USAGE;
+	}
+
+	parser_config = gd_flags_to_parser_config(parser_flag);
+	switch (parser_config) {
+	case -1:
+		printf("Bad value for parser flags: %d\n", parser_flag);
+		return CMD_RET_FAILURE;
+	case 0:
+		printf("Want to set current parser to %s, but its code was not compiled!\n",
+			parser_name);
+		return CMD_RET_FAILURE;
+	}
+
+	reset_parser_gd_flags();
+	gd->flags |= parser_flag;
+
+	cli_init();
+	cli_loop();
+
+	/* cli_loop() should never return. */
+	return CMD_RET_FAILURE;
+}
+
+static struct cmd_tbl parser_sub[] = {
+	U_BOOT_CMD_MKENT(get, 1, 1, do_cli_get, "", ""),
+	U_BOOT_CMD_MKENT(set, 2, 1, do_cli_set, "", ""),
+};
+
+static int do_cli(struct cmd_tbl *cmdtp, int flag, int argc,
+		  char *const argv[])
+{
+	struct cmd_tbl *cp;
+
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
+	/* drop initial "parser" arg */
+	argc--;
+	argv++;
+
+	cp = find_cmd_tbl(argv[0], parser_sub, ARRAY_SIZE(parser_sub));
+	if (cp)
+		return cp->cmd(cmdtp, flag, argc, argv);
+
+	return CMD_RET_USAGE;
+}
+
+#if CONFIG_IS_ENABLED(SYS_LONGHELP)
+static char cli_help_text[] =
+	"get - print current cli\n"
+	"set - set the current cli, possible value are: old, modern"
+	;
+#endif
+
+U_BOOT_CMD(cli, 3, 1, do_cli,
+	   "cli",
+#if CONFIG_IS_ENABLED(SYS_LONGHELP)
+	   cli_help_text
+#endif
+);