[dpdk-dev] [PATCH v1 1/1] cmdline: add any multi string mode to token string

[dpdk-dev] [PATCH v1 1/1] cmdline: add any multi string mode to token string

[Piotr Azarewicz]

While parsing token string there may be several modes:
- fixed single string
- multi-choice single string
- any single string

This patch add one more mode - any multi string.

Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
---
 app/test/test_cmdline_string.c            |    9 ++++---
 lib/librte_cmdline/cmdline_parse.c        |    8 ++++++
 lib/librte_cmdline/cmdline_parse.h        |    3 +++
 lib/librte_cmdline/cmdline_parse_string.c |   41 ++++++++++++++++++++---------
 lib/librte_cmdline/cmdline_parse_string.h |    2 ++
 5 files changed, 48 insertions(+), 15 deletions(-)

diff --git a/app/test/test_cmdline_string.c b/app/test/test_cmdline_string.c
index 915a7d7..96f0e20 100644
--- a/app/test/test_cmdline_string.c
+++ b/app/test/test_cmdline_string.c
@@ -97,6 +97,11 @@ struct string_parse_str string_parse_strs[] = {
 		{"two with\rgarbage\tcharacters\n",
 				"one#two with\rgarbage\tcharacters\n#three",
 				"two with\rgarbage\tcharacters\n"},
+		{"one two", "one", "one"}, /* fixed string */
+		{"one two", TOKEN_STRING_MULTI, "one two"}, /* multi string */
+		{"one two", NULL, "one"}, /* any string */
+		{"one two #three", TOKEN_STRING_MULTI, "one two "},
+		/* multi string with comment */
 };
 
 
@@ -124,7 +129,6 @@ struct string_invalid_str string_invalid_strs[] = {
 		 "toolong!!!toolong!!!toolong!!!toolong!!!toolong!!!toolong!!!"
 		 "toolong!!!toolong!!!toolong!!!toolong!!!toolong!!!toolong!!!"
 		 "toolong!!!" },
-		 {"invalid", ""},
 		 {"", "invalid"}
 };
 
@@ -350,8 +354,7 @@ test_parse_string_valid(void)
 					string_parse_strs[i].str, help_str);
 			return -1;
 		}
-		if (strncmp(buf, string_parse_strs[i].result,
-				sizeof(string_parse_strs[i].result) - 1) != 0) {
+		if (strcmp(buf, string_parse_strs[i].result) != 0) {
 			printf("Error: result mismatch!\n");
 			return -1;
 		}
diff --git a/lib/librte_cmdline/cmdline_parse.c b/lib/librte_cmdline/cmdline_parse.c
index 24a6ed6..b496067 100644
--- a/lib/librte_cmdline/cmdline_parse.c
+++ b/lib/librte_cmdline/cmdline_parse.c
@@ -118,6 +118,14 @@ cmdline_isendoftoken(char c)
 	return 0;
 }
 
+int
+cmdline_isendofcommand(char c)
+{
+	if (!c || iscomment(c) || isendofline(c))
+		return 1;
+	return 0;
+}
+
 static unsigned int
 nb_common_chars(const char * s1, const char * s2)
 {
diff --git a/lib/librte_cmdline/cmdline_parse.h b/lib/librte_cmdline/cmdline_parse.h
index 4b25c45..4ac05d6 100644
--- a/lib/librte_cmdline/cmdline_parse.h
+++ b/lib/librte_cmdline/cmdline_parse.h
@@ -184,6 +184,9 @@ int cmdline_complete(struct cmdline *cl, const char *buf, int *state,
  * isendofline(c)) */
 int cmdline_isendoftoken(char c);
 
+/* return true if(!c || iscomment(c) || isendofline(c)) */
+int cmdline_isendofcommand(char c);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_cmdline/cmdline_parse_string.c b/lib/librte_cmdline/cmdline_parse_string.c
index 45883b3..274c9a3 100644
--- a/lib/librte_cmdline/cmdline_parse_string.c
+++ b/lib/librte_cmdline/cmdline_parse_string.c
@@ -76,9 +76,10 @@ struct cmdline_token_ops cmdline_token_string_ops = {
 	.get_help = cmdline_get_help_string,
 };
 
-#define MULTISTRING_HELP "Mul-choice STRING"
-#define ANYSTRING_HELP   "Any STRING"
-#define FIXEDSTRING_HELP "Fixed STRING"
+#define CHOICESTRING_HELP "Mul-choice STRING"
+#define ANYSTRING_HELP    "Any STRING"
+#define ANYSTRINGS_HELP   "Any STRINGS"
+#define FIXEDSTRING_HELP  "Fixed STRING"
 
 static unsigned int
 get_token_len(const char *s)
@@ -123,8 +124,8 @@ cmdline_parse_string(cmdline_parse_token_hdr_t *tk, const char *buf, void *res,
 
 	sd = &tk2->string_data;
 
-	/* fixed string */
-	if (sd->str) {
+	/* fixed string (known single token) */
+	if ((sd->str != NULL) && (strcmp(sd->str, TOKEN_STRING_MULTI) != 0)) {
 		str = sd->str;
 		do {
 			token_len = get_token_len(str);
@@ -148,7 +149,18 @@ cmdline_parse_string(cmdline_parse_token_hdr_t *tk, const char *buf, void *res,
 		if (!str)
 			return -1;
 	}
-	/* unspecified string */
+	/* multi string */
+	else if (sd->str != NULL) {
+		token_len = 0;
+		while (!cmdline_isendofcommand(buf[token_len]) &&
+		      token_len < (ressize - 1))
+			token_len++;
+
+		/* return if token too long */
+		if (token_len >= (ressize - 1))
+			return -1;
+	}
+	/* unspecified string (unknown single token) */
 	else {
 		token_len = 0;
 		while(!cmdline_isendoftoken(buf[token_len]) &&
@@ -162,12 +174,15 @@ cmdline_parse_string(cmdline_parse_token_hdr_t *tk, const char *buf, void *res,
 	}
 
 	if (res) {
-		/* we are sure that token_len is < STR_TOKEN_SIZE-1 */
-		snprintf(res, STR_TOKEN_SIZE, "%s", buf);
-		*((char *)res + token_len) = 0;
+		if ((sd->str != NULL) && (strcmp(sd->str, TOKEN_STRING_MULTI) == 0))
+			snprintf(res, token_len + 1, "%s", buf);
+		else {
+			/* we are sure that token_len is < STR_TOKEN_SIZE-1 */
+			snprintf(res, STR_TOKEN_SIZE, "%s", buf);
+			*((char *)res + token_len) = 0;
+		}
 	}
 
-
 	return token_len;
 }
 
@@ -242,8 +257,10 @@ int cmdline_get_help_string(cmdline_parse_token_hdr_t *tk, char *dstbuf,
 	s = sd->str;
 
 	if (s) {
-		if (get_next_token(s))
-			snprintf(dstbuf, size, MULTISTRING_HELP);
+		if (strcmp(s, TOKEN_STRING_MULTI) == 0)
+			snprintf(dstbuf, size, ANYSTRINGS_HELP);
+		else if (get_next_token(s))
+			snprintf(dstbuf, size, CHOICESTRING_HELP);
 		else
 			snprintf(dstbuf, size, FIXEDSTRING_HELP);
 	} else
diff --git a/lib/librte_cmdline/cmdline_parse_string.h b/lib/librte_cmdline/cmdline_parse_string.h
index 94aa1f1..6dde269 100644
--- a/lib/librte_cmdline/cmdline_parse_string.h
+++ b/lib/librte_cmdline/cmdline_parse_string.h
@@ -92,6 +92,8 @@ int cmdline_complete_get_elt_string(cmdline_parse_token_hdr_t *tk, int idx,
 int cmdline_get_help_string(cmdline_parse_token_hdr_t *tk, char *dstbuf,
 			    unsigned int size);
 
+#define TOKEN_STRING_MULTI ""
+
 #define TOKEN_STRING_INITIALIZER(structure, field, string)  \
 {                                                           \
 	/* hdr */                                               \
-- 
1.7.9.5

Re: [dpdk-dev] [PATCH v1 1/1] cmdline: add any multi string mode to token string

[Olivier Matz]

Hi Piotr,

This is globally ok for me. Please see a comment below.

On 04/01/2016 01:36 PM, Piotr Azarewicz wrote:
> @@ -162,12 +174,15 @@ cmdline_parse_string(cmdline_parse_token_hdr_t *tk, const char *buf, void *res,
>  	}
>  
>  	if (res) {
> -		/* we are sure that token_len is < STR_TOKEN_SIZE-1 */
> -		snprintf(res, STR_TOKEN_SIZE, "%s", buf);
> -		*((char *)res + token_len) = 0;
> +		if ((sd->str != NULL) && (strcmp(sd->str, TOKEN_STRING_MULTI) == 0))
> +			snprintf(res, token_len + 1, "%s", buf);
> +		else {
> +			/* we are sure that token_len is < STR_TOKEN_SIZE-1 */
> +			snprintf(res, STR_TOKEN_SIZE, "%s", buf);
> +			*((char *)res + token_len) = 0;
> +		}
>  	}
>  

Using token_len + 1 as the buffer size in the snprintf looks a
bit dangerous, as it won't protect from overflows.

See the following example:


struct cmd_foo_result {
	cmdline_fixed_string_t args;
        cmdline_fixed_string_t foo;
};

static void
cmd_foo_parsed(void *parsed_result,
        __rte_unused struct cmdline *cl,
        __rte_unused void *data)
{
        struct cmd_foo_result *res = parsed_result;
        printf("foo=%s, args=%s\n", res->foo, res->args);
}

cmdline_parse_token_string_t cmd_foo_foo =
        TOKEN_STRING_INITIALIZER(struct cmd_foo_result, foo,
                                 "foo");
cmdline_parse_token_string_t cmd_foo_args =
        TOKEN_STRING_INITIALIZER(struct cmd_foo_result, args,
                TOKEN_STRING_MULTI);

cmdline_parse_inst_t cmd_foo = {
        .f = cmd_foo_parsed,  /* function to call */
        .data = NULL,      /* 2nd arg of func */
        .help_str = "test",
        .tokens = {        /* token list, NULL terminated */
                (void *)&cmd_foo_foo,
                (void *)&cmd_foo_args,
                NULL,
        },
};


The result will be:

# ok
RTE>>foo xxx
foo=foo, args=xxx

# not ok, args overflows in foo
RTE>>foo
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
foo=xxxxxxxxxxxxxxxxxxxxxxx,
args=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx


That's why snprintf() should still use STR_TOKEN_SIZE.


Regards,
Olivier

Re: [dpdk-dev] [PATCH v1 1/1] cmdline: add any multi string mode to token string

[Azarewicz, PiotrX T]

Hi Olivier,

> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Monday, April 4, 2016 10:01 AM
> To: Azarewicz, PiotrX T <piotrx.t.azarewicz@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v1 1/1] cmdline: add any multi string mode to token
> string
> 
> Hi Piotr,
> 
> This is globally ok for me. Please see a comment below.
> 
Good to know it, thanks.

<snip>

> Using token_len + 1 as the buffer size in the snprintf looks a bit dangerous, as
> it won't protect from overflows.
> 
> See the following example:
 <snip>
 > That's why snprintf() should still use STR_TOKEN_SIZE.
>
Okay, I see it.
But this is a problem that we may need longer string than STR_TOKEN_SIZE in multi token case.
So what you think about adding new typedef cmdline_multi_string_t for this case?
For example:
#define STR_MULTI_TOKEN_SIZE 1024
typedef char cmdline_multi_string_t[STR_MULTI_TOKEN_SIZE];

> 
> Regards,
> Olivier

Re: [dpdk-dev] [PATCH v1 1/1] cmdline: add any multi string mode to token string

[Olivier Matz]

Hi Piotr,

On 04/04/2016 04:11 PM, Azarewicz, PiotrX T wrote:
>> Using token_len + 1 as the buffer size in the snprintf looks a bit dangerous, as
>> it won't protect from overflows.
>>
>> See the following example:
>  <snip>
>  > That's why snprintf() should still use STR_TOKEN_SIZE.
>>
> Okay, I see it.
> But this is a problem that we may need longer string than STR_TOKEN_SIZE in multi token case.
> So what you think about adding new typedef cmdline_multi_string_t for this case?
> For example:
> #define STR_MULTI_TOKEN_SIZE 1024
> typedef char cmdline_multi_string_t[STR_MULTI_TOKEN_SIZE];

It should do the job, indeed.

By the way, it would be nice to have an example of use.

Regards,
Olivier

Re: [dpdk-dev] [PATCH v1 1/1] cmdline: add any multi string mode to token string

[Azarewicz, PiotrX T]

Hi Olivier,

> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Monday, April 4, 2016 5:58 PM
> >> Using token_len + 1 as the buffer size in the snprintf looks a bit
> >> dangerous, as it won't protect from overflows.
> >>
> >> See the following example:
> >  <snip>
> >  > That's why snprintf() should still use STR_TOKEN_SIZE.
> >>
> > Okay, I see it.
> > But this is a problem that we may need longer string than STR_TOKEN_SIZE
> in multi token case.
> > So what you think about adding new typedef cmdline_multi_string_t for
> this case?
> > For example:
> > #define STR_MULTI_TOKEN_SIZE 1024
> > typedef char cmdline_multi_string_t[STR_MULTI_TOKEN_SIZE];
> 
> It should do the job, indeed.

That's great.
We want to set the value of the buffer to 4096, to not to regret in the future.

> By the way, it would be nice to have an example of use.

Based on this we plan a lot of changes in ip_pipeline example next release.

Regards,
Piotr

[dpdk-dev] [PATCH v2 1/1] cmdline: add any multi string mode to token string

[Piotr Azarewicz]

While parsing token string there may be several modes:
- fixed single string
- multi-choice single string
- any single string

This patch add one more mode - any multi string.

Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
---

v2 changes:
- add cmdline_multi_string_t type for the new mode

 app/test/test_cmdline_string.c            |   15 ++++++----
 lib/librte_cmdline/cmdline_parse.c        |    8 ++++++
 lib/librte_cmdline/cmdline_parse.h        |    3 ++
 lib/librte_cmdline/cmdline_parse_string.c |   43 +++++++++++++++++++++--------
 lib/librte_cmdline/cmdline_parse_string.h |    7 +++++
 5 files changed, 60 insertions(+), 16 deletions(-)

diff --git a/app/test/test_cmdline_string.c b/app/test/test_cmdline_string.c
index 915a7d7..c5bb9c0 100644
--- a/app/test/test_cmdline_string.c
+++ b/app/test/test_cmdline_string.c
@@ -35,6 +35,7 @@
 #include <string.h>
 #include <inttypes.h>
 
+#include <rte_common.h>
 #include <rte_string_fns.h>
 
 #include <cmdline_parse.h>
@@ -65,9 +66,10 @@ struct string_elt_str string_elt_strs[] = {
 		{"one#two\nwith\nnewlines#three", "two\nwith\nnewlines", 1},
 };
 
-#if CMDLINE_TEST_BUFSIZE < STR_TOKEN_SIZE
+#if (CMDLINE_TEST_BUFSIZE < STR_TOKEN_SIZE) \
+|| (CMDLINE_TEST_BUFSIZE < STR_MULTI_TOKEN_SIZE)
 #undef CMDLINE_TEST_BUFSIZE
-#define CMDLINE_TEST_BUFSIZE STR_TOKEN_SIZE
+#define CMDLINE_TEST_BUFSIZE RTE_MAX(STR_TOKEN_SIZE, STR_MULTI_TOKEN_SIZE)
 #endif
 
 struct string_nb_str {
@@ -97,6 +99,11 @@ struct string_parse_str string_parse_strs[] = {
 		{"two with\rgarbage\tcharacters\n",
 				"one#two with\rgarbage\tcharacters\n#three",
 				"two with\rgarbage\tcharacters\n"},
+		{"one two", "one", "one"}, /* fixed string */
+		{"one two", TOKEN_STRING_MULTI, "one two"}, /* multi string */
+		{"one two", NULL, "one"}, /* any string */
+		{"one two #three", TOKEN_STRING_MULTI, "one two "},
+		/* multi string with comment */
 };
 
 
@@ -124,7 +131,6 @@ struct string_invalid_str string_invalid_strs[] = {
 		 "toolong!!!toolong!!!toolong!!!toolong!!!toolong!!!toolong!!!"
 		 "toolong!!!toolong!!!toolong!!!toolong!!!toolong!!!toolong!!!"
 		 "toolong!!!" },
-		 {"invalid", ""},
 		 {"", "invalid"}
 };
 
@@ -350,8 +356,7 @@ test_parse_string_valid(void)
 					string_parse_strs[i].str, help_str);
 			return -1;
 		}
-		if (strncmp(buf, string_parse_strs[i].result,
-				sizeof(string_parse_strs[i].result) - 1) != 0) {
+		if (strcmp(buf, string_parse_strs[i].result) != 0) {
 			printf("Error: result mismatch!\n");
 			return -1;
 		}
diff --git a/lib/librte_cmdline/cmdline_parse.c b/lib/librte_cmdline/cmdline_parse.c
index 24a6ed6..b496067 100644
--- a/lib/librte_cmdline/cmdline_parse.c
+++ b/lib/librte_cmdline/cmdline_parse.c
@@ -118,6 +118,14 @@ cmdline_isendoftoken(char c)
 	return 0;
 }
 
+int
+cmdline_isendofcommand(char c)
+{
+	if (!c || iscomment(c) || isendofline(c))
+		return 1;
+	return 0;
+}
+
 static unsigned int
 nb_common_chars(const char * s1, const char * s2)
 {
diff --git a/lib/librte_cmdline/cmdline_parse.h b/lib/librte_cmdline/cmdline_parse.h
index 4b25c45..4ac05d6 100644
--- a/lib/librte_cmdline/cmdline_parse.h
+++ b/lib/librte_cmdline/cmdline_parse.h
@@ -184,6 +184,9 @@ int cmdline_complete(struct cmdline *cl, const char *buf, int *state,
  * isendofline(c)) */
 int cmdline_isendoftoken(char c);
 
+/* return true if(!c || iscomment(c) || isendofline(c)) */
+int cmdline_isendofcommand(char c);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_cmdline/cmdline_parse_string.c b/lib/librte_cmdline/cmdline_parse_string.c
index 45883b3..35917a7 100644
--- a/lib/librte_cmdline/cmdline_parse_string.c
+++ b/lib/librte_cmdline/cmdline_parse_string.c
@@ -76,9 +76,10 @@ struct cmdline_token_ops cmdline_token_string_ops = {
 	.get_help = cmdline_get_help_string,
 };
 
-#define MULTISTRING_HELP "Mul-choice STRING"
-#define ANYSTRING_HELP   "Any STRING"
-#define FIXEDSTRING_HELP "Fixed STRING"
+#define CHOICESTRING_HELP "Mul-choice STRING"
+#define ANYSTRING_HELP    "Any STRING"
+#define ANYSTRINGS_HELP   "Any STRINGS"
+#define FIXEDSTRING_HELP  "Fixed STRING"
 
 static unsigned int
 get_token_len(const char *s)
@@ -123,8 +124,8 @@ cmdline_parse_string(cmdline_parse_token_hdr_t *tk, const char *buf, void *res,
 
 	sd = &tk2->string_data;
 
-	/* fixed string */
-	if (sd->str) {
+	/* fixed string (known single token) */
+	if ((sd->str != NULL) && (strcmp(sd->str, TOKEN_STRING_MULTI) != 0)) {
 		str = sd->str;
 		do {
 			token_len = get_token_len(str);
@@ -148,7 +149,21 @@ cmdline_parse_string(cmdline_parse_token_hdr_t *tk, const char *buf, void *res,
 		if (!str)
 			return -1;
 	}
-	/* unspecified string */
+	/* multi string */
+	else if (sd->str != NULL) {
+		if (ressize < STR_MULTI_TOKEN_SIZE)
+			return -1;
+
+		token_len = 0;
+		while (!cmdline_isendofcommand(buf[token_len]) &&
+		      token_len < (STR_MULTI_TOKEN_SIZE - 1))
+			token_len++;
+
+		/* return if token too long */
+		if (token_len >= (STR_MULTI_TOKEN_SIZE - 1))
+			return -1;
+	}
+	/* unspecified string (unknown single token) */
 	else {
 		token_len = 0;
 		while(!cmdline_isendoftoken(buf[token_len]) &&
@@ -162,12 +177,16 @@ cmdline_parse_string(cmdline_parse_token_hdr_t *tk, const char *buf, void *res,
 	}
 
 	if (res) {
-		/* we are sure that token_len is < STR_TOKEN_SIZE-1 */
-		snprintf(res, STR_TOKEN_SIZE, "%s", buf);
+		if ((sd->str != NULL) && (strcmp(sd->str, TOKEN_STRING_MULTI) == 0))
+			/* we are sure that token_len is < STR_MULTI_TOKEN_SIZE-1 */
+			snprintf(res, STR_MULTI_TOKEN_SIZE, "%s", buf);
+		else
+			/* we are sure that token_len is < STR_TOKEN_SIZE-1 */
+			snprintf(res, STR_TOKEN_SIZE, "%s", buf);
+
 		*((char *)res + token_len) = 0;
 	}
 
-
 	return token_len;
 }
 
@@ -242,8 +261,10 @@ int cmdline_get_help_string(cmdline_parse_token_hdr_t *tk, char *dstbuf,
 	s = sd->str;
 
 	if (s) {
-		if (get_next_token(s))
-			snprintf(dstbuf, size, MULTISTRING_HELP);
+		if (strcmp(s, TOKEN_STRING_MULTI) == 0)
+			snprintf(dstbuf, size, ANYSTRINGS_HELP);
+		else if (get_next_token(s))
+			snprintf(dstbuf, size, CHOICESTRING_HELP);
 		else
 			snprintf(dstbuf, size, FIXEDSTRING_HELP);
 	} else
diff --git a/lib/librte_cmdline/cmdline_parse_string.h b/lib/librte_cmdline/cmdline_parse_string.h
index 94aa1f1..3ceac57 100644
--- a/lib/librte_cmdline/cmdline_parse_string.h
+++ b/lib/librte_cmdline/cmdline_parse_string.h
@@ -70,8 +70,13 @@ extern "C" {
 /* size of a parsed string */
 #define STR_TOKEN_SIZE 128
 
+/* size of a parsed multi string */
+#define STR_MULTI_TOKEN_SIZE 4096
+
 typedef char cmdline_fixed_string_t[STR_TOKEN_SIZE];
 
+typedef char cmdline_multi_string_t[STR_MULTI_TOKEN_SIZE];
+
 struct cmdline_token_string_data {
 	const char *str;
 };
@@ -92,6 +97,8 @@ int cmdline_complete_get_elt_string(cmdline_parse_token_hdr_t *tk, int idx,
 int cmdline_get_help_string(cmdline_parse_token_hdr_t *tk, char *dstbuf,
 			    unsigned int size);
 
+#define TOKEN_STRING_MULTI ""
+
 #define TOKEN_STRING_INITIALIZER(structure, field, string)  \
 {                                                           \
 	/* hdr */                                               \
-- 
1.7.9.5

Re: [dpdk-dev] [PATCH v2 1/1] cmdline: add any multi string mode to token string

[Olivier Matz]

On 04/05/2016 10:47 AM, Piotr Azarewicz wrote:
> While parsing token string there may be several modes:
> - fixed single string
> - multi-choice single string
> - any single string
> 
> This patch add one more mode - any multi string.
> 
> Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>

Acked-by: Olivier Matz <olivier.matz@6wind.com>

Re: [dpdk-dev] [PATCH v2 1/1] cmdline: add any multi string mode to token string

[Thomas Monjalon]

2016-04-05 13:21, Olivier Matz:
> 
> On 04/05/2016 10:47 AM, Piotr Azarewicz wrote:
> > While parsing token string there may be several modes:
> > - fixed single string
> > - multi-choice single string
> > - any single string
> > 
> > This patch add one more mode - any multi string.
> > 
> > Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
> 
> Acked-by: Olivier Matz <olivier.matz@6wind.com>

This patch does not fix anything working currently.
As the release is closing in few days,
it will be integrated in 16.07.
Thanks

Re: [dpdk-dev] [PATCH v2 1/1] cmdline: add any multi string mode to token string

[Wiles, Keith]

>While parsing token string there may be several modes:
>- fixed single string
>- multi-choice single string
>- any single string
>
>This patch add one more mode - any multi string.
>
>Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>

Does this patch also require updates to the documentation?
>

Regards,
Keith

Re: [dpdk-dev] [PATCH v2 1/1] cmdline: add any multi string mode to token string

[Olivier MATZ]

Hi Keith,

On 04/15/2016 04:41 PM, Wiles, Keith wrote:
>> While parsing token string there may be several modes:
>> - fixed single string
>> - multi-choice single string
>> - any single string
>>
>> This patch add one more mode - any multi string.
>>
>> Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
>
> Does this patch also require updates to the documentation?

I did a quick check and it seems we do not dive as far in the
command line documentation (guides/sample_app_ug/cmd_line.rst).

But I agree that a comment could be added above the definition
of TOKEN_STRING_MULTI that would explain what is the behavior
in that case.

Piotr, do you think this is something you can do?

Thanks,
Olivier

Re: [dpdk-dev] [PATCH v2 1/1] cmdline: add any multi string mode to token string

[Azarewicz, PiotrX T]

> But I agree that a comment could be added above the definition of
> TOKEN_STRING_MULTI that would explain what is the behavior in that case.
> 
> Piotr, do you think this is something you can do?

Okay, I will.

Regards,
Piotr

[dpdk-dev] [PATCH v3 1/1] cmdline: add any multi string mode to token string

[Piotr Azarewicz]

While parsing token string there may be several modes:
- fixed single string
- multi-choice single string
- any single string

This patch add one more mode - any multi string.

Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
---

v3 changes:
- add a comment above the definiton of TOKEN_STRING_MULTI

v2 changes:
- add cmdline_multi_string_t type for the new mode

 app/test/test_cmdline_string.c            |   15 ++++++----
 lib/librte_cmdline/cmdline_parse.c        |    8 ++++++
 lib/librte_cmdline/cmdline_parse.h        |    3 ++
 lib/librte_cmdline/cmdline_parse_string.c |   43 +++++++++++++++++++++--------
 lib/librte_cmdline/cmdline_parse_string.h |   15 ++++++++++
 5 files changed, 68 insertions(+), 16 deletions(-)

diff --git a/app/test/test_cmdline_string.c b/app/test/test_cmdline_string.c
index 915a7d7..c5bb9c0 100644
--- a/app/test/test_cmdline_string.c
+++ b/app/test/test_cmdline_string.c
@@ -35,6 +35,7 @@
 #include <string.h>
 #include <inttypes.h>
 
+#include <rte_common.h>
 #include <rte_string_fns.h>
 
 #include <cmdline_parse.h>
@@ -65,9 +66,10 @@ struct string_elt_str string_elt_strs[] = {
 		{"one#two\nwith\nnewlines#three", "two\nwith\nnewlines", 1},
 };
 
-#if CMDLINE_TEST_BUFSIZE < STR_TOKEN_SIZE
+#if (CMDLINE_TEST_BUFSIZE < STR_TOKEN_SIZE) \
+|| (CMDLINE_TEST_BUFSIZE < STR_MULTI_TOKEN_SIZE)
 #undef CMDLINE_TEST_BUFSIZE
-#define CMDLINE_TEST_BUFSIZE STR_TOKEN_SIZE
+#define CMDLINE_TEST_BUFSIZE RTE_MAX(STR_TOKEN_SIZE, STR_MULTI_TOKEN_SIZE)
 #endif
 
 struct string_nb_str {
@@ -97,6 +99,11 @@ struct string_parse_str string_parse_strs[] = {
 		{"two with\rgarbage\tcharacters\n",
 				"one#two with\rgarbage\tcharacters\n#three",
 				"two with\rgarbage\tcharacters\n"},
+		{"one two", "one", "one"}, /* fixed string */
+		{"one two", TOKEN_STRING_MULTI, "one two"}, /* multi string */
+		{"one two", NULL, "one"}, /* any string */
+		{"one two #three", TOKEN_STRING_MULTI, "one two "},
+		/* multi string with comment */
 };
 
 
@@ -124,7 +131,6 @@ struct string_invalid_str string_invalid_strs[] = {
 		 "toolong!!!toolong!!!toolong!!!toolong!!!toolong!!!toolong!!!"
 		 "toolong!!!toolong!!!toolong!!!toolong!!!toolong!!!toolong!!!"
 		 "toolong!!!" },
-		 {"invalid", ""},
 		 {"", "invalid"}
 };
 
@@ -350,8 +356,7 @@ test_parse_string_valid(void)
 					string_parse_strs[i].str, help_str);
 			return -1;
 		}
-		if (strncmp(buf, string_parse_strs[i].result,
-				sizeof(string_parse_strs[i].result) - 1) != 0) {
+		if (strcmp(buf, string_parse_strs[i].result) != 0) {
 			printf("Error: result mismatch!\n");
 			return -1;
 		}
diff --git a/lib/librte_cmdline/cmdline_parse.c b/lib/librte_cmdline/cmdline_parse.c
index 24a6ed6..b496067 100644
--- a/lib/librte_cmdline/cmdline_parse.c
+++ b/lib/librte_cmdline/cmdline_parse.c
@@ -118,6 +118,14 @@ cmdline_isendoftoken(char c)
 	return 0;
 }
 
+int
+cmdline_isendofcommand(char c)
+{
+	if (!c || iscomment(c) || isendofline(c))
+		return 1;
+	return 0;
+}
+
 static unsigned int
 nb_common_chars(const char * s1, const char * s2)
 {
diff --git a/lib/librte_cmdline/cmdline_parse.h b/lib/librte_cmdline/cmdline_parse.h
index 4b25c45..4ac05d6 100644
--- a/lib/librte_cmdline/cmdline_parse.h
+++ b/lib/librte_cmdline/cmdline_parse.h
@@ -184,6 +184,9 @@ int cmdline_complete(struct cmdline *cl, const char *buf, int *state,
  * isendofline(c)) */
 int cmdline_isendoftoken(char c);
 
+/* return true if(!c || iscomment(c) || isendofline(c)) */
+int cmdline_isendofcommand(char c);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_cmdline/cmdline_parse_string.c b/lib/librte_cmdline/cmdline_parse_string.c
index 45883b3..35917a7 100644
--- a/lib/librte_cmdline/cmdline_parse_string.c
+++ b/lib/librte_cmdline/cmdline_parse_string.c
@@ -76,9 +76,10 @@ struct cmdline_token_ops cmdline_token_string_ops = {
 	.get_help = cmdline_get_help_string,
 };
 
-#define MULTISTRING_HELP "Mul-choice STRING"
-#define ANYSTRING_HELP   "Any STRING"
-#define FIXEDSTRING_HELP "Fixed STRING"
+#define CHOICESTRING_HELP "Mul-choice STRING"
+#define ANYSTRING_HELP    "Any STRING"
+#define ANYSTRINGS_HELP   "Any STRINGS"
+#define FIXEDSTRING_HELP  "Fixed STRING"
 
 static unsigned int
 get_token_len(const char *s)
@@ -123,8 +124,8 @@ cmdline_parse_string(cmdline_parse_token_hdr_t *tk, const char *buf, void *res,
 
 	sd = &tk2->string_data;
 
-	/* fixed string */
-	if (sd->str) {
+	/* fixed string (known single token) */
+	if ((sd->str != NULL) && (strcmp(sd->str, TOKEN_STRING_MULTI) != 0)) {
 		str = sd->str;
 		do {
 			token_len = get_token_len(str);
@@ -148,7 +149,21 @@ cmdline_parse_string(cmdline_parse_token_hdr_t *tk, const char *buf, void *res,
 		if (!str)
 			return -1;
 	}
-	/* unspecified string */
+	/* multi string */
+	else if (sd->str != NULL) {
+		if (ressize < STR_MULTI_TOKEN_SIZE)
+			return -1;
+
+		token_len = 0;
+		while (!cmdline_isendofcommand(buf[token_len]) &&
+		      token_len < (STR_MULTI_TOKEN_SIZE - 1))
+			token_len++;
+
+		/* return if token too long */
+		if (token_len >= (STR_MULTI_TOKEN_SIZE - 1))
+			return -1;
+	}
+	/* unspecified string (unknown single token) */
 	else {
 		token_len = 0;
 		while(!cmdline_isendoftoken(buf[token_len]) &&
@@ -162,12 +177,16 @@ cmdline_parse_string(cmdline_parse_token_hdr_t *tk, const char *buf, void *res,
 	}
 
 	if (res) {
-		/* we are sure that token_len is < STR_TOKEN_SIZE-1 */
-		snprintf(res, STR_TOKEN_SIZE, "%s", buf);
+		if ((sd->str != NULL) && (strcmp(sd->str, TOKEN_STRING_MULTI) == 0))
+			/* we are sure that token_len is < STR_MULTI_TOKEN_SIZE-1 */
+			snprintf(res, STR_MULTI_TOKEN_SIZE, "%s", buf);
+		else
+			/* we are sure that token_len is < STR_TOKEN_SIZE-1 */
+			snprintf(res, STR_TOKEN_SIZE, "%s", buf);
+
 		*((char *)res + token_len) = 0;
 	}
 
-
 	return token_len;
 }
 
@@ -242,8 +261,10 @@ int cmdline_get_help_string(cmdline_parse_token_hdr_t *tk, char *dstbuf,
 	s = sd->str;
 
 	if (s) {
-		if (get_next_token(s))
-			snprintf(dstbuf, size, MULTISTRING_HELP);
+		if (strcmp(s, TOKEN_STRING_MULTI) == 0)
+			snprintf(dstbuf, size, ANYSTRINGS_HELP);
+		else if (get_next_token(s))
+			snprintf(dstbuf, size, CHOICESTRING_HELP);
 		else
 			snprintf(dstbuf, size, FIXEDSTRING_HELP);
 	} else
diff --git a/lib/librte_cmdline/cmdline_parse_string.h b/lib/librte_cmdline/cmdline_parse_string.h
index 94aa1f1..a84291b 100644
--- a/lib/librte_cmdline/cmdline_parse_string.h
+++ b/lib/librte_cmdline/cmdline_parse_string.h
@@ -70,8 +70,13 @@ extern "C" {
 /* size of a parsed string */
 #define STR_TOKEN_SIZE 128
 
+/* size of a parsed multi string */
+#define STR_MULTI_TOKEN_SIZE 4096
+
 typedef char cmdline_fixed_string_t[STR_TOKEN_SIZE];
 
+typedef char cmdline_multi_string_t[STR_MULTI_TOKEN_SIZE];
+
 struct cmdline_token_string_data {
 	const char *str;
 };
@@ -92,6 +97,16 @@ int cmdline_complete_get_elt_string(cmdline_parse_token_hdr_t *tk, int idx,
 int cmdline_get_help_string(cmdline_parse_token_hdr_t *tk, char *dstbuf,
 			    unsigned int size);
 
+/**
+* Token marked as TOKEN_STRING_MULTI takes entire parsing string
+* until “#” sign appear. Everything after “#” sign is treated as a comment.
+*
+* Note:
+* In this case second parameter of TOKEN_STRING_INITIALIZER must be a type of
+* cmdline_multi_string_t.
+*/
+#define TOKEN_STRING_MULTI ""
+
 #define TOKEN_STRING_INITIALIZER(structure, field, string)  \
 {                                                           \
 	/* hdr */                                               \
-- 
1.7.9.5

Re: [dpdk-dev] [PATCH v3 1/1] cmdline: add any multi string mode to token string

[Thomas Monjalon]

2016-04-29 16:29, Piotr Azarewicz:
> While parsing token string there may be several modes:
> - fixed single string
> - multi-choice single string
> - any single string
> 
> This patch add one more mode - any multi string.
> 
> Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
> Acked-by: Olivier Matz <olivier.matz@6wind.com>

Applied, thanks