From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <olivier.matz@6wind.com>
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 <stable@dpdk.org>; Fri, 23 Jun 2017 11:04:22 +0200 (CEST)
Received: by mail-wr0-f171.google.com with SMTP id c11so56656689wrc.3
for <stable@dpdk.org>; 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 <olivier.matz@6wind.com>
To: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Cc: Bernard Iremonger <bernard.iremonger@intel.com>, 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 <stable.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/stable>,
<mailto:stable-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/stable/>
List-Post: <mailto:stable@dpdk.org>
List-Help: <mailto:stable-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/stable>,
<mailto:stable-request@dpdk.org?subject=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 <adrien.mazarguil@6wind.com> wrote:
> From: "Bernard.Iremonger" <Bernard.iremonger@intel.com>
>
> 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 <bernard.iremonger@intel.com>
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
>
> ---
>
> 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