[U-Boot] [RESEND PATCH 0/3] Fix command repeat
I am resending the patches because they didn't make it to Patchwork the first time, probably due to a configuration issue on my side.
The intent of these patches is to get command repeat to work again. Currently, successful commands won't be repeated but failed commands will -- neither is as expected.
The issue is that run_command() returns 0 on success, 1 on error. In order to get command repeat to work, we need a variant which returns -1 on error and 0/1 (non-repeatable/repeatable) on succcess, the same way as cli_simple_run_command() does.
Patch 2 adds the run_command_repeatable() function, and patch 3 replaces run_command() by run_command_repeatable() where necessary.
Patch 1 is a cleanup of places which call run_command(), but expect it to return -1 on error. This is actually independent of the other two patches -- it just came up when checking run_command() invocations in general.
Best regards, Thomas Betker
Thomas Betker (3): Check run_command() return code properly Add run_command_repeatable() Use run_command_repeatable()
arch/arm/cpu/arm926ejs/kirkwood/cpu.c | 2 +- board/gdsys/p1022/controlcenterd.c | 6 +----- common/cli.c | 24 ++++++++++++++++++++++++ common/cli_simple.c | 2 +- common/cmd_bedbug.c | 2 +- common/cmd_bootm.c | 6 +----- include/common.h | 1 + 7 files changed, 30 insertions(+), 13 deletions(-)
run_command() returns 0 for success, 1 for failure. Fix places which assume that failure is indicated by a negative return code.
Signed-off-by: Thomas Betker thomas.betker@rohde-schwarz.com --- arch/arm/cpu/arm926ejs/kirkwood/cpu.c | 2 +- board/gdsys/p1022/controlcenterd.c | 6 +----- common/cmd_bootm.c | 6 +----- 3 files changed, 3 insertions(+), 11 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/kirkwood/cpu.c b/arch/arm/cpu/arm926ejs/kirkwood/cpu.c index 0937506..da80240 100644 --- a/arch/arm/cpu/arm926ejs/kirkwood/cpu.c +++ b/arch/arm/cpu/arm926ejs/kirkwood/cpu.c @@ -210,7 +210,7 @@ static void kw_sysrst_action(void)
debug("Starting %s process...\n", __FUNCTION__); ret = run_command(s, 0); - if (ret < 0) + if (ret != 0) debug("Error.. %s failed\n", __FUNCTION__); else debug("%s process finished\n", __FUNCTION__); diff --git a/board/gdsys/p1022/controlcenterd.c b/board/gdsys/p1022/controlcenterd.c index 8ccd9ce..642b807 100644 --- a/board/gdsys/p1022/controlcenterd.c +++ b/board/gdsys/p1022/controlcenterd.c @@ -221,11 +221,7 @@ void hw_watchdog_reset(void) #ifdef CONFIG_TRAILBLAZER int do_bootd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { - int rcode = 0; - - if (run_command(getenv("bootcmd"), flag) < 0) - rcode = 1; - return rcode; + return run_command(getenv("bootcmd"), flag); }
int board_early_init_r(void) diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 449bb36..745fc65 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -1079,11 +1079,7 @@ U_BOOT_CMD( #if defined(CONFIG_CMD_BOOTD) int do_bootd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { - int rcode = 0; - - if (run_command(getenv("bootcmd"), flag) < 0) - rcode = 1; - return rcode; + return run_command(getenv("bootcmd"), flag); }
U_BOOT_CMD(
On 5 June 2014 12:07, Thomas Betker thomas.betker@freenet.de wrote:
run_command() returns 0 for success, 1 for failure. Fix places which assume that failure is indicated by a negative return code.
Signed-off-by: Thomas Betker thomas.betker@rohde-schwarz.com
This has come through fine to patchwork - it may have been your missing name that was the problem.
Acked-by: Simon Glass sjg@chromium.org
On 5 June 2014 12:14, Simon Glass sjg@chromium.org wrote:
On 5 June 2014 12:07, Thomas Betker thomas.betker@freenet.de wrote:
run_command() returns 0 for success, 1 for failure. Fix places which assume that failure is indicated by a negative return code.
Signed-off-by: Thomas Betker thomas.betker@rohde-schwarz.com
This has come through fine to patchwork - it may have been your missing name that was the problem.
Acked-by: Simon Glass sjg@chromium.org
Tested-by: Simon Glass sjg@chromium.org
Hello Simon:
run_command() returns 0 for success, 1 for failure. Fix places which assume that failure is indicated by a negative return code.
Signed-off-by: Thomas Betker thomas.betker@rohde-schwarz.com
This has come through fine to patchwork - it may have been your missing name that was the problem.
Yes, I checked that the patches made it to Patchwork last night. Thanks for the hint; you were right, it was the missing name in my .gitconfig.
Best regards, Thomas
On 05.06.2014 20:07, Thomas Betker wrote:
run_command() returns 0 for success, 1 for failure. Fix places which assume that failure is indicated by a negative return code.
Signed-off-by: Thomas Betker thomas.betker@rohde-schwarz.com
Thanks for fixing this!
Tested-by: Stefan Roese sr@denx.de
Thanks, Stefan
On Thu, Jun 05, 2014 at 08:07:56PM +0200, Thomas Betker wrote:
run_command() returns 0 for success, 1 for failure. Fix places which assume that failure is indicated by a negative return code.
Signed-off-by: Thomas Betker thomas.betker@rohde-schwarz.com Acked-by: Simon Glass sjg@chromium.org Tested-by: Simon Glass sjg@chromium.org Tested-by: Stefan Roese sr@denx.de
Applied to u-boot/master, thanks!
run_command() returns 0 on success and 1 on error. However, there are some invocations which expect 0 or 1 for success (not repeatable or repeatable) and -1 for error; add run_command_repeatable() for this purpose.
Signed-off-by: Thomas Betker thomas.betker@rohde-schwarz.com --- common/cli.c | 24 ++++++++++++++++++++++++ include/common.h | 1 + 2 files changed, 25 insertions(+)
diff --git a/common/cli.c b/common/cli.c index ea6bfb3..272b028 100644 --- a/common/cli.c +++ b/common/cli.c @@ -41,6 +41,30 @@ int run_command(const char *cmd, int flag) #endif }
+/* + * Run a command using the selected parser, and check if it is repeatable. + * + * @param cmd Command to run + * @param flag Execution flags (CMD_FLAG_...) + * @return 0 (not repeatable) or 1 (repeatable) on success, -1 on error. + */ +int run_command_repeatable(const char *cmd, int flag) +{ +#ifndef CONFIG_SYS_HUSH_PARSER + return cli_simple_run_command(cmd, flag); +#else + /* + * parse_string_outer() returns 1 for failure, so clean up + * its result. + */ + if (parse_string_outer(cmd, + FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP)) + return -1; + + return 0; +#endif +} + int run_command_list(const char *cmd, int len, int flag) { int need_buff = 1; diff --git a/include/common.h b/include/common.h index 91dc0f3..cc74633 100644 --- a/include/common.h +++ b/include/common.h @@ -271,6 +271,7 @@ int print_buffer(ulong addr, const void *data, uint width, uint count, /* common/main.c */ void main_loop (void); int run_command(const char *cmd, int flag); +int run_command_repeatable(const char *cmd, int flag);
/** * Run a list of commands separated by ; or even \0
On 5 June 2014 12:07, Thomas Betker thomas.betker@freenet.de wrote:
run_command() returns 0 on success and 1 on error. However, there are some invocations which expect 0 or 1 for success (not repeatable or repeatable) and -1 for error; add run_command_repeatable() for this purpose.
Signed-off-by: Thomas Betker thomas.betker@rohde-schwarz.com
I feel that a separate function is fine for now.
Acked-by: Simon Glass sjg@chromium.org Tested-by: Simon Glass sjg@chromium.org
On Thu, Jun 05, 2014 at 08:07:57PM +0200, Thomas Betker wrote:
run_command() returns 0 on success and 1 on error. However, there are some invocations which expect 0 or 1 for success (not repeatable or repeatable) and -1 for error; add run_command_repeatable() for this purpose.
Signed-off-by: Thomas Betker thomas.betker@rohde-schwarz.com Acked-by: Simon Glass sjg@chromium.org Tested-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
Replace run_command() by run_command_repeatable() in places which depend on the return code to indicate repeatability.
Signed-off-by: Thomas Betker thomas.betker@rohde-schwarz.com --- common/cli_simple.c | 2 +- common/cmd_bedbug.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/cli_simple.c b/common/cli_simple.c index 413c2eb..222e292 100644 --- a/common/cli_simple.c +++ b/common/cli_simple.c @@ -295,7 +295,7 @@ void cli_simple_loop(void) if (len == -1) puts("<INTERRUPT>\n"); else - rc = run_command(lastcommand, flag); + rc = run_command_repeatable(lastcommand, flag);
if (rc <= 0) { /* invalid command or not repeatable, forget it */ diff --git a/common/cmd_bedbug.c b/common/cmd_bedbug.c index bdcf712..57a8a3f 100644 --- a/common/cmd_bedbug.c +++ b/common/cmd_bedbug.c @@ -238,7 +238,7 @@ void bedbug_main_loop (unsigned long addr, struct pt_regs *regs) if (len == -1) printf ("<INTERRUPT>\n"); else - rc = run_command(lastcommand, flag); + rc = run_command_repeatable(lastcommand, flag);
if (rc <= 0) { /* invalid command or not repeatable, forget it */
On 5 June 2014 12:07, Thomas Betker thomas.betker@freenet.de wrote:
Replace run_command() by run_command_repeatable() in places which depend on the return code to indicate repeatability.
Signed-off-by: Thomas Betker thomas.betker@rohde-schwarz.com
Acked-by: Simon Glass sjg@chromium.org Tested-by: Simon Glass sjg@chromium.org
On Thu, Jun 05, 2014 at 08:07:58PM +0200, Thomas Betker wrote:
Replace run_command() by run_command_repeatable() in places which depend on the return code to indicate repeatability.
Signed-off-by: Thomas Betker thomas.betker@rohde-schwarz.com Acked-by: Simon Glass sjg@chromium.org Tested-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
participants (5)
-
Simon Glass -
Stefan Roese -
Thomas Betker -
Thomas.Betker＠rohde-schwarz.com -
Tom Rini