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

[Simon Glass]

Hi Mike,

On Sun, Apr 8, 2012 at 1:39 AM, Mike Frysinger <vapier at gentoo.org> wrote:
> On Wednesday 04 April 2012 03:12:27 Simon Glass wrote:
>> On Sun, Apr 1, 2012 at 12:48 PM, Mike Frysinger wrote:
>> > On Saturday 31 March 2012 03:30:55 Simon Glass wrote:
>> >> --- a/common/cmd_pxe.c
>> >> +++ b/common/cmd_pxe.c
>> >>
>> >> +int run_command_list(const char *cmd, int len, int flag)
>> >> +{
>> >> +     int need_buff = 1;
>> >> +     char *buff = (char *)cmd;       /* cast away const */
>> >> +     int rcode = 0;
>> >> +
>> >> +     if (len == -1) {
>> >> +             len = strlen(cmd);
>> >> +#ifdef CONFIG_SYS_HUSH_PARSER
>> >> +             /* hush will never change our string */
>> >> +             need_buff = 0;
>> >> +#else
>> >> +             /* the built-in parser will change our string if it sees
>> >> \n */ +             need_buff = strchr(cmd, '\n') != NULL;
>> >> +#endif
>> >> +     }
>> >
>> > we have memchr(), so you should be able to split the len==-1 and the
>> > need_buff logic into two sep steps
>>
>> Are you saying that we should do this check if len is not -1 also?
>>
>> I am only doing it when len is not specified, since if a length is
>> provided we must allocate. This is used by the source command, which
>> does not necessary have a nul-terminated string.
>
> looks to me like it should be:
>        int need_buff;
>
>        if (len == -1)
>                len = strlen(cmd);
> #ifdef CONFIG_SYS_HUSH_PARSER
>        need_buff = 0;
> #else
>        need_buff = memchr(cmd, '\n', len) != NULL;
> #endif
>        if (need_buff) {
>                ...
>        }
>

This logic is different. For example it won't nul terminate a
terminate a command string that is passed in with no terminator. I
think this is required by the 'source' command.

>> > also, should you handle the case where '\n' is the very last char ?  or
>> > not bother ?
>>
>> Do you mean not allocate in that case? I don't think it is a common
>> situation is it?
>
> i don't know how commands get passed down (as i've never poked that area
> before) which is why i'm asking

Well within the normal command parses run_command() is called. They
split up the commands for us. We only need run_command_list() when we
think we might have a raw list of commands, on which no processing has
been done.

Regards,
Simon

> -mike