diff mbox series

[RFC,v7,11/23] cmd: Add new parser command

Message ID 20230330194716.23623-12-francis.laniel@amarulasolutions.com
State RFC
Delegated to: Tom Rini
Headers show
Series Modernize U-Boot shell | expand

Commit Message

Francis Laniel March 30, 2023, 7:46 p.m. UTC
This command can be used to print the current parser with 'parser print'.
It can also be used to set the current parser with 'parser set'.
For the moment, only one value is valid for set: old.

Signed-off-by: Francis Laniel <francis.laniel@amarulasolutions.com>
---
 cmd/Makefile |   2 +
 cmd/parser.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++
 common/cli.c |   3 +-
 3 files changed, 124 insertions(+), 1 deletion(-)
 create mode 100644 cmd/parser.c

Comments

Simon Glass April 1, 2023, 6:32 a.m. UTC | #1
Hi Francis,

On Fri, 31 Mar 2023 at 08:49, Francis Laniel
<francis.laniel@amarulasolutions.com> wrote:
>
> This command can be used to print the current parser with 'parser print'.
> It can also be used to set the current parser with 'parser set'.
> For the moment, only one value is valid for set: old.
>
> Signed-off-by: Francis Laniel <francis.laniel@amarulasolutions.com>
> ---
>  cmd/Makefile |   2 +
>  cmd/parser.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  common/cli.c |   3 +-
>  3 files changed, 124 insertions(+), 1 deletion(-)
>  create mode 100644 cmd/parser.c
>

Reviewed-by: Simon Glass <sjg@chromium.org>

But I wonder whether 'cli' might be a better name for this command? We
use cli everywhere else.

Also can you add doc/usage for this?

Regards,
Simon
Francis Laniel April 1, 2023, 10:44 p.m. UTC | #2
Hi.


Le samedi 1 avril 2023, 07:32:14 WEST Simon Glass a écrit :
> Hi Francis,
> 
> On Fri, 31 Mar 2023 at 08:49, Francis Laniel
> 
> <francis.laniel@amarulasolutions.com> wrote:
> > This command can be used to print the current parser with 'parser print'.
> > It can also be used to set the current parser with 'parser set'.
> > For the moment, only one value is valid for set: old.
> > 
> > Signed-off-by: Francis Laniel <francis.laniel@amarulasolutions.com>
> > ---
> > 
> >  cmd/Makefile |   2 +
> >  cmd/parser.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  common/cli.c |   3 +-
> >  3 files changed, 124 insertions(+), 1 deletion(-)
> >  create mode 100644 cmd/parser.c
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>

Thank you for the reviews!

> 
> But I wonder whether 'cli' might be a better name for this command? We
> use cli everywhere else.

"Naming thing is hard" [1].
Thank you for the suggestion! I changed it to cli which I think is a better 
name as it is less abstract than parser.

> Also can you add doc/usage for this?

Done! I was not aware of doc/usage/cmd before.

> Regards,
> Simon

By the way, I also addressed your other comments on patches 4 and 9.


Best regards.
---
[1] https://www.karlton.org/2017/12/naming-things-hard/
diff mbox series

Patch

diff --git a/cmd/Makefile b/cmd/Makefile
index 36d2daf22a..659a64371a 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -220,6 +220,8 @@  obj-$(CONFIG_CMD_AVB) += avb.o
 # Foundries.IO SCP03
 obj-$(CONFIG_CMD_SCP03) += scp03.o
 
+obj-$(CONFIG_HUSH_PARSER) += parser.o
+
 obj-$(CONFIG_ARM) += arm/
 obj-$(CONFIG_RISCV) += riscv/
 obj-$(CONFIG_SANDBOX) += sandbox/
diff --git a/cmd/parser.c b/cmd/parser.c
new file mode 100644
index 0000000000..0a433b7dc6
--- /dev/null
+++ b/cmd/parser.c
@@ -0,0 +1,120 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <common.h>
+#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(void)
+{
+	if (gd->flags & GD_FLG_HUSH_OLD_PARSER)
+		return "old";
+	return NULL;
+}
+
+static int do_parser_get(struct cmd_tbl *cmdtp, int flag, int argc,
+			 char *const argv[])
+{
+	const char *current = gd_flags_to_parser();
+
+	if (!current) {
+		printf("current parser 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;
+	return -1;
+}
+
+static void reset_parser_gd_flags(void)
+{
+	gd->flags &= ~GD_FLG_HUSH_OLD_PARSER;
+}
+
+static int do_parser_set(struct cmd_tbl *cmdtp, int flag, int argc,
+			      char *const argv[])
+{
+	char *parser_name;
+	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;
+	}
+
+	if (parser_flag == GD_FLG_HUSH_OLD_PARSER &&
+		!CONFIG_IS_ENABLED(HUSH_OLD_PARSER)) {
+		printf("Want to set current parser to old, but its code was not compiled!\n");
+		return CMD_RET_FAILURE;
+	}
+
+	if (parser_flag == GD_FLG_HUSH_2021_PARSER &&
+		!CONFIG_IS_ENABLED(HUSH_2021_PARSER)) {
+		printf("Want to set current parser to 2021, but its code was not compiled!\n");
+		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_parser_get, "", ""),
+	U_BOOT_CMD_MKENT(set, 2, 1, do_parser_set, "", ""),
+};
+
+static int do_parser(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 parser_help_text[] =
+	"get - print current parser\n"
+	"set - set the current parser, possible value is: old"
+	;
+#endif
+
+U_BOOT_CMD(parser, 3, 1, do_parser,
+	   "parser",
+#if CONFIG_IS_ENABLED(SYS_LONGHELP)
+	   parser_help_text
+#endif
+);
diff --git a/common/cli.c b/common/cli.c
index 5eef0cad8c..21f98ca99c 100644
--- a/common/cli.c
+++ b/common/cli.c
@@ -255,7 +255,8 @@  void cli_loop(void)
 void cli_init(void)
 {
 #ifdef CONFIG_HUSH_PARSER
-	if (!(gd->flags & GD_FLG_HUSH_OLD_PARSER))
+	if (!(gd->flags & GD_FLG_HUSH_OLD_PARSER)
+		&& CONFIG_IS_ENABLED(HUSH_OLD_PARSER))
 		gd->flags |= GD_FLG_HUSH_OLD_PARSER;
 	u_boot_hush_start();
 #endif