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

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

[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 <michael@walle.cc>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Mike Frysinger <vapier@gentoo.org>
---

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);	/* restore Control C checking */
@@ -410,7 +428,7 @@ void main_loop (void)
 	if (menukey == CONFIG_MENUKEY) {
 		s = getenv("menucmd");
 		if (s)
-			run_command2(s, 0);
+			source_commands(s, 0);
 	}
 #endif /* CONFIG_MENUKEY */
 #endif /* CONFIG_BOOTDELAY */
@@ -1408,7 +1426,7 @@ int do_run (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 			return 1;
 		}
 
-		if (run_command2(arg, flag) != 0)
+		if (source_commands(arg, flag) != 0)
 			return 1;
 	}
 	return 0;
diff --git a/include/common.h b/include/common.h
index 3df1def..c2e7813 100644
--- a/include/common.h
+++ b/include/common.h
@@ -261,9 +261,7 @@ int	print_buffer (ulong addr, void* data, uint width, uint count, uint linelen);
 /* common/main.c */
 void	main_loop	(void);
 int	run_command	(const char *cmd, int flag);
-#ifdef CONFIG_CMD_PXE
-int run_command2(const char *cmd, int flag);
-#endif
+int source_commands(const char *cmd, int flag);
 int	readline	(const char *const prompt);
 int	readline_into_buffer	(const char *const prompt, char * buffer);
 int	parse_line (char *, char *[]);
-- 
1.7.2.5

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

[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 <michael@walle.cc>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Mike Frysinger <vapier@gentoo.org>
---
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 <watchdog.h>
 #include <command.h>
 #include <version.h>
-#ifdef CONFIG_MODEM_SUPPORT
 #include <malloc.h>		/* for free() prototype */
-#endif
 
 #ifdef CONFIG_SYS_HUSH_PARSER
 #include <hush.h>
@@ -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);
 
 # ifdef CONFIG_AUTOBOOT_KEYED
 		disable_ctrlc(prev);	/* restore Control C checking */
@@ -410,7 +426,7 @@ void main_loop (void)
 	if (menukey == CONFIG_MENUKEY) {
 		s = getenv("menucmd");
 		if (s)
-			run_command2(s, 0);
+			source_commands(s, 0);
 	}
 #endif /* CONFIG_MENUKEY */
 #endif /* CONFIG_BOOTDELAY */
@@ -1408,7 +1424,7 @@ int do_run (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 			return 1;
 		}
 
-		if (run_command2(arg, flag) != 0)
+		if (source_commands(arg, flag) != 0)
 			return 1;
 	}
 	return 0;
diff --git a/include/common.h b/include/common.h
index 3df1def..c2e7813 100644
--- a/include/common.h
+++ b/include/common.h
@@ -261,9 +261,7 @@ int	print_buffer (ulong addr, void* data, uint width, uint count, uint linelen);
 /* common/main.c */
 void	main_loop	(void);
 int	run_command	(const char *cmd, int flag);
-#ifdef CONFIG_CMD_PXE
-int run_command2(const char *cmd, int flag);
-#endif
+int source_commands(const char *cmd, int flag);
 int	readline	(const char *const prompt);
 int	readline_into_buffer	(const char *const prompt, char * buffer);
 int	parse_line (char *, char *[]);
-- 
1.7.2.5

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

[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 <michael@walle.cc>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Mike Frysinger <vapier@gentoo.org>
> ---
> changes v2:
>  - unconditionally include malloc.h, fixes compiler warning

Actually, this patch is v3. Sorry.

-- 
Michael

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

[Simon Glass]

Hi Michael,

On Fri, Jan 20, 2012 at 3:37 PM, Michael Walle <michael@walle.cc> 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 <michael@walle.cc>
>> Cc: Wolfgang Denk <wd@denx.de>
>> Cc: Mike Frysinger <vapier@gentoo.org>
>> ---
>> 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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

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

[Wolfgang Denk]

Dear Michael Walle,

In message <1326499314-8121-1-git-send-email-michael@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: wd at 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] [PATCH v2] main: unify command parsing functions

[Michael Walle]

Hi Wolfgang,

Am Dienstag 06 M?rz 2012, 21:39:16 schrieb Wolfgang Denk:
> In message <1326499314-8121-1-git-send-email-michael@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] [PATCH v2] main: unify command parsing functions

[Simon Glass]

Hi,

On Tue, Mar 6, 2012 at 2:29 PM, Michael Walle <michael@walle.cc> wrote:
> Hi Wolfgang,
>
> Am Dienstag 06 M?rz 2012, 21:39:16 schrieb Wolfgang Denk:
>> In message <1326499314-8121-1-git-send-email-michael@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] [PATCH v2] main: unify command parsing functions

[Wolfgang Denk]

Dear Michael Walle,

In message <201203062329.54956.michael@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: wd at 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] [PATCH v2] main: unify command parsing functions

[Wolfgang Denk]

Dear Simon Glass,

In message <CAPnjgZ1JwBM=GSR87jU79zZzm0Xv2QCQbh5=VoFRh-1B5y1jLQ@mail.gmail.com> 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: wd at denx.de
Der Horizont vieler Menschen ist ein Kreis mit Radius Null --
und das nennen sie ihren Standpunkt.

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

[Simon Glass]

Hi Wolfgang,

On Wed, Mar 7, 2012 at 3:49 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon Glass,
>
> In message <CAPnjgZ1JwBM=GSR87jU79zZzm0Xv2QCQbh5=VoFRh-1B5y1jLQ@mail.gmail.com> 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: wd at denx.de
> Der Horizont vieler Menschen ist ein Kreis mit Radius Null --
> und das nennen sie ihren Standpunkt.

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

[Simon Glass]

Hi Wolfgang,

On Wed, Mar 7, 2012 at 3:49 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon Glass,
>
> In message <CAPnjgZ1JwBM=GSR87jU79zZzm0Xv2QCQbh5=VoFRh-1B5y1jLQ@mail.gmail.com> 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: wd at denx.de
> Der Horizont vieler Menschen ist ein Kreis mit Radius Null --
> und das nennen sie ihren Standpunkt.