Re: [PATCH 2/5] cmd: Add support for (limited) command substitution

On 2/28/21 6:59 PM, Heinrich Schuchardt wrote:

Am 1. März 2021 00:47:15 MEZ schrieb Sean Anderson seanga2@gmail.com:

...

The traditional way to grab the output from a shell script is to use command substitution. Unfortunately, we don't really have the concept of pipes in U-Boot (at least not without console recording). Even if we did, stdout is severely polluted by informational printfs.

Instead of redirecting stdout, instead use a special global variable. This lets commands set a result without having to modify the function signature of every command, and everything that calls commands. This is a bit of a hack, but seemed like the least-invasive of all options.

Currently, the value of cmd_result is printed if it is set. This makes sense for things like echo, where you can do something like

=> echo a b c d e a b c d e => var=c => echo a $(echo b $var d) e a b c d e

but it makes less sense for some other commands

All callers of cmd_process must

1. Print cmd_result (unless it is being "redirected")
2. free() cmd_result
3. set cmd_result to NULL

Calling cmd_process with a non-NULL value in cmd_result is a bug.

I don't understand what you are changing from the lines above.

=> echo a $(echo b $var d) e

This is a syntax error at the moment, and this series makes it work.

Before extending the hush shell we should write a man-page in doc/usage/ describing what it actually does.

What would it be named? Is there an existing section of the documentation which documents the standard and non-standard features of hush?

Building in new gimmicks without documentation does not make any sense to me.

Yes, this should likely be better-documented. However, I'm not sure that this approach is the best one, so I wanted to get some feedback before writing out documentation. I suppose this makes this an RFC series.

--Sean

Best regards

Heinrich

...

Signed-off-by: Sean Anderson seanga2@gmail.com

common/cli_hush.c | 39 ++++++++++++++++++++++++++++--- common/cli_simple.c | 8 ++++++- common/command.c | 3 +++ include/asm-generic/global_data.h | 4 ++++ 4 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/common/cli_hush.c b/common/cli_hush.c index 83329763c6..8fed7eb14e 100644 --- a/common/cli_hush.c +++ b/common/cli_hush.c @@ -475,6 +475,8 @@ static char *make_string(char **inp, int *nonnull); static int handle_dollar(o_string *dest, struct p_context *ctx, struct in_str *input); #ifndef __U_BOOT__ static int parse_string(o_string *dest, struct p_context *ctx, const char *src); +#else +static void update_ifs_map(void); #endif static int parse_stream(o_string *dest, struct p_context *ctx, struct in_str *input0, int end_trigger); /* setup: */ @@ -1673,6 +1675,10 @@ static int run_pipe_real(struct pipe *pi) "'run' command\n", child->argv[i]); return -1; }

• if (gd->cmd_result)
  

• 	puts(gd->cmd_result);
  

• free(gd->cmd_result);
  

• gd->cmd_result = NULL;
  

  /* Process the command */ return cmd_process(flag, child->argc - i, child->argv + i, &flag_repeat, NULL);

@@ -2683,6 +2689,7 @@ FILE *generate_stream_from_list(struct pipe *head) #endif return pf; } +#endif /* __U_BOOT__ */

/* this version hacked for testing purposes */ /* return code is exit status of the process that is run. */ @@ -2691,7 +2698,11 @@ static int process_command_subs(o_string *dest, struct p_context *ctx, struct in int retcode; o_string result=NULL_O_STRING; struct p_context inner; +#ifdef __U_BOOT__

• int list_retcode;

+#else FILE *p; +#endif struct in_str pipe_str; initialize_context(&inner);

@@ -2702,13 +2713,21 @@ static int process_command_subs(o_string *dest, struct p_context *ctx, struct in done_pipe(&inner, PIPE_SEQ); b_free(&result);

+#ifdef __U_BOOT__

• list_retcode = run_list_real(inner.list_head);
• setup_string_in_str(&pipe_str, gd->cmd_result ?: "");
• /* Restore the original map as best we can */
• update_ifs_map();

+#else p=generate_stream_from_list(inner.list_head); if (p==NULL) return 1; mark_open(fileno(p)); setup_file_in_str(&pipe_str, p); +#endif

/* now send results of command back into original context */ retcode = parse_stream(dest, ctx, &pipe_str, '\0'); +#ifndef __U_BOOT__ /* XXX In case of a syntax error, should we try to kill the child? * That would be tough to do right, so just read until EOF. */ if (retcode == 1) { @@ -2723,12 +2742,18 @@ static int process_command_subs(o_string *dest, struct p_context *ctx, struct in * to the KISS philosophy of this program. */ mark_closed(fileno(p)); retcode=pclose(p); +#else

• free(gd->cmd_result);
• gd->cmd_result = NULL;
• retcode = list_retcode;

+#endif free_pipe_list(inner.list_head,0); debug_printf("pclosed, retcode=%d\n",retcode); /* XXX this process fails to trim a single trailing newline */ return retcode; }

+#ifndef __U_BOOT__ static int parse_group(o_string *dest, struct p_context *ctx, struct in_str *input, int ch) { @@ -2896,11 +2921,11 @@ static int handle_dollar(o_string *dest, struct p_context *ctx, struct in_str *i } b_addchr(dest, SPECIAL_VAR_SYMBOL); break; -#ifndef __U_BOOT__ case '(': b_getch(input); process_command_subs(dest, ctx, input, ')'); break; +#ifndef __U_BOOT__ case '*': sep[0]=ifs[0]; for (i=1; i<global_argc; i++) { @@ -3165,7 +3190,7 @@ static void update_ifs_map(void) mapset(subst, 3); /* never flow through */ } mapset((uchar *)"\$'"", 3); /* never flow through */

• mapset((uchar *)";&|#", 1); /* flow through if quoted */

• mapset((uchar *)";&|()#", 1); /* flow through if quoted */

#endif mapset(ifs, 2); /* also flow through if quoted */ } @@ -3185,7 +3210,8 @@ static int parse_stream_outer(struct in_str *inp, int flag) ctx.type = flag; initialize_context(&ctx); update_ifs_map();

• if (!(flag & FLAG_PARSE_SEMICOLON) || (flag & FLAG_REPARSING))
  

mapset((uchar *)";$&|", 0);

• if (!(flag & FLAG_PARSE_SEMICOLON) || (flag & FLAG_REPARSING))
  

• 	mapset((uchar *)";$&|()", 0);
  

  inp->promptmode=1; rcode = parse_stream(&temp, &ctx, inp, flag & FLAG_CONT_ON_NEWLINE ? -1 : '\n');

@@ -3205,6 +3231,13 @@ static int parse_stream_outer(struct in_str *inp, int flag) run_list(ctx.list_head); #else code = run_list(ctx.list_head);

• 	if (!(flag & FLAG_REPARSING) && gd->cmd_result) {
  

• 		puts(gd->cmd_result);
  

• 		free(gd->cmd_result);
  

• 		gd->cmd_result = NULL;
  

• 	}
  

• if (code == -2) {	/* exit */
  	b_free(&temp);
  	code = 0;
  

diff --git a/common/cli_simple.c b/common/cli_simple.c index e80ba488a5..5df30d964f 100644 --- a/common/cli_simple.c +++ b/common/cli_simple.c @@ -15,14 +15,16 @@ #include <console.h> #include <env.h> #include <log.h> +#include <malloc.h> #include <linux/ctype.h>

+DECLARE_GLOBAL_DATA_PTR;

#define DEBUG_PARSER 0 /* set to 1 to debug */

#define debug_parser(fmt, args...) \ debug_cond(DEBUG_PARSER, fmt, ##args)

int cli_simple_parse_line(char *line, char *argv[]) { int nargs = 0; @@ -257,6 +259,10 @@ int cli_simple_run_command(const char *cmd, int flag)

if (cmd_process(flag, argc, argv, &repeatable, NULL))
	rc = -1;

• if (gd->cmd_result)
  

• 	puts(gd->cmd_result);
  

• free(gd->cmd_result);
  

• gd->cmd_result = NULL;
  
  

  /* Did the user stop this? */ if (had_ctrlc())

diff --git a/common/command.c b/common/command.c index 3fe6791eda..952a8f00eb 100644 --- a/common/command.c +++ b/common/command.c @@ -588,6 +588,9 @@ enum command_ret_t cmd_process(int flag, int argc, char *const argv[], enum command_ret_t rc = CMD_RET_SUCCESS; struct cmd_tbl *cmdtp;

• /* Clear previous result */
• assert(!gd->cmd_result);

#if defined(CONFIG_SYS_XTRACE) char *xtrace;

diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index b6a9991fc9..85262d9566 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -453,6 +453,10 @@ struct global_data { */ char *smbios_version; #endif

• /**

• * @cmd_result: Result of the current command
  

• */
  

• char *cmd_result;

};

/**