From mboxrd@z Thu Jan 1 00:00:00 1970 From: Liam Girdwood Subject: Re: [PATCH v2 07/13] topology: Add private data parser Date: Wed, 08 Jul 2015 15:21:15 +0100 Message-ID: <1436365275.2474.41.camel@loki> References: <1435758275-4047-1-git-send-email-liam.r.girdwood@linux.intel.com> <1435758275-4047-7-git-send-email-liam.r.girdwood@linux.intel.com> <1436284442.7271.30.camel@loki> <1436345852.2474.10.camel@loki> <559D262F.20403@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by alsa0.perex.cz (Postfix) with ESMTP id B17162615F7 for ; Wed, 8 Jul 2015 16:21:19 +0200 (CEST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Takashi Iwai Cc: Vinod Koul , "Jin, Yao" , Mark Brown , alsa-devel@alsa-project.org List-Id: alsa-devel@alsa-project.org On Wed, 2015-07-08 at 16:14 +0200, Takashi Iwai wrote: > 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. > Ok, lets make commas at the end illegal. > 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. > So I'll fix this to be "0x01, 0x02, 0x03" syntax only. i.e. comma between each value and no comma at the end. Liam