| Message ID | 1347645856-3326-1-git-send-email-hs@denx.de |
|---|---|
| State | Rejected |
| Delegated to: | Wolfgang Denk |
| Headers | show |
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
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
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
========== 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;
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