[PATCH] cli_hush: fix 'exit' cmd that was not exiting scripts

[Marek Vasut]

On 11/18/22 12:19, Hector Palacios wrote:
> Commit 8c4e3b79bd0bb76eea16869e9666e19047c0d005 supposedly
> passed one-level up the argument passed to 'exit' but it also
> broke 'exit' purpose of stopping a script.
> 
> In reality, even if 'do_exit()' is capable of returning any
> integer, the cli only admits '1' or '0' as return values.
> 
> This commit respects the current implementation to allow 'exit'
> to at least return '1' for future processing, but returns
> when the command being run is 'exit'.
> 
> Before this:
> 
> 	=> setenv foo 'echo bar ; exit 3 ; echo should not see this'; run foo; echo $?
> 	bar
> 	should not see this
> 	0
> 	=> setenv foo 'echo bar ; exit 1 ; echo should not see this'; run foo; echo $?
> 	bar
> 	should not see this
> 	0
> 	=> setenv foo 'echo bar ; exit 0 ; echo should not see this'; run foo; echo $?
> 	bar
> 	should not see this
> 	0
> 	=> setenv foo 'echo bar ; exit -1 ; echo should not see this'; run foo; echo $?
> 	bar
> 	should not see this
> 	0
> 	=> setenv foo 'echo bar ; exit -2 ; echo should not see this'; run foo; echo $?
> 	bar
> 	should not see this
> 	0
> 	=> setenv foo 'echo bar ; exit ; echo should not see this'; run foo; echo $?
> 	bar
> 	should not see this
> 	0
> 
> After this:
> 
>          => setenv foo 'echo bar ; exit 3 ; echo should not see this'; run foo; echo $?
>          bar
>          1
>          => setenv foo 'echo bar ; exit 1 ; echo should not see this'; run foo; echo $?
>          bar
>          1
>          => setenv foo 'echo bar ; exit 0 ; echo should not see this'; run foo; echo $?
>          bar
>          0
>          => setenv foo 'echo bar ; exit -1 ; echo should not see this'; run foo; echo $?
>          bar
>          0
>          => setenv foo 'echo bar ; exit -2 ; echo should not see this'; run foo; echo $?
>          bar
>          0
>          => setenv foo 'echo bar ; exit ; echo should not see this'; run foo; echo $?
>          bar
>          0
> 
> Reported-by: Adrian Vovk <avovk at cc-sw.com>
> Signed-off-by: Hector Palacios <hector.palacios at digi.com>
> ---
>   common/cli_hush.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/common/cli_hush.c b/common/cli_hush.c
> index 1467ff81b35b..9fe8b87e02d7 100644
> --- a/common/cli_hush.c
> +++ b/common/cli_hush.c
> @@ -1902,6 +1902,10 @@ static int run_list_real(struct pipe *pi)
>   			last_return_code = -rcode - 2;
>   			return -2;	/* exit */
>   		}
> +		if (!strcmp(pi->progs->argv[0], "exit")) {
> +			last_return_code = rcode;
> +			return rcode;   /* exit */
> +		}
>   		last_return_code=(rcode == 0) ? 0 : 1;
>   #endif
>   #ifndef __U_BOOT__

Looking at the code just above this change 'if (rcode < -1) 
last_return_code = -rcode - 2', that explains the odd 'return -r - 2' in 
cmd/exit.c I think.

I wonder, can we somehow fix the return code handling in cmd/exit.c 
instead, so that it would cover both this behavior listed in this patch, 
and 8c4e3b79bd0 ("cmd: exit: Fix return value") ? The cmd/exit.c seems 
like the right place to fix it.

btw. it would be good to write a unit test for this, since it is 
becoming messy.