patches for DPDK stable branches
 help / color / mirror / Atom feed
From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: Olivier Matz <olivier.matz@6wind.com>
Cc: dev@dpdk.org, stable@dpdk.org,
	Bernard Iremonger <bernard.iremonger@intel.com>
Subject: [dpdk-stable] [PATCH v3 1/3] cmdline: fix dynamic tokens initialization
Date: Mon, 10 Jul 2017 14:09:34 +0200	[thread overview]
Message-ID: <784ee3797f60d0e1fc7d6aaa009812a5bf77cdd8.1499687422.git.adrien.mazarguil@6wind.com> (raw)
In-Reply-To: <cover.1499687422.git.adrien.mazarguil@6wind.com>

To avoid redundant calls to the token-generating function and provide it
with helpful context, a temporary fixed-size array allocated on the stack
of cmdline_parse() and cmdline_complete() keeps the address of preceding
tokens for the command instance being processed (cmdline_parse_inst_t).

Like the static tokens array in cmdline_parse_inst_t, it must be
NULL-terminated, however this is not properly enforced as it is initialized
once at the beginning of each function and a NULL terminator is never
appended once replaced with generated tokens.

When several commands rely on dynamic tokens, subsequent ones inherit an
already initialized array whose tokens are not regenerated, which causes
various issues such as mixed and repeated tokens from the first command.

Enforcing NULL termination of the dynamic tokens array and reinitializing
its first entry at each iteration solves these issues. Doing so is also
less expensive than a full memset() at each iteration.

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>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_cmdline/cmdline_parse.c | 78 +++++++++++++++------------------
 1 file changed, 36 insertions(+), 42 deletions(-)

diff --git a/lib/librte_cmdline/cmdline_parse.c b/lib/librte_cmdline/cmdline_parse.c
index b814880..67e452d 100644
--- a/lib/librte_cmdline/cmdline_parse.c
+++ b/lib/librte_cmdline/cmdline_parse.c
@@ -139,6 +139,32 @@ nb_common_chars(const char * s1, const char * s2)
 	return i;
 }
 
+/** Retrieve either static or dynamic token at a given index. */
+static cmdline_parse_token_hdr_t *
+get_token(cmdline_parse_inst_t *inst,
+	  unsigned int index,
+	  cmdline_parse_token_hdr_t
+		*(*dyn_tokens)[CMDLINE_PARSE_DYNAMIC_TOKENS])
+{
+	/* check presence of static tokens first */
+	if (inst->tokens[0] || !inst->f)
+		return inst->tokens[index];
+	/* for dynamic tokens, make sure index does not overflow */
+	if (index >= CMDLINE_PARSE_DYNAMIC_TOKENS - 1)
+		return NULL;
+	/* in case token is already generated, return it */
+	if ((*dyn_tokens)[index])
+		return (*dyn_tokens)[index];
+	/* generate token */
+	inst->f(&(*dyn_tokens)[index], NULL, dyn_tokens);
+	/* return immediately if there are no more tokens to expect */
+	if (!(*dyn_tokens)[index])
+		return NULL;
+	/* terminate list with a NULL entry */
+	(*dyn_tokens)[index + 1] = NULL;
+	return (*dyn_tokens)[index];
+}
+
 /**
  * try to match the buffer with an instruction (only the first
  * nb_match_token tokens if != 0). Return 0 if we match all the
@@ -156,12 +182,7 @@ match_inst(cmdline_parse_inst_t *inst, const char *buf,
 	int n = 0;
 	struct cmdline_token_hdr token_hdr;
 
-	token_p = inst->tokens[token_num];
-	if (!token_p && dyn_tokens && inst->f) {
-		if (!(*dyn_tokens)[0])
-			inst->f(&(*dyn_tokens)[0], NULL, dyn_tokens);
-		token_p = (*dyn_tokens)[0];
-	}
+	token_p = get_token(inst, token_num, dyn_tokens);
 	if (token_p)
 		memcpy(&token_hdr, token_p, sizeof(token_hdr));
 
@@ -203,17 +224,7 @@ match_inst(cmdline_parse_inst_t *inst, const char *buf,
 		buf += n;
 
 		token_num ++;
-		if (!inst->tokens[0]) {
-			if (token_num < (CMDLINE_PARSE_DYNAMIC_TOKENS - 1)) {
-				if (!(*dyn_tokens)[token_num])
-					inst->f(&(*dyn_tokens)[token_num],
-						NULL,
-						dyn_tokens);
-				token_p = (*dyn_tokens)[token_num];
-			} else
-				token_p = NULL;
-		} else
-			token_p = inst->tokens[token_num];
+		token_p = get_token(inst, token_num, dyn_tokens);
 		if (token_p)
 			memcpy(&token_hdr, token_p, sizeof(token_hdr));
 	}
@@ -276,7 +287,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
@@ -317,6 +327,7 @@ cmdline_parse(struct cmdline *cl, const char * buf)
 
 	/* parse it !! */
 	inst = ctx[inst_num];
+	dyn_tokens[0] = NULL;
 	while (inst) {
 		debug_printf("INST %d\n", inst_num);
 
@@ -354,6 +365,7 @@ cmdline_parse(struct cmdline *cl, const char * buf)
 
 		inst_num ++;
 		inst = ctx[inst_num];
+		dyn_tokens[0] = NULL;
 	}
 
 	/* call func */
@@ -400,7 +412,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++) {
@@ -421,6 +432,7 @@ cmdline_complete(struct cmdline *cl, const char *buf, int *state,
 		nb_non_completable = 0;
 
 		inst = ctx[inst_num];
+		dyn_tokens[0] = NULL;
 		while (inst) {
 			/* parse the first tokens of the inst */
 			if (nb_token &&
@@ -429,18 +441,7 @@ cmdline_complete(struct cmdline *cl, const char *buf, int *state,
 				goto next;
 
 			debug_printf("instruction match\n");
-			if (!inst->tokens[0]) {
-				if (nb_token <
-				    (CMDLINE_PARSE_DYNAMIC_TOKENS - 1)) {
-					if (!dyn_tokens[nb_token])
-						inst->f(&dyn_tokens[nb_token],
-							NULL,
-							&dyn_tokens);
-					token_p = dyn_tokens[nb_token];
-				} else
-					token_p = NULL;
-			} else
-				token_p = inst->tokens[nb_token];
+			token_p = get_token(inst, nb_token, &dyn_tokens);
 			if (token_p)
 				memcpy(&token_hdr, token_p, sizeof(token_hdr));
 
@@ -494,6 +495,7 @@ cmdline_complete(struct cmdline *cl, const char *buf, int *state,
 			debug_printf("next\n");
 			inst_num ++;
 			inst = ctx[inst_num];
+			dyn_tokens[0] = NULL;
 		}
 
 		debug_printf("total choices %d for this completion\n",
@@ -526,6 +528,7 @@ cmdline_complete(struct cmdline *cl, const char *buf, int *state,
 
 	inst_num = 0;
 	inst = ctx[inst_num];
+	dyn_tokens[0] = NULL;
 	while (inst) {
 		/* we need to redo it */
 		inst = ctx[inst_num];
@@ -534,17 +537,7 @@ cmdline_complete(struct cmdline *cl, const char *buf, int *state,
 		    match_inst(inst, buf, nb_token, NULL, 0, &dyn_tokens))
 			goto next2;
 
-		if (!inst->tokens[0]) {
-			if (nb_token < (CMDLINE_PARSE_DYNAMIC_TOKENS - 1)) {
-				if (!dyn_tokens[nb_token])
-					inst->f(&dyn_tokens[nb_token],
-						NULL,
-						&dyn_tokens);
-				token_p = dyn_tokens[nb_token];
-			} else
-				token_p = NULL;
-		} else
-			token_p = inst->tokens[nb_token];
+		token_p = get_token(inst, nb_token, &dyn_tokens);
 		if (token_p)
 			memcpy(&token_hdr, token_p, sizeof(token_hdr));
 
@@ -617,6 +610,7 @@ cmdline_complete(struct cmdline *cl, const char *buf, int *state,
 	next2:
 		inst_num ++;
 		inst = ctx[inst_num];
+		dyn_tokens[0] = NULL;
 	}
 	return 0;
 }
-- 
2.1.4

  parent reply	other threads:[~2017-07-10 12:10 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-13 10:07 [dpdk-stable] [PATCH] librte_cmdline: fix parsing initialisation Bernard Iremonger
2017-06-15 10:15 ` [dpdk-stable] [PATCH v2] " Adrien Mazarguil
2017-06-23  9:04   ` Olivier Matz
     [not found]   ` <cover.1499687422.git.adrien.mazarguil@6wind.com>
2017-07-10 12:09     ` Adrien Mazarguil [this message]
2017-07-10 12:09     ` [dpdk-stable] [PATCH v3 2/3] cmdline: fix dynamic tokens interface Adrien Mazarguil
2017-07-10 12:09     ` [dpdk-stable] [PATCH v3 3/3] app/testpmd: fix token matching in flow command Adrien Mazarguil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=784ee3797f60d0e1fc7d6aaa009812a5bf77cdd8.1499687422.git.adrien.mazarguil@6wind.com \
    --to=adrien.mazarguil@6wind.com \
    --cc=bernard.iremonger@intel.com \
    --cc=dev@dpdk.org \
    --cc=olivier.matz@6wind.com \
    --cc=stable@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).