[U-Boot] [PATCH v4 4/9] Stop using builtin_run_command()

[Simon Glass]

Hi Michael,

On Tue, Feb 14, 2012 at 2:42 PM, Simon Glass <sjg at chromium.org> wrote:

> Hi Michael,
>
> On Sat, Jan 21, 2012 at 8:07 AM, Michael Walle <michael at walle.cc> wrote:
>
>>
>> Hi Simon,
>>
>> Am Samstag 14 Januar 2012, 07:45:52 schrieb Simon Glass:
>> > diff --git a/common/cmd_source.c b/common/cmd_source.c
>> > index 481241c..32fff5c 100644
>> > --- a/common/cmd_source.c
>> > +++ b/common/cmd_source.c
>> > @@ -179,7 +179,7 @@ source (ulong addr, const char *fit_uname)
>> >                               if (*line) {
>> >                                       debug ("** exec: \"%s\"\n",
>> >                                               line);
>> > -                                     if (builtin_run_command(line, 0)
>> < 0)
>> {
>> > +                                     if (run_command(line, 0) < 0) {
>> >                                               rcode = 1;
>> >                                               break;
>> >                                       }
>> > @@ -189,7 +189,7 @@ source (ulong addr, const char *fit_uname)
>> >                       ++next;
>> >               }
>> >               if (rcode == 0 && *line)
>> > -                     rcode = (builtin_run_command(line, 0) >= 0);
>> > +                     rcode = (run_command(line, 0) >= 0);
>> >       }
>> >  #endif
>> >       free (cmd);
>>
>> Please have a look at this file. There is already a conditional on
>> CONFIG_SYS_HUSH_PARSER, which can be removed, since it is now handled in
>> run_command(). Do not forget to remove the hush.h include, too.
>>
>
> OK
>
>>
>> Could you move this script handling into an own function into
>> common/main.c?
>> This way others can easily execute whole scripts without duplicating this
>> code. I'm planning to submit support for a board which needs exactly
>> that, so
>> there will be at least one board which make use of this.
>>
>> Mike proposed source_commands() as a function name. My original patch
>> named it
>> run_script(). Dunno whats a better name.
>>
>
> I will go with run_command_list() so it matches run_command(). Will send
> an updated series with two new commits which hopefully does what you want.
>
>
>>
>> Feel free to reuse/incorporate my patch at
>>  http://patchwork.ozlabs.org/patch/137122/
>> into your patchset ;)
>>
>
> Thanks, have done so.
>

Unfortunately I think that this approach is a little broken. We are running
a command sequence from getenv(), but while processing it, we might update
the environment variable. Worse, we actually overwrite the newlines in the
variable as we process it.

Since this seems like a bigger change than I thought, and I think it missed
the merge window, I have created a two separate patches for this so people
can review it properly. Will send through shortly.

Regards,
Simon


>
>
>>
>> --
>> michael
>>
>
> Regards,
> Simon
>
>