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

[Simon Glass]

Hi Mike,

On Sun, Apr 1, 2012 at 12:48 PM, Mike Frysinger <vapier at gentoo.org> wrote:
> On Saturday 31 March 2012 03:30:55 Simon Glass wrote:
>> --- a/common/cmd_pxe.c
>> +++ b/common/cmd_pxe.c
>>
>> +     return run_command_list(localcmd, strlen(localcmd), 0);
>
> should be -1 instead of strlen()

done

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

 * Note that if 'len' is not -1, then the command does not need to be nul
 * terminated, Memory will be allocated for the command in that case.

Sorry if I am missing your point.

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

> -mike

Regards,
Simon