From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f171.google.com (mail-wr0-f171.google.com [209.85.128.171]) by dpdk.org (Postfix) with ESMTP id D19A329CA for ; Fri, 23 Jun 2017 11:04:22 +0200 (CEST) Received: by mail-wr0-f171.google.com with SMTP id c11so56656689wrc.3 for ; Fri, 23 Jun 2017 02:04:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=gfsTE/PBX5aTl+qvGMc3E4qIPVl83UbPo1MmXZi729Q=; b=iPgVCeebu6hq4YZucKkWc3PivOg8KoZHx7cK3H0G1Q+b2ZiagqTfIzw1A5LmsP+2Am 2f2NTRnD0mzlmRQNaAmHi92BiIMuEbByQBjh49+lMF/nTiEgZ33dTzL6+VYSKvaonG0+ wmVJsYPWIGGQaBrOifsd7dBCRkSToX8T28TpGwVbFbDYCbwT+H6ERfaBSc44VfVL0qC8 dx/vyasOGPRgMNsoplb4uLYXNhvifNBs5oS/qYRCArDs20egrX9PLMRASKdRXpo9sUSX qYQEDAXVRys2OtO9WrTMu0EitYSGvOOX7Ro2ncP/MOO7lspkJsRtSbPbyWHaHpedjDo+ n+Gg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=gfsTE/PBX5aTl+qvGMc3E4qIPVl83UbPo1MmXZi729Q=; b=kAXtssNdjqasobh5JqwkD2WLkj+fGGqMUf0Adoivtd0grRNm3sIQ+Y9wLkZiyNklvg /6nzF6vXlnmCL16dtPgiBb5EFDHLsnbQGaJMGPRRQjR0k7LkK34YtepUwmE24mrWQCOK LtmPO9Q3YMldbhaFO3BGUmBoNtm6sa8GkGjvHiT7SWH9bS+Qian8kwhIgPny0xix0tzz 5iKby1cejnGOWIlK6NHfABbxqGU9iVMLw+p4P7ARJdVwdjH1G9EI6abUzESVyViB1mlo x1HlE0grtwdeeOkRikXZneV9Rz67EbPiQ572KSL1FG0Hsl6nGm+U0A2ses+nPMrojISu koaQ== X-Gm-Message-State: AKS2vOwk4EXPGapQJ5KrSPBI4ekN8s+wXKafxpXAEeR3o3pOU1XHt9mC oNAobFJ4YBwL5Hz5 X-Received: by 10.223.171.181 with SMTP id s50mr5208719wrc.177.1498208662516; Fri, 23 Jun 2017 02:04:22 -0700 (PDT) Received: from platinum (2a01cb0c03c651000226b0fffeed02fc.ipv6.abo.wanadoo.fr. [2a01:cb0c:3c6:5100:226:b0ff:feed:2fc]) by smtp.gmail.com with ESMTPSA id u67sm2429908wmd.10.2017.06.23.02.04.21 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 23 Jun 2017 02:04:21 -0700 (PDT) Date: Fri, 23 Jun 2017 11:04:19 +0200 From: Olivier Matz To: Adrien Mazarguil Cc: Bernard Iremonger , dev@dpdk.org, stable@dpdk.org Message-ID: <20170623110419.43b9ac89@platinum> In-Reply-To: <327b5be12221f51fbf3a6d8e9d155de786992388.1497521374.git.adrien.mazarguil@6wind.com> References: <1497348447-1167-1-git-send-email-bernard.iremonger@intel.com> <327b5be12221f51fbf3a6d8e9d155de786992388.1497521374.git.adrien.mazarguil@6wind.com> X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-stable] [PATCH v2] librte_cmdline: fix parsing initialisation X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 23 Jun 2017 09:04:23 -0000 Hi Adrien, On Thu, 15 Jun 2017 12:15:48 +0200, Adrien Mazarguil wrote: > From: "Bernard.Iremonger" > > The dyn_tokens array is initialised at the beginning of the cmdline_parse > function. However when the inst_num variable is incremented later in the > function the dyn_tokens array is not reinitialised so the tokens from the > previous command are used. > > The solution is to initialise the dyn_tokens array in all while (inst) > loops, before calling match_inst(). > > Fixes: 4fffc05a2b2c ("cmdline: support dynamic tokens") > > CC: stable@dpdk.org > Signed-off-by: Bernard Iremonger > Signed-off-by: Adrien Mazarguil > > --- > > Nice catch Bernard. This issue can be seen when implementing several > flow-like commands in testpmd. > > While testing your original patch though, it appeared that it did not fully > address the issue, as completion remained partially broken. Actually all > match_inst() calls should be preceded by a memset(), so here's an updated > version instead of requesting you to do these changes. > --- > lib/librte_cmdline/cmdline_parse.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/lib/librte_cmdline/cmdline_parse.c b/lib/librte_cmdline/cmdline_parse.c > index b814880..c1d9f23 100644 > --- a/lib/librte_cmdline/cmdline_parse.c > +++ b/lib/librte_cmdline/cmdline_parse.c > @@ -276,7 +276,6 @@ cmdline_parse(struct cmdline *cl, const char * buf) > return CMDLINE_PARSE_BAD_ARGS; > > ctx = cl->ctx; > - memset(&dyn_tokens, 0, sizeof(dyn_tokens)); > > /* > * - look if the buffer contains at least one line > @@ -321,6 +320,7 @@ cmdline_parse(struct cmdline *cl, const char * buf) > debug_printf("INST %d\n", inst_num); > > /* fully parsed */ > + memset(&dyn_tokens, 0, sizeof(dyn_tokens)); > tok = match_inst(inst, buf, 0, tmp_result.buf, > sizeof(tmp_result.buf), &dyn_tokens); > > @@ -400,7 +400,6 @@ cmdline_complete(struct cmdline *cl, const char *buf, int *state, > > debug_printf("%s called\n", __func__); > memset(&token_hdr, 0, sizeof(token_hdr)); > - memset(&dyn_tokens, 0, sizeof(dyn_tokens)); > > /* count the number of complete token to parse */ > for (i=0 ; buf[i] ; i++) { > @@ -423,6 +422,7 @@ cmdline_complete(struct cmdline *cl, const char *buf, int *state, > inst = ctx[inst_num]; > while (inst) { > /* parse the first tokens of the inst */ > + memset(&dyn_tokens, 0, sizeof(dyn_tokens)); > if (nb_token && > match_inst(inst, buf, nb_token, NULL, 0, > &dyn_tokens)) > @@ -530,6 +530,7 @@ cmdline_complete(struct cmdline *cl, const char *buf, int *state, > /* we need to redo it */ > inst = ctx[inst_num]; > > + memset(&dyn_tokens, 0, sizeof(dyn_tokens)); > if (nb_token && > match_inst(inst, buf, nb_token, NULL, 0, &dyn_tokens)) > goto next2; It looks you call memset() before each call to match_inst(). So, I have 2 questions: - why not putting this memset() at the beginning of match_inst()? - does the following test at the beginning of match_inst() still make sense? if (!token_p && dyn_tokens && inst->f) { if (!(*dyn_tokens)[0]) /* <<<<<<<<< here */ inst->f(&(*dyn_tokens)[0], NULL, dyn_tokens); token_p = (*dyn_tokens)[0]; } Thanks, Olivier