[U-Boot] [RESEND PATCH 0/3] Fix command repeat

[U-Boot] [RESEND PATCH 0/3] Fix command repeat

[Thomas Betker]

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

[U-Boot] [RESEND PATCH 1/3] Check run_command() return code properly

[Thomas Betker]

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

[U-Boot] [RESEND PATCH 2/3] Add run_command_repeatable()

[Thomas Betker]

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
-- 
1.9.3

[U-Boot] [RESEND PATCH 3/3] Use run_command_repeatable()

[Thomas Betker]

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 */
-- 
1.9.3

[U-Boot] [RESEND PATCH 1/3] Check run_command() return code properly

[Simon Glass]

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>

[U-Boot] [RESEND PATCH 2/3] Add run_command_repeatable()

[Simon Glass]

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>

[U-Boot] [RESEND PATCH 3/3] Use run_command_repeatable()

[Simon Glass]

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>

[U-Boot] [RESEND PATCH 1/3] Check run_command() return code properly

[Simon Glass]

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>

[U-Boot] [RESEND PATCH 1/3] Check run_command() return code properly

[Thomas.Betker at rohde-schwarz.com]

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

[U-Boot] [RESEND PATCH 1/3] Check run_command() return code properly

[Stefan Roese]

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

[U-Boot] [U-Boot, RESEND, 1/3] Check run_command() return code properly

[Tom Rini]

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!

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

[U-Boot] [U-Boot,RESEND,2/3] Add run_command_repeatable()

[Tom Rini]

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!

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

[U-Boot] [U-Boot,RESEND,3/3] Use run_command_repeatable()

[Tom Rini]

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!

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