Re: [U-Boot] [PATCH v4 4/9] Stop using builtin_run_command()
Am Mittwoch 15 Februar 2012, 07:05:24 schrieb Simon Glass: Unfortunately I think that this approach is a little broken. We are running a command sequence from getenv(), but while processing it, we might update the environment variable. Worse, we actually overwrite the newlines in the variable as we process it. Isn't it as easy as copying the current content and processing that instead? -- Michael ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 4/9] Stop using builtin_run_command()
Hi Michael, On Wed, Feb 15, 2012 at 11:44 AM, Michael Walle mich...@walle.cc wrote: Am Mittwoch 15 Februar 2012, 07:05:24 schrieb Simon Glass: Unfortunately I think that this approach is a little broken. We are running a command sequence from getenv(), but while processing it, we might update the environment variable. Worse, we actually overwrite the newlines in the variable as we process it. Isn't it as easy as copying the current content and processing that instead? Yes, that's what I have done - but it is not trivial so did not try to bring it into the old series. Regards, Simon -- Michael ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 4/9] Stop using builtin_run_command()
Hi Mike, On Sat, Feb 4, 2012 at 7:23 PM, Mike Frysinger vap...@gentoo.org wrote: On Saturday 14 January 2012 01:45:52 Simon Glass wrote: This function is only one of the parser options, so use run_command() instead. It knows how to use hush if selected. i can't grok this either :/ -mike Reworded... Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 4/9] Stop using builtin_run_command()
Hi Michael, On Sat, Jan 21, 2012 at 8:07 AM, Michael Walle mich...@walle.cc wrote: Hi Simon, Am Samstag 14 Januar 2012, 07:45:52 schrieb Simon Glass: diff --git a/common/cmd_source.c b/common/cmd_source.c index 481241c..32fff5c 100644 --- a/common/cmd_source.c +++ b/common/cmd_source.c @@ -179,7 +179,7 @@ source (ulong addr, const char *fit_uname) if (*line) { debug (** exec: \%s\\n, line); - if (builtin_run_command(line, 0) 0) { + if (run_command(line, 0) 0) { rcode = 1; break; } @@ -189,7 +189,7 @@ source (ulong addr, const char *fit_uname) ++next; } if (rcode == 0 *line) - rcode = (builtin_run_command(line, 0) = 0); + rcode = (run_command(line, 0) = 0); } #endif free (cmd); Please have a look at this file. There is already a conditional on CONFIG_SYS_HUSH_PARSER, which can be removed, since it is now handled in run_command(). Do not forget to remove the hush.h include, too. OK Could you move this script handling into an own function into common/main.c? This way others can easily execute whole scripts without duplicating this code. I'm planning to submit support for a board which needs exactly that, so there will be at least one board which make use of this. Mike proposed source_commands() as a function name. My original patch named it run_script(). Dunno whats a better name. I will go with run_command_list() so it matches run_command(). Will send an updated series with two new commits which hopefully does what you want. Feel free to reuse/incorporate my patch at http://patchwork.ozlabs.org/patch/137122/ into your patchset ;) Thanks, have done so. -- michael Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 4/9] Stop using builtin_run_command()
Hi Michael, On Tue, Feb 14, 2012 at 2:42 PM, Simon Glass s...@chromium.org wrote: Hi Michael, On Sat, Jan 21, 2012 at 8:07 AM, Michael Walle mich...@walle.cc wrote: Hi Simon, Am Samstag 14 Januar 2012, 07:45:52 schrieb Simon Glass: diff --git a/common/cmd_source.c b/common/cmd_source.c index 481241c..32fff5c 100644 --- a/common/cmd_source.c +++ b/common/cmd_source.c @@ -179,7 +179,7 @@ source (ulong addr, const char *fit_uname) if (*line) { debug (** exec: \%s\\n, line); - if (builtin_run_command(line, 0) 0) { + if (run_command(line, 0) 0) { rcode = 1; break; } @@ -189,7 +189,7 @@ source (ulong addr, const char *fit_uname) ++next; } if (rcode == 0 *line) - rcode = (builtin_run_command(line, 0) = 0); + rcode = (run_command(line, 0) = 0); } #endif free (cmd); Please have a look at this file. There is already a conditional on CONFIG_SYS_HUSH_PARSER, which can be removed, since it is now handled in run_command(). Do not forget to remove the hush.h include, too. OK Could you move this script handling into an own function into common/main.c? This way others can easily execute whole scripts without duplicating this code. I'm planning to submit support for a board which needs exactly that, so there will be at least one board which make use of this. Mike proposed source_commands() as a function name. My original patch named it run_script(). Dunno whats a better name. I will go with run_command_list() so it matches run_command(). Will send an updated series with two new commits which hopefully does what you want. Feel free to reuse/incorporate my patch at http://patchwork.ozlabs.org/patch/137122/ into your patchset ;) Thanks, have done so. Unfortunately I think that this approach is a little broken. We are running a command sequence from getenv(), but while processing it, we might update the environment variable. Worse, we actually overwrite the newlines in the variable as we process it. Since this seems like a bigger change than I thought, and I think it missed the merge window, I have created a two separate patches for this so people can review it properly. Will send through shortly. Regards, Simon -- michael Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 4/9] Stop using builtin_run_command()
On Saturday 14 January 2012 01:45:52 Simon Glass wrote: This function is only one of the parser options, so use run_command() instead. It knows how to use hush if selected. i can't grok this either :/ -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 4/9] Stop using builtin_run_command()
Hi Simon, Am Samstag 14 Januar 2012, 07:45:52 schrieb Simon Glass: diff --git a/common/cmd_source.c b/common/cmd_source.c index 481241c..32fff5c 100644 --- a/common/cmd_source.c +++ b/common/cmd_source.c @@ -179,7 +179,7 @@ source (ulong addr, const char *fit_uname) if (*line) { debug (** exec: \%s\\n, line); - if (builtin_run_command(line, 0) 0) { + if (run_command(line, 0) 0) { rcode = 1; break; } @@ -189,7 +189,7 @@ source (ulong addr, const char *fit_uname) ++next; } if (rcode == 0 *line) - rcode = (builtin_run_command(line, 0) = 0); + rcode = (run_command(line, 0) = 0); } #endif free (cmd); Please have a look at this file. There is already a conditional on CONFIG_SYS_HUSH_PARSER, which can be removed, since it is now handled in run_command(). Do not forget to remove the hush.h include, too. Could you move this script handling into an own function into common/main.c? This way others can easily execute whole scripts without duplicating this code. I'm planning to submit support for a board which needs exactly that, so there will be at least one board which make use of this. Mike proposed source_commands() as a function name. My original patch named it run_script(). Dunno whats a better name. Feel free to reuse/incorporate my patch at http://patchwork.ozlabs.org/patch/137122/ into your patchset ;) -- michael ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v4 4/9] Stop using builtin_run_command()
This function is only one of the parser options, so use run_command() instead. It knows how to use hush if selected. Signed-off-by: Simon Glass s...@chromium.org --- arch/arm/cpu/arm926ejs/kirkwood/cpu.c |7 + board/esd/common/auto_update.c|2 +- board/esd/common/cmd_loadpci.c|2 +- board/esd/du440/du440.c |2 +- common/cmd_bedbug.c |2 +- common/cmd_bootm.c|8 +- common/cmd_source.c |4 +- common/main.c | 45 - include/common.h |1 - 9 files changed, 30 insertions(+), 43 deletions(-) diff --git a/arch/arm/cpu/arm926ejs/kirkwood/cpu.c b/arch/arm/cpu/arm926ejs/kirkwood/cpu.c index 54d15ea..fba5e01 100644 --- a/arch/arm/cpu/arm926ejs/kirkwood/cpu.c +++ b/arch/arm/cpu/arm926ejs/kirkwood/cpu.c @@ -226,12 +226,7 @@ static void kw_sysrst_action(void) } debug(Starting %s process...\n, __FUNCTION__); -#if !defined(CONFIG_SYS_HUSH_PARSER) - ret = builtin_run_command(s, 0); -#else - ret = parse_string_outer(s, FLAG_PARSE_SEMICOLON - | FLAG_EXIT_FROM_LOOP); -#endif + ret = run_command(s, 0); if (ret 0) debug(Error.. %s failed\n, __FUNCTION__); else diff --git a/board/esd/common/auto_update.c b/board/esd/common/auto_update.c index 4cc15fa..fc60545 100644 --- a/board/esd/common/auto_update.c +++ b/board/esd/common/auto_update.c @@ -169,7 +169,7 @@ int au_do_update(int i, long sz) k++; } - builtin_run_command(addr, 0); + run_command(addr, 0); return 0; } diff --git a/board/esd/common/cmd_loadpci.c b/board/esd/common/cmd_loadpci.c index c2bf279..8fcae63 100644 --- a/board/esd/common/cmd_loadpci.c +++ b/board/esd/common/cmd_loadpci.c @@ -110,7 +110,7 @@ int do_loadpci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) * Call run_cmd */ printf(running command at addr 0x%s ...\n, addr); - builtin_run_command((char *)la, 0); + run_command((char *)la, 0); break; default: diff --git a/board/esd/du440/du440.c b/board/esd/du440/du440.c index 75fb200..1ada1bc 100644 --- a/board/esd/du440/du440.c +++ b/board/esd/du440/du440.c @@ -831,7 +831,7 @@ int do_time(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) *d = '\0'; start = get_ticks(); - ret = builtin_run_command(cmd, 0); + ret = run_command(cmd, 0); end = get_ticks(); printf(ticks=%ld\n, (ulong)(end - start)); diff --git a/common/cmd_bedbug.c b/common/cmd_bedbug.c index 0228ee8..5b08123 100644 --- a/common/cmd_bedbug.c +++ b/common/cmd_bedbug.c @@ -237,7 +237,7 @@ void bedbug_main_loop (unsigned long addr, struct pt_regs *regs) if (len == -1) printf (INTERRUPT\n); else - rc = builtin_run_command(lastcommand, flag); + rc = run_command(lastcommand, flag); if (rc = 0) { /* invalid command or not repeatable, forget it */ diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 2e3e159..6bfef62 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -1045,14 +1045,8 @@ int do_bootd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { int rcode = 0; -#ifndef CONFIG_SYS_HUSH_PARSER - if (builtin_run_command(getenv(bootcmd), flag) 0) + if (run_command(getenv(bootcmd), flag) 0) rcode = 1; -#else - if (parse_string_outer(getenv(bootcmd), - FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP) != 0) - rcode = 1; -#endif return rcode; } diff --git a/common/cmd_source.c b/common/cmd_source.c index 481241c..32fff5c 100644 --- a/common/cmd_source.c +++ b/common/cmd_source.c @@ -179,7 +179,7 @@ source (ulong addr, const char *fit_uname) if (*line) { debug (** exec: \%s\\n, line); - if (builtin_run_command(line, 0) 0) { + if (run_command(line, 0) 0) { rcode = 1; break; } @@ -189,7 +189,7 @@ source (ulong addr, const char *fit_uname) ++next; } if (rcode == 0 *line) - rcode = (builtin_run_command(line, 0) = 0); + rcode = (run_command(line, 0) = 0); }