[dpdk-dev] [PATCH v1 1/1] cmdline: add any multi string mode to token string
Hi Olivier, > -Original Message- > From: Olivier Matz [mailto:olivier.matz at 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: > > > > > 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 v1 1/1] cmdline: add any multi string mode to token string
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: > > > 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
[dpdk-dev] [PATCH v1 1/1] cmdline: add any multi string mode to token string
Hi Olivier, > -Original Message- > From: Olivier Matz [mailto:olivier.matz at 6wind.com] > Sent: Monday, April 4, 2016 10:01 AM > To: Azarewicz, PiotrX T > Cc: dev at 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. > 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: > 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
[dpdk-dev] [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. 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 xxx foo=xxx, args=xxx That's why snprintf() should still use STR_TOKEN_SIZE. Regards, Olivier
[dpdk-dev] [PATCH v1 1/1] cmdline: add any multi string mode to token string
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 --- 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(bu