Re: [U-Boot] [PATCH v2] main: unify command parsing functions

2012-03-31 Thread Simon Glass
Hi Wolfgang,

On Wed, Mar 7, 2012 at 3:49 AM, Wolfgang Denk  wrote:
> Dear Simon Glass,
>
> In message 
>  you 
> wrote:
>>
>> > These patches haven't been merged yet.
>>
>> Yes - in fact these should go into 'next' I think. But first I need to
>> respond to Wolfgang's email on that series.
>
> See previous mail.
>
> Actually I'd like to pullthes epatches in for this release.  I
> consider especially the multi-line command handling a bug fix.

Sorry I didn't get to this earlier. I decided it needed a bit more work.

I am going to send the 2 patches updated, plus a sandbox compile error
fix that we now need, and a unit test (just as a proposal). See what
you think.

Regards,
Simon

>
> 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: w...@denx.de
> Der Horizont vieler Menschen ist ein Kreis mit Radius Null --
> und das nennen sie ihren Standpunkt.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2] main: unify command parsing functions

2012-03-07 Thread Simon Glass
Hi Wolfgang,

On Wed, Mar 7, 2012 at 3:49 AM, Wolfgang Denk  wrote:
> Dear Simon Glass,
>
> In message 
>  you 
> wrote:
>>
>> > These patches haven't been merged yet.
>>
>> Yes - in fact these should go into 'next' I think. But first I need to
>> respond to Wolfgang's email on that series.
>
> See previous mail.
>
> Actually I'd like to pullthes epatches in for this release.  I
> consider especially the multi-line command handling a bug fix.

OK, let me have another look at it.

Regards,
Simon

>
> 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: w...@denx.de
> Der Horizont vieler Menschen ist ein Kreis mit Radius Null --
> und das nennen sie ihren Standpunkt.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2] main: unify command parsing functions

2012-03-07 Thread Wolfgang Denk
Dear Simon Glass,

In message  
you wrote:
> 
> > These patches haven't been merged yet.
> 
> Yes - in fact these should go into 'next' I think. But first I need to
> respond to Wolfgang's email on that series.

See previous mail.

Actually I'd like to pullthes epatches in for this release.  I
consider especially the multi-line command handling a bug fix.

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: w...@denx.de
Der Horizont vieler Menschen ist ein Kreis mit Radius Null --
und das nennen sie ihren Standpunkt.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2] main: unify command parsing functions

2012-03-07 Thread Wolfgang Denk
Dear Michael Walle,

In message <201203062329.54956.mich...@walle.cc> you wrote:
> 
> This patches is superseded by
>  [PATCH 1/2] Add run_command_list() to run a list of commands
>  [PATCH 2/2] Allow newlines within command environment vars
> by Simon Glass.
> 
> These patches haven't been merged yet.

Thanks for the explanation.

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: w...@denx.de
"He only drinks when he gets depressed." "Why does he get depressed?"
"Sometimes it's because he hasn't had a drink."
 - Terry Pratchett, _Men at Arms_
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2] main: unify command parsing functions

2012-03-06 Thread Simon Glass
Hi,

On Tue, Mar 6, 2012 at 2:29 PM, Michael Walle  wrote:
> Hi Wolfgang,
>
> Am Dienstag 06 März 2012, 21:39:16 schrieb Wolfgang Denk:
>> In message <1326499314-8121-1-git-send-email-mich...@walle.cc> you wrote:
>> > Introduce source_commands() which incorporates run_command2() and parts
>> > of source().
>> >
>> > All command script are now treated the same, that is newlines are
>> > accepted within a command script and variable.
>>
>> Please check if this is still needed after applying Simon Glass'
>> "Unified command execution in one place" series (and if so, please
>> rebase against current master).
>
> This patches is superseded by
>  [PATCH 1/2] Add run_command_list() to run a list of commands
>  [PATCH 2/2] Allow newlines within command environment vars
> by Simon Glass.
>
> These patches haven't been merged yet.
>

Yes - in fact these should go into 'next' I think. But first I need to
respond to Wolfgang's email on that 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 v2] main: unify command parsing functions

2012-03-06 Thread Michael Walle
Hi Wolfgang,

Am Dienstag 06 März 2012, 21:39:16 schrieb Wolfgang Denk:
> In message <1326499314-8121-1-git-send-email-mich...@walle.cc> you wrote:
> > Introduce source_commands() which incorporates run_command2() and parts
> > of source().
> > 
> > All command script are now treated the same, that is newlines are
> > accepted within a command script and variable.
> 
> Please check if this is still needed after applying Simon Glass'
> "Unified command execution in one place" series (and if so, please
> rebase against current master).

This patches is superseded by
 [PATCH 1/2] Add run_command_list() to run a list of commands
 [PATCH 2/2] Allow newlines within command environment vars
by Simon Glass.

These patches haven't been merged yet.

-- 
michael
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2] main: unify command parsing functions

2012-03-06 Thread Wolfgang Denk
Dear Michael Walle,

In message <1326499314-8121-1-git-send-email-mich...@walle.cc> you wrote:
> Introduce source_commands() which incorporates run_command2() and parts of
> source().
> 
> All command script are now treated the same, that is newlines are accepted
> within a command script and variable.

Please check if this is still needed after applying Simon Glass'
"Unified command execution in one place" series (and if so, please
rebase against current master).

Thanks.

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: w...@denx.de
Very ugly or very beautiful women should be flattered on their
understanding, and mediocre ones on their beauty.
   -- Philip Earl of Chesterfield
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2] main: unify command parsing functions

2012-01-20 Thread Simon Glass
Hi Michael,

On Fri, Jan 20, 2012 at 3:37 PM, Michael Walle  wrote:
> Am Freitag 20 Januar 2012, 23:46:51 schrieb Michael Walle:
>> Introduce source_commands() which incorporates run_command2() and parts of
>> source().
>>
>> All command script are now treated the same, that is newlines are accepted
>> within a command script and variable.
>>
>> Signed-off-by: Michael Walle 
>> Cc: Wolfgang Denk 
>> Cc: Mike Frysinger 
>> ---
>> changes v2:
>>  - unconditionally include malloc.h, fixes compiler warning
>
> Actually, this patch is v3. Sorry.

This series seems to touch similar things my command execution
refactor starting here:

http://patchwork.ozlabs.org/patch/136069/

If nothing else, that series looks like it would simplify your patch a
lot. You may have comments also since you have been in the same code.

Regards,
Simon

>
> --
> Michael
> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2] main: unify command parsing functions

2012-01-20 Thread Michael Walle
Am Freitag 20 Januar 2012, 23:46:51 schrieb Michael Walle:
> Introduce source_commands() which incorporates run_command2() and parts of
> source().
> 
> All command script are now treated the same, that is newlines are accepted
> within a command script and variable.
> 
> Signed-off-by: Michael Walle 
> Cc: Wolfgang Denk 
> Cc: Mike Frysinger 
> ---
> changes v2:
>  - unconditionally include malloc.h, fixes compiler warning

Actually, this patch is v3. Sorry.

-- 
Michael
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2] main: unify command parsing functions

2012-01-20 Thread Michael Walle
Introduce source_commands() which incorporates run_command2() and parts of
source().

All command script are now treated the same, that is newlines are accepted
within a command script and variable.

Signed-off-by: Michael Walle 
Cc: Wolfgang Denk 
Cc: Mike Frysinger 
---
changes v2:
 - unconditionally include malloc.h, fixes compiler warning

 common/cmd_pxe.c|2 +-
 common/cmd_source.c |   32 +-
 common/main.c   |   54 +-
 include/common.h|4 +--
 4 files changed, 38 insertions(+), 54 deletions(-)

diff --git a/common/cmd_pxe.c b/common/cmd_pxe.c
index 7c0cb66..347fde1 100644
--- a/common/cmd_pxe.c
+++ b/common/cmd_pxe.c
@@ -537,7 +537,7 @@ static int label_localboot(struct pxe_label *label)
 
printf("running: %s\n", dupcmd);
 
-   ret = run_command2(dupcmd, 0);
+   ret = source_commands(dupcmd, 0);
 
free(dupcmd);
 
diff --git a/common/cmd_source.c b/common/cmd_source.c
index 16a627a..33242df 100644
--- a/common/cmd_source.c
+++ b/common/cmd_source.c
@@ -160,38 +160,8 @@ source (ulong addr, const char *fit_uname)
memmove (cmd, (char *)data, len);
*(cmd + len) = 0;
 
-#ifdef CONFIG_SYS_HUSH_PARSER /*?? */
-   rcode = parse_string_outer (cmd, FLAG_PARSE_SEMICOLON);
-#else
-   {
-   char *line = cmd;
-   char *next = cmd;
+   rcode = source_commands(cmd, 0);
 
-   /*
-* break into individual lines,
-* and execute each line;
-* terminate on error.
-*/
-   while (*next) {
-   if (*next == '\n') {
-   *next = '\0';
-   /* run only non-empty commands */
-   if (*line) {
-   debug ("** exec: \"%s\"\n",
-   line);
-   if (run_command (line, 0) < 0) {
-   rcode = 1;
-   break;
-   }
-   }
-   line = next + 1;
-   }
-   ++next;
-   }
-   if (rcode == 0 && *line)
-   rcode = (run_command(line, 0) >= 0);
-   }
-#endif
free (cmd);
return rcode;
 }
diff --git a/common/main.c b/common/main.c
index e96c95a..7327821 100644
--- a/common/main.c
+++ b/common/main.c
@@ -31,9 +31,7 @@
 #include 
 #include 
 #include 
-#ifdef CONFIG_MODEM_SUPPORT
 #include /* for free() prototype */
-#endif
 
 #ifdef CONFIG_SYS_HUSH_PARSER
 #include 
@@ -266,26 +264,44 @@ int abortboot(int bootdelay)
 # endif/* CONFIG_AUTOBOOT_KEYED */
 #endif /* CONFIG_BOOTDELAY >= 0  */
 
+static inline int _run_command(const char *cmd, int flag)
+{
+#ifndef CONFIG_SYS_HUSH_PARSER
+   return (run_command(cmd, flag) == -1);
+#else
+   flag = FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP;
+   return parse_string_outer(cmd, flag);
+#endif
+}
+
 /*
+ * Run a series of commands separated by '\n'.
+ *
  * Return 0 on success, or != 0 on error.
  */
-#ifndef CONFIG_CMD_PXE
-static inline
-#endif
-int run_command2(const char *cmd, int flag)
+int source_commands(const char *commands, int flag)
 {
-#ifndef CONFIG_SYS_HUSH_PARSER
+   int rcode = 0;
+   char *_commands = strdup(commands);
+   char *stringp = _commands;
+   char *line;
+
/*
-* run_command can return 0 or 1 for success, so clean up its result.
+* break into individual lines and execute each line;
+* terminate on error.
 */
-   if (run_command(cmd, flag) == -1)
-   return 1;
+   while ((line = strsep(&stringp, "\n"))) {
+   /* skip empty lines */
+   if (*line == '\0')
+   continue;
+   if (_run_command(line, flag)) {
+   rcode = 1;
+   break;
+   }
+   }
 
-   return 0;
-#else
-   return parse_string_outer(cmd,
-   FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP);
-#endif
+   free(_commands);
+   return rcode;
 }
 
 //
@@ -354,7 +370,7 @@ void main_loop (void)
int prev = disable_ctrlc(1);/* disable Control C checking */
 # endif
 
-   run_command2(p, 0);
+   source_commands(p, 0);
 
 # ifdef CONFIG_AUTOBOOT_KEYED
disable_ctrlc(prev);/* restore Control C checking */
@@ -399,7 +415,7 @@ void main_loop (void)
int prev = disable_ctrlc(1);/* disable Control C checking */
 # endif
 
-   run_command2(s, 0);
+   source_commands(s, 0);
 
 # if

[U-Boot] [PATCH v2] main: unify command parsing functions

2012-01-13 Thread Michael Walle
Introduce source_commands() which incorporates run_command2() and parts of
source().

All command script are now treated the same, that is newlines are accepted
within a command script and variable.

Signed-off-by: Michael Walle 
Cc: Wolfgang Denk 
Cc: Mike Frysinger 
---

This patch supersedes "cmd_source: introduce run_script()"
v2:
 - argument is now const char*
 - unify run_commands2() and source()
 - allow newlines in command variables
 - use strsep()

 common/cmd_pxe.c|2 +-
 common/cmd_source.c |   32 +--
 common/main.c   |   52 ++
 include/common.h|4 +--
 4 files changed, 38 insertions(+), 52 deletions(-)

diff --git a/common/cmd_pxe.c b/common/cmd_pxe.c
index 7c0cb66..347fde1 100644
--- a/common/cmd_pxe.c
+++ b/common/cmd_pxe.c
@@ -537,7 +537,7 @@ static int label_localboot(struct pxe_label *label)
 
printf("running: %s\n", dupcmd);
 
-   ret = run_command2(dupcmd, 0);
+   ret = source_commands(dupcmd, 0);
 
free(dupcmd);
 
diff --git a/common/cmd_source.c b/common/cmd_source.c
index 16a627a..33242df 100644
--- a/common/cmd_source.c
+++ b/common/cmd_source.c
@@ -160,38 +160,8 @@ source (ulong addr, const char *fit_uname)
memmove (cmd, (char *)data, len);
*(cmd + len) = 0;
 
-#ifdef CONFIG_SYS_HUSH_PARSER /*?? */
-   rcode = parse_string_outer (cmd, FLAG_PARSE_SEMICOLON);
-#else
-   {
-   char *line = cmd;
-   char *next = cmd;
+   rcode = source_commands(cmd, 0);
 
-   /*
-* break into individual lines,
-* and execute each line;
-* terminate on error.
-*/
-   while (*next) {
-   if (*next == '\n') {
-   *next = '\0';
-   /* run only non-empty commands */
-   if (*line) {
-   debug ("** exec: \"%s\"\n",
-   line);
-   if (run_command (line, 0) < 0) {
-   rcode = 1;
-   break;
-   }
-   }
-   line = next + 1;
-   }
-   ++next;
-   }
-   if (rcode == 0 && *line)
-   rcode = (run_command(line, 0) >= 0);
-   }
-#endif
free (cmd);
return rcode;
 }
diff --git a/common/main.c b/common/main.c
index e96c95a..cc25d38 100644
--- a/common/main.c
+++ b/common/main.c
@@ -266,26 +266,44 @@ int abortboot(int bootdelay)
 # endif/* CONFIG_AUTOBOOT_KEYED */
 #endif /* CONFIG_BOOTDELAY >= 0  */
 
+static inline int _run_command(const char *cmd, int flag)
+{
+#ifndef CONFIG_SYS_HUSH_PARSER
+   return (run_command(cmd, flag) == -1);
+#else
+   flag = FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP;
+   return parse_string_outer(cmd, flag);
+#endif
+}
+
 /*
+ * Run a series of commands separated by '\n'.
+ *
  * Return 0 on success, or != 0 on error.
  */
-#ifndef CONFIG_CMD_PXE
-static inline
-#endif
-int run_command2(const char *cmd, int flag)
+int source_commands(const char *commands, int flag)
 {
-#ifndef CONFIG_SYS_HUSH_PARSER
+   int rcode = 0;
+   char *_commands = strdup(commands);
+   char *stringp = _commands;
+   char *line;
+
/*
-* run_command can return 0 or 1 for success, so clean up its result.
+* break into individual lines and execute each line;
+* terminate on error.
 */
-   if (run_command(cmd, flag) == -1)
-   return 1;
+   while ((line = strsep(&stringp, "\n"))) {
+   /* skip empty lines */
+   if (*line == '\0')
+   continue;
+   if (_run_command(line, flag)) {
+   rcode = 1;
+   break;
+   }
+   }
 
-   return 0;
-#else
-   return parse_string_outer(cmd,
-   FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP);
-#endif
+   free(_commands);
+   return rcode;
 }
 
 //
@@ -354,7 +372,7 @@ void main_loop (void)
int prev = disable_ctrlc(1);/* disable Control C checking */
 # endif
 
-   run_command2(p, 0);
+   source_commands(p, 0);
 
 # ifdef CONFIG_AUTOBOOT_KEYED
disable_ctrlc(prev);/* restore Control C checking */
@@ -399,7 +417,7 @@ void main_loop (void)
int prev = disable_ctrlc(1);/* disable Control C checking */
 # endif
 
-   run_command2(s, 0);
+   source_commands(s, 0);
 
 # ifdef CONFIG_AUTOBOOT_KEYED
disable_ctrlc(prev);