[dpdk-dev,v1,1/1] cmdline: add any multi string mode to token string
Commit Message
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(-)
Comments
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
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
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
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
@@ -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;
}
@@ -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)
{
@@ -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
@@ -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
@@ -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 */ \