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(-)

1.9.3

run_command() returns 0 for success, 1 for failure. Fix places which assume that failure is indicated by a negative return code.

---
 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(-)

index 0937506..da80240 100644
@@ -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__);

index 8ccd9ce..642b807 100644
@@ -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)

index 449bb36..745fc65 100644
@@ -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(

1.9.3

This has come through fine to patchwork - it may have been your missing name that was the problem.

Hello Simon:

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

Thanks for fixing this!

Thanks,
Stefan

Applied to u-boot/master, thanks!

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140611/27a03037/attachment.pgp>

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.

---
 common/cli.c     | 24 ++++++++++++++++++++++++
 include/common.h |  1 +
 2 files changed, 25 insertions(+)

index ea6bfb3..272b028 100644
@@ -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;

index 91dc0f3..cc74633 100644
@@ -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

1.9.3

I feel that a separate function is fine for now.

Applied to u-boot/master, thanks!

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140611/10d66915/attachment.pgp>

Replace run_command() by run_command_repeatable() in places which depend on the return code to indicate repeatability.

---
 common/cli_simple.c | 2 +-
 common/cmd_bedbug.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

index 413c2eb..222e292 100644
@@ -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 */

index bdcf712..57a8a3f 100644
@@ -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 */

1.9.3

Applied to u-boot/master, thanks!

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140611/f6f444bb/attachment.pgp>