From patchwork Mon Jul 10 12:09:34 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adrien Mazarguil X-Patchwork-Id: 26724 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 4667A7CAB; Mon, 10 Jul 2017 14:10:17 +0200 (CEST) Received: from mail-wr0-f173.google.com (mail-wr0-f173.google.com [209.85.128.173]) by dpdk.org (Postfix) with ESMTP id 9EEDD5699 for ; Mon, 10 Jul 2017 14:10:13 +0200 (CEST) Received: by mail-wr0-f173.google.com with SMTP id 77so135988501wrb.1 for ; Mon, 10 Jul 2017 05:10:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=10f2VwDRlB4U3KsaE9OfG21FolWa73DwjMHZ6fqUFrU=; b=um8fKoqY5Qxc0tnG9LH/US8Mbr3toGu6FSOYleJKXA1K+VfoJgv9WtG9G4sGp3y1aK NmN9qqzdQJS3en/1RFXg3GFSg9RnbuHFc2Og+c9pNgvgiuAsK7v/qjA6qV+hE077+Bi5 nSFFIyUeFTlwFbOjjF5DtsIrKbucdwrnS+AtnJJp0fEEwyTpkziHPwg+ytqpZLI8/05k zOpr2reFK+Sakjm800fnLHPmLcoUpLIsShsIOUYVEzSoEQrNrFU3XseqR/qgKCgCWTNz MTEyTPx5ctKkR+LPpBC1dU/u2HwC2tjWUL/AxHjOMaZsX26D/CmyxHTe1EtA0+yBoTjK I8MQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=10f2VwDRlB4U3KsaE9OfG21FolWa73DwjMHZ6fqUFrU=; b=hTsa8BJ1SoNLx+SFpJI8WgjCGVMz6HZmh0/Is/sC0IwtBb7/p2/+bNf9Fs0fMLetLo ejSTExhLDxq64APLAQhcMqSSQO5zw3zprrzQMrnekwJKosGZ7XPuhA329t2oEyHPD2dv FIeqtTZdm0OUKNq8JcaHinu0CFOhREXYfj9z/9dPn/KzEIYWkSz/2ayLB7c5hDE9dDWQ vfBdnYxx3EOXWh3erHuYLI+fuWd+nZ8nQUYkVqdi38WgtifjkV5yOomzIf8FQbAkUHPZ Wnb47wYNWTK+1P2y42q2Ap8NAkLdqDNVRZpO3M9pbSibG2TcQwiZr4c96W02FGeaI9w2 JzOg== X-Gm-Message-State: AIVw110kAet47UrhGDLPacT+iKwrgCKwFY9fyAKm8zlzw8gvRwKJ7M5p zycAfzAQP2ZukjnG X-Received: by 10.28.67.6 with SMTP id q6mr7735338wma.6.1499688613304; Mon, 10 Jul 2017 05:10:13 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id o4sm13607436wrb.27.2017.07.10.05.10.12 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Mon, 10 Jul 2017 05:10:12 -0700 (PDT) From: Adrien Mazarguil To: Olivier Matz Cc: dev@dpdk.org, stable@dpdk.org, Bernard Iremonger Date: Mon, 10 Jul 2017 14:09:34 +0200 Message-Id: <784ee3797f60d0e1fc7d6aaa009812a5bf77cdd8.1499687422.git.adrien.mazarguil@6wind.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: References: <327b5be12221f51fbf3a6d8e9d155de786992388.1497521374.git.adrien.mazarguil@6wind.com> Subject: [dpdk-dev] [PATCH v3 1/3] cmdline: fix dynamic tokens initialization 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 Signed-off-by: Adrien Mazarguil Acked-by: Olivier Matz --- 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; }