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