Re: [U-Boot] [PATCH v4 4/9] Stop using builtin_run_command()

2012-02-15 Thread Michael Walle
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()

2012-02-15 Thread Simon Glass
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()

2012-02-14 Thread Simon Glass
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()

2012-02-14 Thread Simon Glass
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()

2012-02-14 Thread Simon Glass
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()

2012-02-04 Thread Mike Frysinger
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()

2012-01-21 Thread Michael Walle

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()

2012-01-13 Thread Simon Glass
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);
}