Hi Xueming, Sorry for the late response.
On Tue, Dec 26, 2017 at 12:57:41PM +0000, Xueming(Steven) Li wrote: > HI Olivier, > > By reading p1 comments carefully, looks like the pointer to result buffer > issue > not resolved by result copy. How about this: > > @@ -263,6 +263,7 @@ > #ifdef RTE_LIBRTE_CMDLINE_DEBUG > char debug_buf[BUFSIZ]; > #endif > + char *result_buf = result.buf; > > if (!cl || !buf) > return CMDLINE_PARSE_BAD_ARGS; > @@ -312,16 +313,13 @@ > debug_printf("INST %d\n", inst_num); > > /* fully parsed */ > - tok = match_inst(inst, buf, 0, tmp_result.buf, > - sizeof(tmp_result.buf)); > + tok = match_inst(inst, buf, 0, result_buf, sizeof(result.buf)); If we don't use tmp_result, it is maybe better to also replace sizeof(result.buf) by CMDLINE_PARSE_RESULT_BUFSIZE > > if (tok > 0) /* we matched at least one token */ > err = CMDLINE_PARSE_BAD_ARGS; > > else if (!tok) { > debug_printf("INST fully parsed\n"); > - memcpy(&result, &tmp_result, > - sizeof(result)); > /* skip spaces */ > while (isblank2(*curbuf)) { > curbuf++; > @@ -332,6 +330,7 @@ > if (!f) { > memcpy(&f, &inst->f, sizeof(f)); > memcpy(&data, &inst->data, > sizeof(data)); > + result_buf = tmp_result.buf; > } > else { > /* more than 1 inst matches */ > I guess the issue you are talking about is the one described in "cmdline: fix dynamic tokens parsing" in my previous description? I think this patch is ok, and is probably better than the initial suggestion, because it avoids to copy the buffer. However, I don't understand why the previous patch was wrong, can you detail? Thanks Olivier