From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olivier MATZ Subject: Re: CLI parsing issue Date: Thu, 20 Apr 2017 10:54:55 +0200 Message-ID: <20170420105455.02ccc25f@glumotte.dev.6wind.com> References: <6A0DE07E22DDAD4C9103DF62FEBC09093B59B248@shsmsx102.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: "dev@dpdk.org" To: "Lu, Wenzhuo" Return-path: Received: from mail-wm0-f51.google.com (mail-wm0-f51.google.com [74.125.82.51]) by dpdk.org (Postfix) with ESMTP id C08542BA4 for ; Thu, 20 Apr 2017 10:54:28 +0200 (CEST) Received: by mail-wm0-f51.google.com with SMTP id m123so31236309wma.0 for ; Thu, 20 Apr 2017 01:54:28 -0700 (PDT) In-Reply-To: <6A0DE07E22DDAD4C9103DF62FEBC09093B59B248@shsmsx102.ccr.corp.intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Wenzhuo, On Thu, 20 Apr 2017 08:36:38 +0000, "Lu, Wenzhuo" wrote: > Hi Olivier, > I met a problem thar the parsing result of CLI is wrong. > I checked this function, cmdline_parse. It will check the CLI instances one by one. Even if an instance is matched, the parsing will not stop for ambiguous check. Seems the following check may change the parsing result of the previous one, > /* fully parsed */ > tok = match_inst(inst, buf, 0, result.buf, sizeof(result.buf), > &dyn_tokens); > > > Is it better to use a temporary validate for match_inst and only store the result when it matches, so the previous result has no chance to be changed? Like bellow, > > > diff --git a/lib/librte_cmdline/cmdline_parse.c b/lib/librte_cmdline/cmdline_parse.c > index 763c286..663efd1 100644 > --- a/lib/librte_cmdline/cmdline_parse.c > +++ b/lib/librte_cmdline/cmdline_parse.c > @@ -259,6 +259,7 @@ > char buf[CMDLINE_PARSE_RESULT_BUFSIZE]; > long double align; /* strong alignment constraint for buf */ > } result; > + char tmp_buf[CMDLINE_PARSE_RESULT_BUFSIZE]; > cmdline_parse_token_hdr_t *dyn_tokens[CMDLINE_PARSE_DYNAMIC_TOKENS]; > void (*f)(void *, struct cmdline *, void *) = NULL; > void *data = NULL; > @@ -321,7 +322,7 @@ > debug_printf("INST %d\n", inst_num); > > /* fully parsed */ > - tok = match_inst(inst, buf, 0, result.buf, sizeof(result.buf), > + tok = match_inst(inst, buf, 0, tmp_buf, sizeof(tmp_buf), > &dyn_tokens); > > if (tok > 0) /* we matched at least one token */ > @@ -329,6 +330,8 @@ > > else if (!tok) { > debug_printf("INST fully parsed\n"); > + memcpy(result.buf, tmp_buf, > + CMDLINE_PARSE_RESULT_BUFSIZE); > /* skip spaces */ > while (isblank2(*curbuf)) { > curbuf++; > > At first glance, I think your patch doesn't hurt. Do you have an example code that triggers the issue? Thanks, Olivier