Re: [U-Boot] [PATCH 1/2] Add run_command_list() to run a list of commands

16 Feb 2012

Am Mittwoch 15 Februar 2012, 23:23:19 schrieb Simon Glass:
...
Hi Michael,
On Wed, Feb 15, 2012 at 11:38 AM, Michael Walle michael@walle.cc wrote:
...
Am Mittwoch 15 Februar 2012, 07:07:09 schrieb Simon Glass:
...
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 michael@walle.cc
Signed-off-by: Simon Glass sjg@chromium.org

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..]
...

diff --git a/common/main.c b/common/main.c
index db181d3..87f2855 100644
--- a/common/main.c
+++ b/common/main.c
@@ -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;


}


if (rcode == 0 && *cmd)


        rcode = (builtin_run_command(cmd, 0) >= 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.
...
...






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


@param flag       Execution flags (CMD_FLAG_...)



@return 0 on success, or != 0 on error.


*/

+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?
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

    