Patchwork [U-Boot,v3] bootm: Handle errors consistently

login
register
mail settings
Submitter Tom Rini
Date July 10, 2013, 1:25 p.m.
Message ID <1373462733-15054-1-git-send-email-trini@ti.com>
Download mbox | patch
Permalink /patch/258048/
State Accepted
Delegated to: Tom Rini
Headers show

Comments

Tom Rini - July 10, 2013, 1:25 p.m.
From: Simon Glass <sjg@chromium.org>

A recent bootm fix left the error path incomplete. If CONFIG_TRACE is
set it may still not be a supported command, so cover that with the
unsupported subcommand print.  Once we handle BOOTM_STATE_OS_GO, we can
just move into the error handler itself, no need for a goto there.

Signed-off-by: Simon Glass <sjg@chromium.org>
[trini: Update slightly based on Simon's changes to also cover
CONFIG_TRACE/BOOTM_STATE_FAKE_OS_GO]
Signed-off-by: Tom Rini <trini@ti.com>
---
 common/cmd_bootm.c |   21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)
Simon Glass - July 10, 2013, 8:19 p.m.
Hi Tom,

On Wed, Jul 10, 2013 at 6:25 AM, Tom Rini <trini@ti.com> wrote:

> From: Simon Glass <sjg@chromium.org>
>
> A recent bootm fix left the error path incomplete. If CONFIG_TRACE is
> set it may still not be a supported command, so cover that with the
> unsupported subcommand print.  Once we handle BOOTM_STATE_OS_GO, we can
> just move into the error handler itself, no need for a goto there.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> [trini: Update slightly based on Simon's changes to also cover
> CONFIG_TRACE/BOOTM_STATE_FAKE_OS_GO]
> Signed-off-by: Tom Rini <trini@ti.com>
> ---
>  common/cmd_bootm.c |   21 ++++++++-------------
>  1 file changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
> index a783cea..6caa0e9 100644
> --- a/common/cmd_bootm.c
> +++ b/common/cmd_bootm.c
> @@ -705,13 +705,6 @@ static int do_bootm_states(cmd_tbl_t *cmdtp, int
> flag, int argc,
>         if (!ret && (states & BOOTM_STATE_OS_PREP))
>                 ret = boot_fn(BOOTM_STATE_OS_PREP, argc, argv, images);
>
> -       /* Check for unsupported subcommand. */
> -       if (ret) {
> -               puts("subcommand not supported\n");
> -               return ret;
> -       }
> -
> -
>  #ifdef CONFIG_TRACE
>         /* Pretend to run the OS, then run a user command */
>         if (!ret && (states & BOOTM_STATE_OS_FAKE_GO)) {
> @@ -723,15 +716,17 @@ static int do_bootm_states(cmd_tbl_t *cmdtp, int
> flag, int argc,
>                         ret = run_command_list(cmd_list, -1, flag);
>         }
>  #endif
> +
> +       /* Check for unsupported subcommand. */
> +       if (ret) {
> +               puts("subcommand not supported\n");
> +               return ret;
> +       }
> +
>         /* Now run the OS! We hope this doesn't return */
> -       if (!ret && (states & BOOTM_STATE_OS_GO)) {
> +       if (!ret && (states & BOOTM_STATE_OS_GO))
>                 ret = boot_selected_os(argc, argv, BOOTM_STATE_OS_GO,
>                                 images, boot_fn);
> -               if (ret)
> -                       goto err;
> -       }
> -
> -       return ret;
>
>         /* Deal with any fallout */
>  err:
> --
> 1.7.9.5
>
>
Yes I missed your previous change which solved this problem. This patch
looks good to me, thanks.

Regards,
Simon
Tom Rini - July 12, 2013, 9:23 p.m.
On Wed, Jul 10, 2013 at 09:25:33AM -0400, Tom Rini wrote:

> From: Simon Glass <sjg@chromium.org>
> 
> A recent bootm fix left the error path incomplete. If CONFIG_TRACE is
> set it may still not be a supported command, so cover that with the
> unsupported subcommand print.  Once we handle BOOTM_STATE_OS_GO, we can
> just move into the error handler itself, no need for a goto there.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> [trini: Update slightly based on Simon's changes to also cover
> CONFIG_TRACE/BOOTM_STATE_FAKE_OS_GO]
> Signed-off-by: Tom Rini <trini@ti.com>

Applied to u-boot/master, thanks!

Patch

diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index a783cea..6caa0e9 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -705,13 +705,6 @@  static int do_bootm_states(cmd_tbl_t *cmdtp, int flag, int argc,
 	if (!ret && (states & BOOTM_STATE_OS_PREP))
 		ret = boot_fn(BOOTM_STATE_OS_PREP, argc, argv, images);
 
-	/* Check for unsupported subcommand. */
-	if (ret) {
-		puts("subcommand not supported\n");
-		return ret;
-	}
-
-
 #ifdef CONFIG_TRACE
 	/* Pretend to run the OS, then run a user command */
 	if (!ret && (states & BOOTM_STATE_OS_FAKE_GO)) {
@@ -723,15 +716,17 @@  static int do_bootm_states(cmd_tbl_t *cmdtp, int flag, int argc,
 			ret = run_command_list(cmd_list, -1, flag);
 	}
 #endif
+
+	/* Check for unsupported subcommand. */
+	if (ret) {
+		puts("subcommand not supported\n");
+		return ret;
+	}
+
 	/* Now run the OS! We hope this doesn't return */
-	if (!ret && (states & BOOTM_STATE_OS_GO)) {
+	if (!ret && (states & BOOTM_STATE_OS_GO))
 		ret = boot_selected_os(argc, argv, BOOTM_STATE_OS_GO,
 				images, boot_fn);
-		if (ret)
-			goto err;
-	}
-
-	return ret;
 
 	/* Deal with any fallout */
 err: