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

[Hector Palacios]

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 at denx.de>
> ---
> Cc: Adrian Vovk <avovk at cc-sw.com>
> Cc: Hector Palacios <hector.palacios at digi.com>
> Cc: Pantelis Antoniou <pantelis.antoniou at konsulko.com>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Tom Rini <trini at 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__
> --
> 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 at digi.com>

Thanks!