Re: [PATCH v2 07/13] topology: Add private data parser

[Takashi Iwai <tiwai@suse.de>]

On Wed, 08 Jul 2015 15:31:27 +0200,
Jin, Yao wrote:
> 
> 
> 
> On 2015/7/8 16:57, Liam Girdwood wrote:
> > On Tue, 2015-07-07 at 18:19 +0200, Takashi Iwai wrote:
> >> On Tue, 07 Jul 2015 17:54:02 +0200,
> >> Liam Girdwood wrote:
> >>>
> >>> On Wed, 2015-07-01 at 18:20 +0200, Takashi Iwai wrote:
> >>>
> >>>>
> >>>>> +static int get_hex_num(const char *str)
> >>>>> +{
> >>>>> +	char *tmp, *s = NULL;
> >>>>> +	int i = 0;
> >>>>> +
> >>>>> +	tmp = strdup(str);
> >>>>> +	if (tmp == NULL)
> >>>>> +		return -ENOMEM;
> >>>>> +
> >>>>> +	s = strtok(tmp, ",");
> >>>>> +	while (s != NULL) {
> >>>>> +		s = strtok(NULL, ",");
> >>>>> +		i++;
> >>>>> +	}
> >>>>> +
> >>>>> +	free(tmp);
> >>>>> +	return i;
> >>>>
> >>>> Hmm, this just counts the number of comma + 1, so you don't need to
> >>>> duplicate the string?
> >>>>
> >>>
> >>> The string here is duplicated since strtok is destructive (it overwrites
> >>> the delimiters with NULL) and the string is being used later on by the
> >>> calling function.
> >>
> >> Yes, but what I meant is something like below:
> >>
> >> static int get_hex_num(const char *str)
> >> {
> >> 	int i = 0;
> >>
> >> 	if (!*str)
> >> 		return 0;
> >> 	for (;;) {
> >> 		i++;
> >> 		str = strchr(str, ',');
> >> 		if (!str)
> >> 			return i;
> >> 		str++;
> >> 	}
> >> }
> >>
> >> ... so that it works without strdup().
> >>
> >> But it seems that strtok() skips the starting delimiter or handles
> >> multiple delimiters as a single, so the result will become
> >> inconsistent.  That is, all the following strings will give "a" and
> >> "b" via strtok():
> >> 	a,b
> >> 	a,,b
> >> 	,a,b
> >> 	a,b,
> >>
> >> I guess you don't want to have an empty list element, right?
> >>
> > 
> > Lets ask the author :) but IMO an empty list should be skipped here.
> > 
> > Yao, what's your rational behind this code ?
> 
> Sorry for replying late, I just see this mail.
> 
> The get_hex_num() returns the number of hexadecimal in string.
> 
> Say the string is "0x12,0x34,0x56,0x78" or "0x12,0x34,0x56,0x78,",
> the get_hex_num() returns 4.
> 
> But if I use strchr, for above string, I get different number (3 or 4).
> That's the reason I choose the strtok().
> 
> I do this is because I think user may append the comma at the end of
> string.

So, is this allowed intentionally as a valid syntax?  As I showed,
a string like ",,,0x12,,0x34,,,,0x56,0x78,," would be handled as a
valid string.

The above may look strange, but the strtok() behavior is natural if
you imagine to replace "," with a space.


Takashi