[PATCH v2 2/3] allow positional arguments with "run" command

Rasmus Villemoes
2020-10-07T07:20:51Z

Currently, the only way to emulate functions with arguments in the
U-Boot shell is by doing "foo=arg1; bar=arg2; run func" and having
"func" refer to $foo and $bar. That works, but is a bit clunky, and
also suffers from foo and bar being set globally - if func itself wants
to run other "functions" defined in the environment, those other
functions better not use the same parameter names:

setenv g 'do_g_stuff $foo'
setenv f 'do_f_stuff $foo $bar; foo=123; run g; do_more_f_stuff $foo $bar'
foo=arg1; bar=arg2; run f

Sure, f could do a "saved_foo=$foo; .... foo=$saved_foo" dance, but
that makes everything even more clunky.

In order to increase readability, allow passing positional arguments
to the functions invoked via run: When invoked with a -- separator,
the remaining arguments are use to set the local shell variables $1 through
$9 (and $#). As in a "real" shell, they are local to the current
function, so if f is called with two arguments, and f calls g with one
argument, g sees $2 as unset. Then the above can be written

setenv g 'do_g_stuff $1'
setenv f 'do_f_stuff $1 $2; run g -- 123; do_more_f_stuff $1 $2'
run f -- arg1 arg2

Everything except

- b_addchr(dest, '?');
+ b_addchr(dest, ch);

is under CONFIG_CMD_RUN_ARGS, and when CONFIG_CMD_RUN_ARGS=n, the ch
there can only be '?'. So no functional change when
CONFIG_CMD_RUN_ARGS is not selected.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
cmd/Kconfig | 10 ++++++++++
cmd/nvedit.c | 7 ++++++-
common/cli.c | 44 ++++++++++++++++++++++++++++++++++++++------
common/cli_hush.c | 32 +++++++++++++++++++++++++++++++-
include/cli_hush.h | 9 +++++++++
5 files changed, 94 insertions(+), 8 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 0c984d735d..b8426d19d7 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -443,6 +443,16 @@ config CMD_RUN
help
Run the command in the given environment variable.

+config CMD_RUN_ARGS
+ bool "allow positional arguments with run command"
+ depends on HUSH_PARSER
+ depends on CMD_RUN
+ help
+ Allow invoking 'run' as 'run f g -- arg1 arg2 ...', which
+ will run the commands defined in the environment variables f
+ and g with positional arguments $1..$9 set to the arguments
+ following the -- separator.
+
config CMD_IMI
bool "iminfo"
default y
diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 7fce723800..202139bfb9 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -1575,7 +1575,12 @@ U_BOOT_CMD_COMPLETE(
run, CONFIG_SYS_MAXARGS, 1, do_run,
"run commands in an environment variable",
"var [...]\n"
- " - run the commands in the environment variable(s) 'var'",
+ " - run the commands in the environment variable(s) 'var'\n"
+ CONFIG_IS_ENABLED(CMD_RUN_ARGS, (
+ "run var [...] -- arg1 arg2 [...]\n"
+ " - run the commands in the environment variable(s) 'var',\n"
+ " with shell variables $1, $2, ... set to arg1, arg2, ...\n"
+ )),
var_complete
);
#endif
diff --git a/common/cli.c b/common/cli.c
index 6635ab2bcf..f970bd1eae 100644
--- a/common/cli.c
+++ b/common/cli.c
@@ -131,24 +131,56 @@ int run_command_list(const char *cmd, int len, int flag)
#if defined(CONFIG_CMD_RUN)
int do_run(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
{
- int i;
+ struct run_args ra;
+ int i, j;
+ int ret = 0;
+ int cmds = argc;

if (argc < 2)
return CMD_RET_USAGE;

- for (i = 1; i < argc; ++i) {
+ if (CONFIG_IS_ENABLED(CMD_RUN_ARGS)) {
+ for (i = 1; i < argc; ++i) {
+ if (!strcmp(argv[i], "--")) {
+ cmds = i;
+ ++i;
+ break;
+ }
+ }
+ ra.count = argc - i;
+ if (ra.count > MAX_RUN_ARGS) {
+ printf("## Error: At most %d positional arguments allowed\n",
+ MAX_RUN_ARGS);
+ return 1;
+ }
+ for (j = i; j < argc; ++j)
+ ra.args[j - i] = argv[j];
+
+ ra.prev = current_run_args;
+ current_run_args = &ra;
+ }
+
+ for (i = 1; i < cmds; ++i) {
char *arg;

arg = env_get(argv[i]);
if (arg == NULL) {
printf("## Error: \"%s\" not defined\n", argv[i]);
- return 1;
+ ret = 1;
+ goto out;
}

- if (run_command(arg, flag | CMD_FLAG_ENV) != 0)
- return 1;
+ if (run_command(arg, flag | CMD_FLAG_ENV) != 0) {
+ ret = 1;
+ goto out;
+ }
}
- return 0;
+
+out:
+ if (CONFIG_IS_ENABLED(CMD_RUN_ARGS))
+ current_run_args = ra.prev;
+
+ return ret;
}
#endif

diff --git a/common/cli_hush.c b/common/cli_hush.c
index 072b871f1e..df35c9c8d2 100644
--- a/common/cli_hush.c
+++ b/common/cli_hush.c
@@ -135,6 +135,11 @@ DECLARE_GLOBAL_DATA_PTR;
#define syntax() syntax_err()
#define xstrdup strdup
#define error_msg printf
+
+#ifdef CONFIG_CMD_RUN_ARGS
+struct run_args *current_run_args;
+#endif
+
#else
typedef enum {
REDIRECT_INPUT = 1,
@@ -2144,6 +2149,10 @@ char *get_local_var(const char *s)
#ifdef __U_BOOT__
if (*s == '$')
return get_dollar_var(s[1]);
+ /* To make ${1:-default} work: */
+ if (IS_ENABLED(CONFIG_CMD_RUN_ARGS) &&
+ '1' <= s[0] && s[0] <= '9' && !s[1])
+ return get_dollar_var(s[0]);
#endif

for (cur = top_vars; cur; cur=cur->next)
@@ -2826,6 +2835,23 @@ static char *get_dollar_var(char ch)
case '?':
sprintf(buf, "%u", (unsigned int)last_return_code);
break;
+#ifdef CONFIG_CMD_RUN_ARGS
+ case '#':
+ if (!current_run_args)
+ return NULL;
+ sprintf(buf, "%u", current_run_args->count);
+ break;
+ case '1' ... '9': {
+ const struct run_args *ra = current_run_args;
+ int i = ch - '1';
+
+ if (!ra)
+ return NULL;
+ if (i >= ra->count)
+ return NULL;
+ return ra->args[i];
+ }
+#endif
default:
return NULL;
}
@@ -2865,10 +2891,14 @@ static int handle_dollar(o_string *dest, struct p_context *ctx, struct in_str *i
} else switch (ch) {
#ifdef __U_BOOT__
case '?':
+#ifdef CONFIG_CMD_RUN_ARGS
+ case '1' ... '9':
+ case '#':
+#endif
ctx->child->sp++;
b_addchr(dest, SPECIAL_VAR_SYMBOL);
b_addchr(dest, '$');
- b_addchr(dest, '?');
+ b_addchr(dest, ch);
b_addchr(dest, SPECIAL_VAR_SYMBOL);
advance = 1;
break;
diff --git a/include/cli_hush.h b/include/cli_hush.h
index 2bd35670c7..d6eb7e908d 100644
--- a/include/cli_hush.h
+++ b/include/cli_hush.h
@@ -23,4 +23,13 @@ char *get_local_var(const char *s);
#if defined(CONFIG_HUSH_INIT_VAR)
extern int hush_init_var (void);
#endif
+
+#define MAX_RUN_ARGS 9
+struct run_args {
+ struct run_args *prev;
+ int count;
+ char *args[MAX_RUN_ARGS]; /* [0] holds $1 etc. */
+};
+extern struct run_args *current_run_args;
+
#endif
--
2.23.0