Add run_command_list() to run a list of commands

The environment variables preboot, bootcmd and menucmd can hold a command to execute. This change permits these variables to have newlines so that they work the same as the 'source' command.

Signed-off-by Simon Glass <[email protected]>

---
 common/main.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

Dear Simon Glass,

Quoted 4 lines · In message <[email protected]> you wrote:

The environment variables preboot, bootcmd and menucmd can hold a command to execute. This change permits these variables to have newlines so that they work the same as the 'source' command.

Quoted 4 lines

Signed-off-by Simon Glass <[email protected]>

---
 common/main.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

Please squash this into the patch that introduces run_command_list() [assuming this is the patch which breaks behaviour?]

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 The word "fit", as I understand it, means "appropriate to a purpose", and I would say the body of the Dean is supremely appropriate to the purpose of sitting around all day and eating big heavy meals.
- Terry Pratchett, _Moving Pictures_

Quoted 11 lines

Quoted 8 lines · In message <[email protected]> you wrote:

The environment variables preboot, bootcmd and menucmd can hold a command to execute. This change permits these variables to have newlines so that they work the same as the 'source' command.

Signed-off-by Simon Glass <[email protected]>

---
 common/main.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

Quoted 3 lines

Please squash this into the patch that introduces run_command_list() [assuming this is the patch which breaks behaviour?]

The first patch is just a clean-up and should not change any behaviour. This second patch changes the behaviour of the named env variables.

Dear Simon Glass,

Quoted 11 lines · In message <CAPnjgZ0XueZukodaNSdQSYvuevyiH5Z-=3-2pkcC7OekddG-JA@mail.gmail.com>…

Quoted 11 lines

Quoted 8 lines · In message <[email protected]> you wrote:

The environment variables preboot, bootcmd and menucmd can hold a command to execute. This change permits these variables to have newlines so that they work the same as the 'source' command.

Signed-off-by Simon Glass <[email protected]>

---
?common/main.c | ? ?6 +++---
?1 files changed, 3 insertions(+), 3 deletions(-)

Quoted 6 lines

Quoted 3 lines

Please squash this into the patch that introduces run_command_list() [assuming this is the patch which breaks behaviour?]

The first patch is just a clean-up and should not change any behaviour. This second patch changes the behaviour of the named env variables.

So which commit introduced the breakage, then?

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 A stone was placed at a ford in a river with the inscription:
"When this stone is covered it is dangerous to ford here."

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

The environment variables preboot, bootcmd and menucmd can hold a command to execute. This change permits these variables to have newlines so that they work the same as the 'source' command.

Quoted 4 lines

Quoted 2 lines

The first patch is just a clean-up and should not change any behaviour. This second patch changes the behaviour of the named env variables.

So which commit introduced the breakage, then?

Which breakage are you referring to? The intent of these two patches is to add a new feature (proposed by Michael Walle <[email protected]> who also provided a patch) on top of the command refactor series.

Sorry, there is some confusion here but I'm not sure what it is.

Dear Simon Glass,

In message <CAPnjgZ1BMMrN-+bHFr9GcH-5wF4WSz_7Qa53C_cHPLEbL2ycqA@mail.gmail.com> you wrote:
...

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

The environment variables preboot, bootcmd and menucmd can hold a command to execute. This change permits these variables to have newlines so that they work the same as the 'source' command.

...

Quoted 10 lines

Quoted 4 lines

Quoted 2 lines

The first patch is just a clean-up and should not change any behaviour. This second patch changes the behaviour of the named env variables.

So which commit introduced the breakage, then?

Which breakage are you referring to? The intent of these two patches is to add a new feature (proposed by Michael Walle <[email protected]> who also provided a patch) on top of the command refactor series.

Sorry, there is some confusion here but I'm not sure what it is.

Guess I'm confused.

I don't understand in which way the variables listed here (preboot, bootcmd and menucmd) are special - actually all variables can hold commands, that then can be executed using the "run" command.

If newline is a valid command separator (and I think it is), this should work for _all_ variable.

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 Backed up the system lately?

The environment variables preboot, bootcmd and menucmd can hold a command to execute. This change permits these variables to have newlines so that they work the same as the 'source' command.

This new function runs a list of commands separated by semicolon. We move this out of cmd_source so that it can be used by other code. The PXE also uses the new function.

Suggested-by Michael Walle <[email protected]>

Signed-off-by Simon Glass <[email protected]>

---
 common/cmd_pxe.c    |   20 ++------------
 common/cmd_source.c |   49 +----------------------------------
 common/main.c       |   72
+++++++++++++++++++++++++++++++++++++++++++++++++++ include/common.h    | 
  1 +
 4 files changed, 77 insertions(+), 65 deletions(-)

common/main.c +26 -0

index db181d3..87f2855 100644
@@ -30,6 +30,7 @@
 #include <common.h>
 #include <watchdog.h>
 #include <command.h>
+#include <malloc.h>
 #include <version.h>
 #ifdef CONFIG_MODEM_SUPPORT
 #include <malloc.h>		/* for free() prototype */

@@ -1373,6 +1374,77 @@ int run_command(const char *cmd, int flag)
 #endif
 }
 
+#ifndef CONFIG_SYS_HUSH_PARSER
+static int builtin_run_command_list(char *cmd, int flag)
+{
+	char *line, *next;
+	int rcode = 0;
+
+	/*
+	 * Break into individual lines, and execute each line; terminate on
+	 * error.
+	 */
+	line = next = cmd;
+	while (*next) {
+		if (*next == '\n') {
+			*next = '\0';
+			/* run only non-empty commands */
+			if (*line) {
+				debug("** exec: \"%s\"\n", line);
+				if (builtin_run_command(line, 0) < 0) {
+					rcode = 1;
+					break;
+				}
+			}
+			line = next + 1;
+		}
+		++next;

common/main.c +2 -0

+ *
+ * Note that if 'len' is not -1, then the command may not be \0

terminated, + * Memory will be allocated for the command in that case.
+ *
+ * @param cmd List of commands to run, each separated bu semicolon
+ * @param len Length of command excluding terminator if known (-1 if

uint count, uint linelen); /* common/main.c */
void main_loop (void);
int run_command(const char *cmd, int flag);

include/common.h +1 -0

+int run_command_list(const char *cmd, int len, int flag);
 int	readline	(const char *const prompt);
 int	readline_into_buffer(const char *const prompt, char *buffer,
 			int timeout);

Am Mittwoch 15 Februar 2012, 07:07:09 schrieb Simon Glass:

Quoted 14 lines

This new function runs a list of commands separated by semicolon. We move this out of cmd_source so that it can be used by other code. The PXE also uses the new function.

Suggested-by Michael Walle <[email protected]>

Signed-off-by Simon Glass <[email protected]>

---

?common/cmd_pxe.c ? ?| ? 20 ++------------
?common/cmd_source.c | ? 49 +----------------------------------
?common/main.c ? ? ? | ? 72

+++++++++++++++++++++++++++++++++++++++++++++++++++ include/common.h ? ?| ? 1 +
?4 files changed, 77 insertions(+), 65 deletions(-)

[..snip..]

Quoted 44 lines

common/main.c +0 -0

index db181d3..87f2855 100644
@@ -30,6 +30,7 @@

?#include <common.h>
?#include <watchdog.h>
?#include <command.h>
+#include <malloc.h>
?#include <version.h>
?#ifdef CONFIG_MODEM_SUPPORT
?#include <malloc.h> ? ? ? ? ?/* for free() prototype */

common/main.c +0 -0

@@ -1373,6 +1374,77 @@ int run_command(const char *cmd, int flag)

?#endif
?}

common/main.c +28 -0

+#ifndef CONFIG_SYS_HUSH_PARSER
+static int builtin_run_command_list(char *cmd, int flag)
+{
+ ? ? char *line, *next;
+ ? ? int rcode = 0;
+
+ ? ? /*
+ ? ? ?* Break into individual lines, and execute each line; terminate on
+ ? ? ?* error.
+ ? ? ?*/
+ ? ? line = next = cmd;
+ ? ? while (*next) {
+ ? ? ? ? ? ? if (*next == '\n') {
+ ? ? ? ? ? ? ? ? ? ? *next = '\0';
+ ? ? ? ? ? ? ? ? ? ? /* run only non-empty commands */
+ ? ? ? ? ? ? ? ? ? ? if (*line) {
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? debug("** exec: \"%s\"\n", line);
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? if (builtin_run_command(line, 0) < 0) {
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? rcode = 1;
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? break;
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? }
+ ? ? ? ? ? ? ? ? ? ? }
+ ? ? ? ? ? ? ? ? ? ? line = next + 1;
+ ? ? ? ? ? ? }
+ ? ? ? ? ? ? ++next;
+ ? ? }
+ ? ? if (rcode == 0 && *cmd)
+ ? ? ? ? ? ? rcode = (builtin_run_command(cmd, 0) >= 0);

common/main.c int run_command(const char *cmd, int flag) · line 1411

Quoted 7 lines

common/main.c +7 -0

+
+ ? ? return rcode;
+}
+#endif
+
+/*
+ * Run a list of commands separated by ;

mh, where is the command string seperated by ';' if no hush is used?

Quoted 7 lines

+ *
+ * Note that if 'len' is not -1, then the command may not be \0 terminated, + * Memory will be allocated for the command in that case.
+ *
+ * @param cmd ? ? ? ?List of commands to run, each separated bu semicolon
+ * @param len ? ? ? ?Length of command excluding terminator if known (-1 if

not)

common/main.c int run_command(const char *cmd, int flag)

Quoted 7 lines

+ * @param flag ? ? ? Execution flags (CMD_FLAG_...)
+ * @return 0 on success, or != 0 on error.
+ */

common/main.c +4 -0

+int run_command_list(const char *cmd, int len, int flag)
+{
+ ? ? int need_buff = 1;
+ ? ? char *buff = (char *)cmd;

mhhh, builtin_run_command_list will modify buff. what do you think about always copying the buffer if no hush parser is used?

include/common.h int ? ? ? print_buffer (ulong addr, void* data, uint

Quoted 33 lines

+ ? ? int rcode = 0;

common/main.c +22 -0

+
+ ? ? if (len == -1) {
+ ? ? ? ? ? ? len = strlen(cmd);
+ ? ? ? ? ? ? need_buff = strchr(cmd, '\n') != NULL;
+ ? ? }
+ ? ? if (need_buff) {
+ ? ? ? ? ? ? buff = malloc(len + 1);
+ ? ? ? ? ? ? if (!buff)
+ ? ? ? ? ? ? ? ? ? ? return 1;
+ ? ? ? ? ? ? memcpy(buff, cmd, len + 1);
+ ? ? }
+#ifdef CONFIG_SYS_HUSH_PARSER
+ ? ? rcode = parse_string_outer(buff, FLAG_PARSE_SEMICOLON);
+#else
+ ? ? rcode = builtin_run_command_list(buff, flag);
+#endif
+ ? ? if (need_buff)
+ ? ? ? ? ? ? free(buff);
+
+ ? ? return rcode;
+}
+

?/************************************************************************* ***/

?#if defined(CONFIG_CMD_RUN)

include/common.h +0 -0

index 0bda049..cc5d61c 100644
@@ -261,6 +261,7 @@ int ? ? ? print_buffer (ulong addr, void* data, uint

width,

Quoted 7 lines

uint count, uint linelen); /* common/main.c */
?void main_loop ? ? ? (void);
?int run_command(const char *cmd, int flag);
+int run_command_list(const char *cmd, int len, int flag); ?int ?readline ? ? ? ?(const char *const prompt);
?int ?readline_into_buffer(const char *const prompt, char *buffer, ? ? ? ? ? ? ? ? ? ? ? int timeout);

--
Michael

Hi Michael,

Quoted 62 lines · Michael Walle

Am Mittwoch 15 Februar 2012, 07:07:09 schrieb Simon Glass:

Quoted 13 lines

This new function runs a list of commands separated by semicolon. We move this out of cmd_source so that it can be used by other code. The PXE also uses the new function.

Suggested-by Michael Walle <[email protected]>

Signed-off-by Simon Glass <[email protected]>

---
 common/cmd_pxe.c    |   20 ++------------
 common/cmd_source.c |   49 +----------------------------------
 common/main.c       |   72
+++++++++++++++++++++++++++++++++++++++++++++++++++ include/common.h  
 | 1 +
 4 files changed, 77 insertions(+), 65 deletions(-)

[..snip..]

Quoted 45 lines

common/main.c +9 -0

index db181d3..87f2855 100644
@@ -30,6 +30,7 @@
 #include <common.h>
 #include <watchdog.h>
 #include <command.h>
+#include <malloc.h>
 #include <version.h>
 #ifdef CONFIG_MODEM_SUPPORT
 #include <malloc.h>          /* for free() prototype */

@@ -1373,6 +1374,77 @@ int run_command(const char *cmd, int flag)
 #endif
 }
 
+#ifndef CONFIG_SYS_HUSH_PARSER
+static int builtin_run_command_list(char *cmd, int flag)
+{
+     char *line, *next;
+     int rcode = 0;
+
+     /*
+      * Break into individual lines, and execute each line; terminate

on + * error.

common/main.c +20 -0

+      */
+     line = next = cmd;
+     while (*next) {
+             if (*next == '\n') {
+                     *next = '\0';
+                     /* run only non-empty commands */
+                     if (*line) {
+                             debug("** exec: \"%s\"\n", line);
+                             if (builtin_run_command(line, 0) < 0) {
+                                     rcode = 1;
+                                     break;
+                             }
+                     }
+                     line = next + 1;
+             }
+             ++next;
+     }
+     if (rcode == 0 && *cmd)
+             rcode = (builtin_run_command(cmd, 0) >= 0);
+

Quoted 8 lines

common/main.c int run_command(const char *cmd, int flag) · line 1411

Quoted 6 lines

common/main.c +6 -0

+     return rcode;
+}
+#endif
+
+/*
+ * Run a list of commands separated by ;

mh, where is the command string seperated by ';' if no hush is used?

In that case it is handled by builtin_run_command() already.

Quoted 10 lines

Quoted 7 lines

+ *
+ * Note that if 'len' is not -1, then the command may not be \0 terminated, + * Memory will be allocated for the command in that case.
+ *
+ * @param cmd List of commands to run, each separated bu
semicolon + * @param len Length of command excluding terminator
if known (-1 if

not)

Quoted 10 lines

common/main.c int run_command(const char *cmd, int flag) · line 1424

Quoted 7 lines

+ * @param flag Execution flags (CMD_FLAG_...)
+ * @return 0 on success, or != 0 on error.
+ */

common/main.c +4 -0

+int run_command_list(const char *cmd, int len, int flag)
+{
+     int need_buff = 1;
+     char *buff = (char *)cmd;

mhhh, builtin_run_command_list will modify buff. what do you think about always copying the buffer if no hush parser is used?

Yes we could do - how would that help?

Am Mittwoch 15 Februar 2012, 23:23:19 schrieb Simon Glass:

Quoted 97 lines

Hi Michael,

Quoted 62 lines · Michael Walle

Am Mittwoch 15 Februar 2012, 07:07:09 schrieb Simon Glass:

Quoted 13 lines

This new function runs a list of commands separated by semicolon. We move this out of cmd_source so that it can be used by other code. The PXE also uses the new function.

Suggested-by Michael Walle <[email protected]>

Signed-off-by Simon Glass <[email protected]>

---

?common/cmd_pxe.c ? ?| ? 20 ++------------
?common/cmd_source.c | ? 49 +----------------------------------
?common/main.c ? ? ? | ? 72

+++++++++++++++++++++++++++++++++++++++++++++++++++ include/common.h ?| 1 +
?4 files changed, 77 insertions(+), 65 deletions(-)

[..snip..]

Quoted 45 lines

common/main.c +0 -0

index db181d3..87f2855 100644
@@ -30,6 +30,7 @@

?#include <common.h>
?#include <watchdog.h>
?#include <command.h>
+#include <malloc.h>
?#include <version.h>
?#ifdef CONFIG_MODEM_SUPPORT
?#include <malloc.h> ? ? ? ? ?/* for free() prototype */

common/main.c +0 -0

@@ -1373,6 +1374,77 @@ int run_command(const char *cmd, int flag)

?#endif
?}

common/main.c +8 -0

+#ifndef CONFIG_SYS_HUSH_PARSER
+static int builtin_run_command_list(char *cmd, int flag)
+{
+ ? ? char *line, *next;
+ ? ? int rcode = 0;
+
+ ? ? /*
+ ? ? ?* Break into individual lines, and execute each line; terminate

on + ? ? ?* error.
+ ? ? ?*/
+ ? ? line = next = cmd;
+ ? ? while (*next) {
+ ? ? ? ? ? ? if (*next == '\n') {
+ ? ? ? ? ? ? ? ? ? ? *next = '\0';
+ ? ? ? ? ? ? ? ? ? ? /* run only non-empty commands */
+ ? ? ? ? ? ? ? ? ? ? if (*line) {
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? debug("** exec: \"%s\"\n", line);
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? if (builtin_run_command(line, 0) < 0) {
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? rcode = 1;
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? break;
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? }
+ ? ? ? ? ? ? ? ? ? ? }
+ ? ? ? ? ? ? ? ? ? ? line = next + 1;
+ ? ? ? ? ? ? }
+ ? ? ? ? ? ? ++next;
+ ? ? }
+ ? ? if (rcode == 0 && *cmd)
+ ? ? ? ? ? ? rcode = (builtin_run_command(cmd, 0) >= 0); +

Quoted 8 lines

common/main.c int run_command(const char *cmd, int flag) · line 1411

Quoted 6 lines

+ ? ? return rcode;

common/main.c +5 -0

+}
+#endif
+
+/*
+ * Run a list of commands separated by ;

mh, where is the command string seperated by ';' if no hush is used?

In that case it is handled by builtin_run_command() already.

Quoted 20 lines

Quoted 7 lines

+ *
+ * Note that if 'len' is not -1, then the command may not be \0 terminated, + * Memory will be allocated for the command in that case.
+ *
+ * @param cmd ? ? ? ?List of commands to run, each separated bu semicolon + * @param len ? ? ? ?Length of command excluding terminator if known (-1 if

not)

common/main.c int run_command(const char *cmd, int flag)

Quoted 7 lines

+ * @param flag ? ? ? Execution flags (CMD_FLAG_...)
+ * @return 0 on success, or != 0 on error.
+ */

common/main.c +4 -0

+int run_command_list(const char *cmd, int len, int flag)
+{
+ ? ? int need_buff = 1;
+ ? ? char *buff = (char *)cmd;

mhhh, builtin_run_command_list will modify buff. what do you think about always copying the buffer if no hush parser is used?

Quoted 1 line

Yes we could do - how would that help?

ah sorry, i hadn't understood your code. too late here ;)

copying the buffer everytime (in case no hush is available):
?- removes the cast, which seems very odd on the first look

?- makes run_command_list() more robust against furture changes in ? builtin_run_command_list(), eg. if it modifies the string not only if ? '\n' is found in the cmd.

at least you should add a comment to keep the check for need_buff in sync with builtin_run_command_list().

--
Michael

This new function runs a list of commands separated by semicolon. We move this out of cmd_source so that it can be used by other code. The PXE also uses the new function.

common/main.c +8 -0

+	line = next = cmd;
+	while (*next) {
+		if (*next == '\n') {
+			*next = '\0';
+			/* run only non-empty commands */
+			if (*line) {
+				debug("** exec: \"%s\"\n", line);
+				if (builtin_run_command(line, 0) < 0) {

Dear Simon Glass,

Quoted 3 lines · In message <[email protected]> you wrote:

This new function runs a list of commands separated by semicolon. We move this out of cmd_source so that it can be used by other code. The PXE also uses the new function.

Separated by semicolon? ?What about newline here?

Quoted 1 line

+ ? ? printf("running: %s\n", localcmd);

Should this be a debug() ?

Quoted 11 lines

+ ? ? line = next = cmd;
+ ? ? while (*next) {
+ ? ? ? ? ? ? if (*next == '\n') {
+ ? ? ? ? ? ? ? ? ? ? *next = '\0';
+ ? ? ? ? ? ? ? ? ? ? /* run only non-empty commands */
+ ? ? ? ? ? ? ? ? ? ? if (*line) {
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? debug("** exec: \"%s\"\n", line);
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? if (builtin_run_command(line, 0) < 0) {
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? rcode = 1;
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? break;

Quoted 8 lines

+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? }
+ ? ? ? ? ? ? ? ? ? ? }
+ ? ? ? ? ? ? ? ? ? ? line = next + 1;
+ ? ? ? ? ? ? }
+ ? ? ? ? ? ? ++next;
+ ? ? }
+ ? ? if (rcode == 0 && *cmd)
+ ? ? ? ? ? ? rcode = (builtin_run_command(cmd, 0) >= 0);

This looks wrong to me. ?There you are re-executing the original command. ?Shoudl this not be

? ? ? ?if (rcode == 0 && *line)
? ? ? ? ? ? ? ?rcode = (run_command(line, 0) >= 0);

??

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 Computers are not intelligent. ?They only think they are.