From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH v2 07/13] topology: Add private data parser Date: Wed, 08 Jul 2015 16:14:01 +0200 Message-ID: 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 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 7A30F2615F7 for ; Wed, 8 Jul 2015 16:14:02 +0200 (CEST) In-Reply-To: <559D262F.20403@linux.intel.com> 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: "Jin, Yao" Cc: Liam Girdwood , Vinod Koul , alsa-devel@alsa-project.org, Mark Brown List-Id: alsa-devel@alsa-project.org 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