[dpdk-dev] lib/librte_cmdline: fix CLI parsing issue

Message ID 1493089885-94889-1-git-send-email-wenzhuo.lu@intel.com (mailing list archive)
State Accepted, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Wenzhuo Lu April 25, 2017, 3:11 a.m. UTC
  When parsing a CLI, all the CLI instances are checked
one by one. Even if an instance already matches the CLI,
the parsing will not stop for ambiguous check.
The problem is that the following check may change the
parsing result of the previous one even if the following
instance doesn't match.

Use a temporary validate for the parsing result when
trying to match an instance and only store the result
when it matches, so the previous result has no chance
to be changed.

Fixes: af75078fece3 ("first public release")
CC: stable@dpdk.org

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 lib/librte_cmdline/cmdline_parse.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
  

Comments

Wenzhuo Lu April 28, 2017, 8:38 a.m. UTC | #1
Hi Oilvier,
As we discussed before, I send this patch. Would you like to take a look at it? Better fix it before release 17.05, thanks.


> -----Original Message-----
> From: Lu, Wenzhuo
> Sent: Tuesday, April 25, 2017 11:11 AM
> To: dev@dpdk.org
> Cc: olivier.matz@6wind.com; Lu, Wenzhuo; stable@dpdk.org
> Subject: [PATCH] lib/librte_cmdline: fix CLI parsing issue
> 
> When parsing a CLI, all the CLI instances are checked one by one. Even if an
> instance already matches the CLI, the parsing will not stop for ambiguous
> check.
> The problem is that the following check may change the parsing result of the
> previous one even if the following instance doesn't match.
> 
> Use a temporary validate for the parsing result when trying to match an
> instance and only store the result when it matches, so the previous result
> has no chance to be changed.
> 
> Fixes: af75078fece3 ("first public release")
> CC: stable@dpdk.org
> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
>  lib/librte_cmdline/cmdline_parse.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_cmdline/cmdline_parse.c
> b/lib/librte_cmdline/cmdline_parse.c
> index 763c286..b814880 100644
> --- a/lib/librte_cmdline/cmdline_parse.c
> +++ b/lib/librte_cmdline/cmdline_parse.c
> @@ -258,7 +258,7 @@
>  	union {
>  		char buf[CMDLINE_PARSE_RESULT_BUFSIZE];
>  		long double align; /* strong alignment constraint for buf */
> -	} result;
> +	} result, tmp_result;
>  	cmdline_parse_token_hdr_t
> *dyn_tokens[CMDLINE_PARSE_DYNAMIC_TOKENS];
>  	void (*f)(void *, struct cmdline *, void *) = NULL;
>  	void *data = NULL;
> @@ -321,14 +321,16 @@
>  		debug_printf("INST %d\n", inst_num);
> 
>  		/* fully parsed */
> -		tok = match_inst(inst, buf, 0, result.buf, sizeof(result.buf),
> -				 &dyn_tokens);
> +		tok = match_inst(inst, buf, 0, tmp_result.buf,
> +				 sizeof(tmp_result.buf), &dyn_tokens);
> 
>  		if (tok > 0) /* we matched at least one token */
>  			err = CMDLINE_PARSE_BAD_ARGS;
> 
>  		else if (!tok) {
>  			debug_printf("INST fully parsed\n");
> +			memcpy(&result, &tmp_result,
> +			       sizeof(result));
>  			/* skip spaces */
>  			while (isblank2(*curbuf)) {
>  				curbuf++;
> --
> 1.9.3
  
Olivier Matz April 28, 2017, 9:13 a.m. UTC | #2
Hi Wenzhuo,

On Fri, 28 Apr 2017 08:38:21 +0000, "Lu, Wenzhuo" <wenzhuo.lu@intel.com> wrote:
> Hi Oilvier,
> As we discussed before, I send this patch. Would you like to take a look at it? Better fix it before release 17.05, thanks.
> 
> 
> > -----Original Message-----
> > From: Lu, Wenzhuo
> > Sent: Tuesday, April 25, 2017 11:11 AM
> > To: dev@dpdk.org
> > Cc: olivier.matz@6wind.com; Lu, Wenzhuo; stable@dpdk.org
> > Subject: [PATCH] lib/librte_cmdline: fix CLI parsing issue
> > 
> > When parsing a CLI, all the CLI instances are checked one by one. Even if an
> > instance already matches the CLI, the parsing will not stop for ambiguous
> > check.
> > The problem is that the following check may change the parsing result of the
> > previous one even if the following instance doesn't match.
> > 
> > Use a temporary validate for the parsing result when trying to match an
> > instance and only store the result when it matches, so the previous result
> > has no chance to be changed.
> > 
> > Fixes: af75078fece3 ("first public release")
> > CC: stable@dpdk.org
> > 
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

Sorry for the delay,

Acked-by: Olivier Matz <olivier.matz@6wind.com>
  
Thomas Monjalon April 30, 2017, 10:18 p.m. UTC | #3
28/04/2017 11:13, Olivier Matz:
> Hi Wenzhuo,
> 
> On Fri, 28 Apr 2017 08:38:21 +0000, "Lu, Wenzhuo" <wenzhuo.lu@intel.com> wrote:
> > Hi Oilvier,
> > As we discussed before, I send this patch. Would you like to take a look at it? Better fix it before release 17.05, thanks.
> > 
> > 
> > > When parsing a CLI, all the CLI instances are checked one by one. Even if an
> > > instance already matches the CLI, the parsing will not stop for ambiguous
> > > check.
> > > The problem is that the following check may change the parsing result of the
> > > previous one even if the following instance doesn't match.
> > > 
> > > Use a temporary validate for the parsing result when trying to match an
> > > instance and only store the result when it matches, so the previous result
> > > has no chance to be changed.
> > > 
> > > Fixes: af75078fece3 ("first public release")
> > > CC: stable@dpdk.org
> > > 
> > > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> 
> Sorry for the delay,
> 
> Acked-by: Olivier Matz <olivier.matz@6wind.com>

Applied, thanks
  

Patch

diff --git a/lib/librte_cmdline/cmdline_parse.c b/lib/librte_cmdline/cmdline_parse.c
index 763c286..b814880 100644
--- a/lib/librte_cmdline/cmdline_parse.c
+++ b/lib/librte_cmdline/cmdline_parse.c
@@ -258,7 +258,7 @@ 
 	union {
 		char buf[CMDLINE_PARSE_RESULT_BUFSIZE];
 		long double align; /* strong alignment constraint for buf */
-	} result;
+	} result, tmp_result;
 	cmdline_parse_token_hdr_t *dyn_tokens[CMDLINE_PARSE_DYNAMIC_TOKENS];
 	void (*f)(void *, struct cmdline *, void *) = NULL;
 	void *data = NULL;
@@ -321,14 +321,16 @@ 
 		debug_printf("INST %d\n", inst_num);
 
 		/* fully parsed */
-		tok = match_inst(inst, buf, 0, result.buf, sizeof(result.buf),
-				 &dyn_tokens);
+		tok = match_inst(inst, buf, 0, tmp_result.buf,
+				 sizeof(tmp_result.buf), &dyn_tokens);
 
 		if (tok > 0) /* we matched at least one token */
 			err = CMDLINE_PARSE_BAD_ARGS;
 
 		else if (!tok) {
 			debug_printf("INST fully parsed\n");
+			memcpy(&result, &tmp_result,
+			       sizeof(result));
 			/* skip spaces */
 			while (isblank2(*curbuf)) {
 				curbuf++;