From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f173.google.com (mail-wr0-f173.google.com [209.85.128.173]) by dpdk.org (Postfix) with ESMTP id D707C2C00 for ; Fri, 23 Jun 2017 11:04:22 +0200 (CEST) Received: by mail-wr0-f173.google.com with SMTP id r103so56809582wrb.0 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=bLUt/k2hOsMrKsiWnQxV8Dmsv0IRo46dXq8z7zNhMmtrrVqE1u8OYvHQfUZAylALLM +WFAhM9Bj+keQRtw+VKindcIHKHtZo1uNfUzAa3Ztle4fD85xS03jJcLOupsSf9Eznlu v64DVe4M7nYPv9wuIctTZKKRjBJJdf6DFXxfupK+q+vg0f8dNpoGOXBDV7CnZej7O985 9+7VWX0uxljEDQbDObMDqhKVwpFbWFfCVe5fA3XJZ2ZvWR265JDyTmFmGmXrcXkxB8OQ RTsqQKnd80W7B3lmZAKU2dGTk8KIZDPXPUJPetwwlEIwrrAvBbUf/rvPIdez4hJm7A3v hW2g== X-Gm-Message-State: AKS2vOxdUIVfOJ/+t/Sh5JyOfP+ejZzExUQVQd4UyzCVzIwKQme1ySU2 xb8eTzdxdnJsf9V5 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-dev] [PATCH v2] librte_cmdline: fix parsing initialisation X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions 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