[U-Boot] [PATCH] common, hush [BUG]: exit not work in hush shell

[U-Boot] [PATCH] common, hush [BUG]: exit not work in hush shell

[Heiko Schocher]

running the following script in u-boot:
setenv error 'if true; then
        echo **** ERROR ****
        exit;
fi'

setenv foo echo "****************This should not be printed"
setenv loadubi

setenv updfs 'if true; then
        echo; echo ========== Updating rootfs ==========; echo;
        if run loadubi; then
                echo ***************loadubi
        else;
                run error
        fi
fi'

echo ========== start ==========

run updfs foo
echo **** end

with:

=> source 80008000
========== start ==========

========== Updating rootfs ==========

## Error: "loadubi" not defined
**** ERROR ****
****************This should not be printed
**** end
=>

should not continue after printing "**** ERROR ****", as exit should
skip the hole script. Fix this! I get with this patch:

=> source 80008000
========== start ==========

========== Updating rootfs ==========

## Error: "loadubi" not defined
**** ERROR ****
=>

Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: Tom Rini <trini@ti.com>
Cc: Jeroen Hofstee <JHofstee@victronenergy.com>
---
 common/cmd_source.c |    4 ++++
 common/hush.c       |    4 +++-
 common/main.c       |    7 ++++++-
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/common/cmd_source.c b/common/cmd_source.c
index c4cde98..0d54641 100644
--- a/common/cmd_source.c
+++ b/common/cmd_source.c
@@ -174,6 +174,10 @@ do_source (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 	printf ("## Executing script at %08lx\n", addr);
 	rcode = source (addr, fit_uname);
+	if (rcode == -2) {
+		debug("Hit exit in script\n");
+		rcode = 0;
+	}
 	return rcode;
 }
 
diff --git a/common/hush.c b/common/hush.c
index 4c84c2f..aa76fc7 100644
--- a/common/hush.c
+++ b/common/hush.c
@@ -3192,12 +3192,12 @@ int parse_stream_outer(struct in_str *inp, int flag)
 			code = run_list(ctx.list_head);
 			if (code == -2) {	/* exit */
 				b_free(&temp);
-				code = 0;
 				/* XXX hackish way to not allow exit from main loop */
 				if (inp->peek == file_peek) {
 					printf("exit not allowed from main input shell.\n");
 					continue;
 				}
+				flag |= FLAG_EXIT_FROM_LOOP;
 				break;
 			}
 			if (code == -1)
@@ -3222,6 +3222,8 @@ int parse_stream_outer(struct in_str *inp, int flag)
 #ifndef __U_BOOT__
 	return 0;
 #else
+	if (code == -2)
+		return -2;
 	return (code != 0) ? 1 : 0;
 #endif /* __U_BOOT__ */
 }
diff --git a/common/main.c b/common/main.c
index 81984ac..0cfb6e7 100644
--- a/common/main.c
+++ b/common/main.c
@@ -1469,6 +1469,7 @@ int run_command_list(const char *cmd, int len, int flag)
 int do_run (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 {
 	int i;
+	int ret;
 
 	if (argc < 2)
 		return CMD_RET_USAGE;
@@ -1481,7 +1482,11 @@ int do_run (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 			return 1;
 		}
 
-		if (run_command(arg, flag) != 0)
+		ret = run_command(arg, flag);
+		/* in case we hit an exit in a script */
+		if (ret == -2)
+			return -2;
+		if (ret != 0)
 			return 1;
 	}
 	return 0;
-- 
1.7.7.6

[U-Boot] [PATCH] common, hush [BUG]: exit not work in hush shell

[Wolfgang Denk]

Dear Heiko Schocher,

In message <1347645856-3326-1-git-send-email-hs@denx.de> you wrote:
> running the following script in u-boot:
> setenv error 'if true; then
>         echo **** ERROR ****
>         exit;
> fi'

I think I once tried to fix this, too.

See here:
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/94660
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/94661

Later, I rejected both these patches (and several other, unpublished
versions I came up with during testing), because none of these worked
for all tests really reliable.



>  	rcode = source (addr, fit_uname);
> +	if (rcode == -2) {
> +		debug("Hit exit in script\n");
> +		rcode = 0;
> +	}

Hm... Note that "exit" can take an argment, which should be reflected
in the return code - I mean, would it not be wrong to reurn 0 if the
user has "exit 1" in his script?


Please make sure to test this extensively.  When I was trying to fix a
few similar issues with hush, I quickly gave up because I decided it
would be an easier (and more promising) undertaking to update the whole
hush code to a more recent version of hush.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Don't worry about people stealing your ideas. If your ideas are  any
good, you'll have to ram them down people's throats."  - Howard Aiken

[U-Boot] [PATCH] common, hush [BUG]: exit not work in hush shell

[Heiko Schocher]

Hello Wolfgang,

On 14.09.2012 20:31, Wolfgang Denk wrote:
> Dear Heiko Schocher,
>
> In message<1347645856-3326-1-git-send-email-hs@denx.de>  you wrote:
>> running the following script in u-boot:
>> setenv error 'if true; then
>>          echo **** ERROR ****
>>          exit;
>> fi'
>
> I think I once tried to fix this, too.
>
> See here:
> http://article.gmane.org/gmane.comp.boot-loaders.u-boot/94660
> http://article.gmane.org/gmane.comp.boot-loaders.u-boot/94661
>
> Later, I rejected both these patches (and several other, unpublished
> versions I came up with during testing), because none of these worked

I can understand exactly how you feel... I had also several patches...

> for all tests really reliable.

Do you still have this test somewhere?

>>   	rcode = source (addr, fit_uname);
>> +	if (rcode == -2) {
>> +		debug("Hit exit in script\n");
>> +		rcode = 0;
>> +	}
>
> Hm... Note that "exit" can take an argment, which should be reflected
> in the return code - I mean, would it not be wrong to reurn 0 if the
> user has "exit 1" in his script?

Oh, Ok, this is a killing argument for this patch!

> Please make sure to test this extensively.  When I was trying to fix a
> few similar issues with hush, I quickly gave up because I decided it
> would be an easier (and more promising) undertaking to update the whole
> hush code to a more recent version of hush.

Hmm... Okay, so I have two options:

- fix "exit" with arguments
- update to a recent version of hush (where is the current code from?)
   But I have the gut feeling, that the problem is not in the original
   code, but in the u-boot adaptions, as the code in hush.c has a lot
   of "#ifndef __U_BOOT__" ...

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] common, hush [BUG]: exit not work in hush shell

[Wolfgang Denk]

Dear Heiko,

In message <505561A0.3060108@denx.de> you wrote:
> 
> Do you still have this test somewhere?

Not really.  It was during the development of an install & upgrade
script, and step by step I removed all constructs that the hush shell
would stumble upon.  Sorry.

> Hmm... Okay, so I have two options:
> 
> - fix "exit" with arguments
> - update to a recent version of hush (where is the current code from?)

Hush is part of Busybox, see http://www.busybox.net/source.html

>    But I have the gut feeling, that the problem is not in the original
>    code, but in the u-boot adaptions, as the code in hush.c has a lot
>    of "#ifndef __U_BOOT__" ...

There are also bugs in the old original code, many of them long fixed
since.

I once started looking into updating hush, and I have to admit that I
don't understand the need for all of the "#ifndef __U_BOOT__"
removals.  When hush support was added, and additional 60 kB of code
was a very heavy thing on most systems, so the primary intention then
was to make the code as lightwight as possible.  Today, a much larger
percentage of users is both able to accept such code sizes and looking
for the features offered.  Many would even accept somewhat bigger code
if they could get, for example, support for shell functions etc.  Not
to mention command substitution...

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Thought for the day: What if there were no hypothetical situations?