| Message ID | 20221218204651.170430-1-marex@denx.de |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [1/2] cmd: exit: Fix return value propagation out of environment scripts | expand |
Hi Marek On 18/12/22 21:46, Marek Vasut wrote: > Make sure the 'exit' command as well as 'exit $val' command exits > from environment scripts immediately and propagates return value > out of those scripts fully. That means the following behavior is > expected: > > " > => setenv foo 'echo bar ; exit 1' ; run foo ; echo $? > bar > 1 > => setenv foo 'echo bar ; exit 0' ; run foo ; echo $? > bar > 0 > => setenv foo 'echo bar ; exit -2' ; run foo ; echo $? > bar > 0 > " > > As well as the followin behavior: > > " > => setenv foo 'echo bar ; exit 3 ; echo fail'; run foo; echo $? > bar > 3 > => setenv foo 'echo bar ; exit 1 ; echo fail'; run foo; echo $? > bar > 1 > => setenv foo 'echo bar ; exit 0 ; echo fail'; run foo; echo $? > bar > 0 > => setenv foo 'echo bar ; exit -1 ; echo fail'; run foo; echo $? > bar > 0 > => setenv foo 'echo bar ; exit -2 ; echo fail'; run foo; echo $? > bar > 0 > => setenv foo 'echo bar ; exit ; echo fail'; run foo; echo $? > bar > 0 > " > > Fixes: 8c4e3b79bd0 ("cmd: exit: Fix return value") > Signed-off-by: Marek Vasut <marex@denx.de> > --- > Cc: Adrian Vovk <avovk@cc-sw.com> > Cc: Hector Palacios <hector.palacios@digi.com> > Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> > Cc: Simon Glass <sjg@chromium.org> > Cc: Tom Rini <trini@konsulko.com> > --- > cmd/exit.c | 7 +++++-- > common/cli.c | 7 ++++--- > common/cli_hush.c | 21 +++++++++++++++------ > 3 files changed, 24 insertions(+), 11 deletions(-) > > diff --git a/cmd/exit.c b/cmd/exit.c > index 2c7132693ad..7bf241ec732 100644 > --- a/cmd/exit.c > +++ b/cmd/exit.c > @@ -10,10 +10,13 @@ > static int do_exit(struct cmd_tbl *cmdtp, int flag, int argc, > char *const argv[]) > { > + int r; > + > + r = 0; > if (argc > 1) > - return dectoul(argv[1], NULL); > + r = simple_strtoul(argv[1], NULL, 10); > > - return 0; > + return -r - 2; > } > > U_BOOT_CMD( > diff --git a/common/cli.c b/common/cli.c > index a47d6a3f2b4..ba45dad2db5 100644 > --- a/common/cli.c > +++ b/common/cli.c > @@ -146,7 +146,7 @@ int run_commandf(const char *fmt, ...) > #if defined(CONFIG_CMD_RUN) > int do_run(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) > { > - int i; > + int i, ret; > > if (argc < 2) > return CMD_RET_USAGE; > @@ -160,8 +160,9 @@ int do_run(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) > return 1; > } > > - if (run_command(arg, flag | CMD_FLAG_ENV) != 0) > - return 1; > + ret = run_command(arg, flag | CMD_FLAG_ENV); > + if (ret) > + return ret; > } > return 0; > } > diff --git a/common/cli_hush.c b/common/cli_hush.c > index 1467ff81b35..b8940b19735 100644 > --- a/common/cli_hush.c > +++ b/common/cli_hush.c > @@ -1902,7 +1902,7 @@ static int run_list_real(struct pipe *pi) > last_return_code = -rcode - 2; > return -2; /* exit */ > } > - last_return_code=(rcode == 0) ? 0 : 1; > + last_return_code = rcode; > #endif > #ifndef __U_BOOT__ > pi->num_progs = save_num_progs; /* restore number of programs */ > @@ -3212,7 +3212,15 @@ static int parse_stream_outer(struct in_str *inp, int flag) > printf("exit not allowed from main input shell.\n"); > continue; > } > - break; > + /* > + * DANGER > + * Return code -2 is special in this context, > + * it indicates exit from inner pipe instead > + * of return code itself, the return code is > + * stored in 'last_return_code' variable! > + * DANGER > + */ > + return -2; > } > if (code == -1) > flag_repeat = 0; > @@ -3249,9 +3257,9 @@ int parse_string_outer(const char *s, int flag) > #endif /* __U_BOOT__ */ > { > struct in_str input; > + int rcode; > #ifdef __U_BOOT__ > char *p = NULL; > - int rcode; > if (!s) > return 1; > if (!*s) > @@ -3263,11 +3271,12 @@ int parse_string_outer(const char *s, int flag) > setup_string_in_str(&input, p); > rcode = parse_stream_outer(&input, flag); > free(p); > - return rcode; > + return rcode == -2 ? last_return_code : rcode; > } else { > #endif > setup_string_in_str(&input, s); > - return parse_stream_outer(&input, flag); > + rcode = parse_stream_outer(&input, flag); > + return rcode == -2 ? last_return_code : rcode; > #ifdef __U_BOOT__ > } > #endif > @@ -3287,7 +3296,7 @@ int parse_file_outer(void) > setup_file_in_str(&input); > #endif > rcode = parse_stream_outer(&input, FLAG_PARSE_SEMICOLON); > - return rcode; > + return rcode == -2 ? last_return_code : rcode; > } > > #ifdef __U_BOOT__ > -- > 2.35.1 > Can you also update the documentation at doc/usage/cmd/exit.rst? It still says 'exit' only returns 0. Otherwise Reviewed-by: Hector Palacios <hector.palacios@digi.com> Thanks!
diff --git a/cmd/exit.c b/cmd/exit.c index 2c7132693ad..7bf241ec732 100644 --- a/cmd/exit.c +++ b/cmd/exit.c @@ -10,10 +10,13 @@ static int do_exit(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { + int r; + + r = 0; if (argc > 1) - return dectoul(argv[1], NULL); + r = simple_strtoul(argv[1], NULL, 10); - return 0; + return -r - 2; } U_BOOT_CMD( diff --git a/common/cli.c b/common/cli.c index a47d6a3f2b4..ba45dad2db5 100644 --- a/common/cli.c +++ b/common/cli.c @@ -146,7 +146,7 @@ int run_commandf(const char *fmt, ...) #if defined(CONFIG_CMD_RUN) int do_run(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { - int i; + int i, ret; if (argc < 2) return CMD_RET_USAGE; @@ -160,8 +160,9 @@ int do_run(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return 1; } - if (run_command(arg, flag | CMD_FLAG_ENV) != 0) - return 1; + ret = run_command(arg, flag | CMD_FLAG_ENV); + if (ret) + return ret; } return 0; } diff --git a/common/cli_hush.c b/common/cli_hush.c index 1467ff81b35..b8940b19735 100644 --- a/common/cli_hush.c +++ b/common/cli_hush.c @@ -1902,7 +1902,7 @@ static int run_list_real(struct pipe *pi) last_return_code = -rcode - 2; return -2; /* exit */ } - last_return_code=(rcode == 0) ? 0 : 1; + last_return_code = rcode; #endif #ifndef __U_BOOT__ pi->num_progs = save_num_progs; /* restore number of programs */ @@ -3212,7 +3212,15 @@ static int parse_stream_outer(struct in_str *inp, int flag) printf("exit not allowed from main input shell.\n"); continue; } - break; + /* + * DANGER + * Return code -2 is special in this context, + * it indicates exit from inner pipe instead + * of return code itself, the return code is + * stored in 'last_return_code' variable! + * DANGER + */ + return -2; } if (code == -1) flag_repeat = 0; @@ -3249,9 +3257,9 @@ int parse_string_outer(const char *s, int flag) #endif /* __U_BOOT__ */ { struct in_str input; + int rcode; #ifdef __U_BOOT__ char *p = NULL; - int rcode; if (!s) return 1; if (!*s) @@ -3263,11 +3271,12 @@ int parse_string_outer(const char *s, int flag) setup_string_in_str(&input, p); rcode = parse_stream_outer(&input, flag); free(p); - return rcode; + return rcode == -2 ? last_return_code : rcode; } else { #endif setup_string_in_str(&input, s); - return parse_stream_outer(&input, flag); + rcode = parse_stream_outer(&input, flag); + return rcode == -2 ? last_return_code : rcode; #ifdef __U_BOOT__ } #endif @@ -3287,7 +3296,7 @@ int parse_file_outer(void) setup_file_in_str(&input); #endif rcode = parse_stream_outer(&input, FLAG_PARSE_SEMICOLON); - return rcode; + return rcode == -2 ? last_return_code : rcode; } #ifdef __U_BOOT__
Make sure the 'exit' command as well as 'exit $val' command exits from environment scripts immediately and propagates return value out of those scripts fully. That means the following behavior is expected: " => setenv foo 'echo bar ; exit 1' ; run foo ; echo $? bar 1 => setenv foo 'echo bar ; exit 0' ; run foo ; echo $? bar 0 => setenv foo 'echo bar ; exit -2' ; run foo ; echo $? bar 0 " As well as the followin behavior: " => setenv foo 'echo bar ; exit 3 ; echo fail'; run foo; echo $? bar 3 => setenv foo 'echo bar ; exit 1 ; echo fail'; run foo; echo $? bar 1 => setenv foo 'echo bar ; exit 0 ; echo fail'; run foo; echo $? bar 0 => setenv foo 'echo bar ; exit -1 ; echo fail'; run foo; echo $? bar 0 => setenv foo 'echo bar ; exit -2 ; echo fail'; run foo; echo $? bar 0 => setenv foo 'echo bar ; exit ; echo fail'; run foo; echo $? bar 0 " Fixes: 8c4e3b79bd0 ("cmd: exit: Fix return value") Signed-off-by: Marek Vasut <marex@denx.de> --- Cc: Adrian Vovk <avovk@cc-sw.com> Cc: Hector Palacios <hector.palacios@digi.com> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> Cc: Simon Glass <sjg@chromium.org> Cc: Tom Rini <trini@konsulko.com> --- cmd/exit.c | 7 +++++-- common/cli.c | 7 ++++--- common/cli_hush.c | 21 +++++++++++++++------ 3 files changed, 24 insertions(+), 11 deletions(-)