[PATCH 1/2] cmd: exit: Fix return value propagation out of environment scripts

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__

Add a test which validates that exit from environment script works as expected, including return value propagation and clipping to positive integers.
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 --- include/test/suites.h | 1 + test/cmd/Makefile | 2 +- test/cmd/exit.c | 135 ++++++++++++++++++++++++++++++++++++++++++ test/cmd_ut.c | 1 + 4 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 test/cmd/exit.c
diff --git a/include/test/suites.h b/include/test/suites.h index a01000e127b..9ce49cbb031 100644 --- a/include/test/suites.h +++ b/include/test/suites.h @@ -38,6 +38,7 @@ int do_ut_compression(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); int do_ut_dm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); int do_ut_env(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); +int do_ut_exit(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); int do_ut_fdt(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); int do_ut_font(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); int do_ut_lib(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); diff --git a/test/cmd/Makefile b/test/cmd/Makefile index bc961df3dce..09e410ec30e 100644 --- a/test/cmd/Makefile +++ b/test/cmd/Makefile @@ -8,7 +8,7 @@ endif ifdef CONFIG_CONSOLE_RECORD obj-$(CONFIG_CMD_PAUSE) += test_pause.o endif -obj-y += mem.o +obj-y += exit.o mem.o obj-$(CONFIG_CMD_ADDRMAP) += addrmap.o obj-$(CONFIG_CMD_FDT) += fdt.o obj-$(CONFIG_CONSOLE_TRUETYPE) += font.o diff --git a/test/cmd/exit.c b/test/cmd/exit.c new file mode 100644 index 00000000000..ca34abef899 --- /dev/null +++ b/test/cmd/exit.c @@ -0,0 +1,135 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Tests for exit command + * + * Copyright 2022 Marek Vasut marex@denx.de + */ + +#include <common.h> +#include <console.h> +#include <mapmem.h> +#include <asm/global_data.h> +#include <test/suites.h> +#include <test/ut.h> + +DECLARE_GLOBAL_DATA_PTR; + +/* Declare a new exit test */ +#define EXIT_TEST(_name, _flags) UNIT_TEST(_name, _flags, exit_test) + +/* Test 'exit addr' getting/setting address */ +static int cmd_exit_test(struct unit_test_state *uts) +{ + int i; + + /* + * Test 'exit' with parameter -3, -2, -1, 0, 1, 2, 3 . Use all those + * parameters to cover also the special return value -2 that is used + * in HUSH to detect exit command. + * + * Always test whether 'exit' command: + * - exits out of the 'run' command + * - return value is propagated out of the 'run' command + * - return value can be tested on outside of 'run' command + * - return value can be printed outside of 'run' command + */ + for (i = -3; i <= 3; i++) { + ut_assertok(console_record_reset_enable()); + ut_assertok(run_commandf("setenv foo 'echo bar ; exit %d ; echo baz' ; run foo ; echo $?", i)); + ut_assert_nextline("bar"); + ut_assert_nextline("%d", i > 0 ? i : 0); + ut_assertok(ut_check_console_end(uts)); + + ut_assertok(console_record_reset_enable()); + ut_assertok(run_commandf("setenv foo 'echo bar ; exit %d ; echo baz' ; run foo && echo quux ; echo $?", i)); + ut_assert_nextline("bar"); + if (i <= 0) + ut_assert_nextline("quux"); + ut_assert_nextline("%d", i > 0 ? i : 0); + ut_assertok(ut_check_console_end(uts)); + + ut_assertok(console_record_reset_enable()); + ut_assertok(run_commandf("setenv foo 'echo bar ; exit %d ; echo baz' ; run foo || echo quux ; echo $?", i)); + ut_assert_nextline("bar"); + if (i > 0) + ut_assert_nextline("quux"); + /* Either 'exit' returns 0, or 'echo quux' returns 0 */ + ut_assert_nextline("0"); + ut_assertok(ut_check_console_end(uts)); + } + + /* Validate that 'exit' behaves the same way as 'exit 0' */ + ut_assertok(console_record_reset_enable()); + ut_assertok(run_commandf("setenv foo 'echo bar ; exit ; echo baz' ; run foo ; echo $?", i)); + ut_assert_nextline("bar"); + ut_assert_nextline("0"); + ut_assertok(ut_check_console_end(uts)); + + ut_assertok(console_record_reset_enable()); + ut_assertok(run_commandf("setenv foo 'echo bar ; exit ; echo baz' ; run foo && echo quux ; echo $?", i)); + ut_assert_nextline("bar"); + ut_assert_nextline("quux"); + ut_assert_nextline("0"); + ut_assertok(ut_check_console_end(uts)); + + ut_assertok(console_record_reset_enable()); + ut_assertok(run_commandf("setenv foo 'echo bar ; exit ; echo baz' ; run foo || echo quux ; echo $?", i)); + ut_assert_nextline("bar"); + /* Either 'exit' returns 0, or 'echo quux' returns 0 */ + ut_assert_nextline("0"); + ut_assertok(ut_check_console_end(uts)); + + /* Validate that return value still propagates from 'run' command */ + ut_assertok(console_record_reset_enable()); + ut_assertok(run_commandf("setenv foo 'echo bar ; true' ; run foo ; echo $?", i)); + ut_assert_nextline("bar"); + ut_assert_nextline("0"); + ut_assertok(ut_check_console_end(uts)); + + ut_assertok(console_record_reset_enable()); + ut_assertok(run_commandf("setenv foo 'echo bar ; true' ; run foo && echo quux ; echo $?", i)); + ut_assert_nextline("bar"); + ut_assert_nextline("quux"); + ut_assert_nextline("0"); + ut_assertok(ut_check_console_end(uts)); + + ut_assertok(console_record_reset_enable()); + ut_assertok(run_commandf("setenv foo 'echo bar ; true' ; run foo || echo quux ; echo $?", i)); + ut_assert_nextline("bar"); + /* The 'true' returns 0 */ + ut_assert_nextline("0"); + ut_assertok(ut_check_console_end(uts)); + + ut_assertok(console_record_reset_enable()); + ut_assertok(run_commandf("setenv foo 'echo bar ; false' ; run foo ; echo $?", i)); + ut_assert_nextline("bar"); + ut_assert_nextline("1"); + ut_assertok(ut_check_console_end(uts)); + + ut_assertok(console_record_reset_enable()); + ut_assertok(run_commandf("setenv foo 'echo bar ; false' ; run foo && echo quux ; echo $?", i)); + ut_assert_nextline("bar"); + ut_assert_nextline("1"); + ut_assertok(ut_check_console_end(uts)); + + ut_assertok(console_record_reset_enable()); + ut_assertok(run_commandf("setenv foo 'echo bar ; false' ; run foo || echo quux ; echo $?", i)); + ut_assert_nextline("bar"); + ut_assert_nextline("quux"); + /* The 'echo quux' returns 0 */ + ut_assert_nextline("0"); + ut_assertok(ut_check_console_end(uts)); + + return 0; +} + +EXIT_TEST(cmd_exit_test, UT_TESTF_CONSOLE_REC); + +int do_ut_exit(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) +{ + struct unit_test *tests = UNIT_TEST_SUITE_START(exit_test); + const int n_ents = UNIT_TEST_SUITE_COUNT(exit_test); + + return cmd_ut_category("cmd_exit", "exit_test_", tests, n_ents, + argc, argv); +} diff --git a/test/cmd_ut.c b/test/cmd_ut.c index 2736582f11c..067bd0828a1 100644 --- a/test/cmd_ut.c +++ b/test/cmd_ut.c @@ -65,6 +65,7 @@ static struct cmd_tbl cmd_ut_sub[] = { #if defined(CONFIG_UT_ENV) U_BOOT_CMD_MKENT(env, CONFIG_SYS_MAXARGS, 1, do_ut_env, "", ""), #endif + U_BOOT_CMD_MKENT(exit, CONFIG_SYS_MAXARGS, 1, do_ut_exit, "", ""), #ifdef CONFIG_CMD_FDT U_BOOT_CMD_MKENT(fdt, CONFIG_SYS_MAXARGS, 1, do_ut_fdt, "", ""), #endif

On Sun, 18 Dec 2022 at 13:47, Marek Vasut marex@denx.de wrote:
Add a test which validates that exit from environment script works as expected, including return value propagation and clipping to positive integers.
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
include/test/suites.h | 1 + test/cmd/Makefile | 2 +- test/cmd/exit.c | 135 ++++++++++++++++++++++++++++++++++++++++++ test/cmd_ut.c | 1 + 4 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 test/cmd/exit.c
Reviewed-by: Simon Glass sjg@chromium.org

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;
#endif #ifndef __U_BOOT__ pi->num_progs = save_num_progs; /* restore number of programs */last_return_code = rcode;
@@ -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;
#ifdef __U_BOOT__ char *p = NULL;int rcode;
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;
#endif setup_string_in_str(&input, s);return rcode == -2 ? last_return_code : rcode; } else {
return parse_stream_outer(&input, flag);
rcode = parse_stream_outer(&input, flag);
#ifdef __U_BOOT__ } #endifreturn rcode == -2 ? last_return_code : rcode;
@@ -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!
participants (3)
-
Hector Palacios
-
Marek Vasut
-
Simon Glass