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

[Simon Glass]

Hi Wolfgang,

On Wed, Mar 7, 2012 at 3:34 AM, Wolfgang Denk <wd at denx.de> wrote:
> Dear Simon Glass,
>
> In message <1329286030-32560-1-git-send-email-sjg at chromium.org> 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.

This took a bit longer to get back to than I expected.

>
> Separated by semicolon?  What about newline here?
>
>> +     printf("running: %s\n", localcmd);
>
> Should this be a debug() ?

I'm not sure - the old code has printf() but I don't know why. I will change it.

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

Yes you are right of course. I created a few tests and found a few
other nits as well, so will send a new series that tidies all this up.

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