Hi Olivier, > -----Original Message----- > From: Olivier MATZ [mailto:olivier.m...@6wind.com] > Sent: Thursday, December 7, 2017 10:48 PM > To: Xueming(Steven) Li <xuemi...@mellanox.com> > Cc: dev@dpdk.org > Subject: Re: [PATCH] lib/cmdline: init parse result memeory > > On Wed, Nov 15, 2017 at 11:54:02PM +0800, Xueming Li wrote: > > Initialize binary result memory before parsing to avoid garbage in > > parsing result. > > > > Signed-off-by: Xueming Li <xuemi...@mellanox.com> > > --- > > lib/librte_cmdline/cmdline_parse.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/lib/librte_cmdline/cmdline_parse.c > > b/lib/librte_cmdline/cmdline_parse.c > > index 3e12ee54f..9124758f1 100644 > > --- a/lib/librte_cmdline/cmdline_parse.c > > +++ b/lib/librte_cmdline/cmdline_parse.c > > @@ -267,6 +267,8 @@ cmdline_parse(struct cmdline *cl, const char * buf) > > if (!cl || !buf) > > return CMDLINE_PARSE_BAD_ARGS; > > > > + memset(tmp_result.buf, 0, sizeof(tmp_result.buf)); > > + > > ctx = cl->ctx; > > > > /* > > > Did you see an issue (a bug or a crash) without the memset()? > Or is it to avoid filling unused fields in the parsed struct? Yes, I'm using same struct for some similar commands, have to avoid filling unused fields.
> > I'm not sure if your patch is enough: cmdline_parse() calls match_inst() > for each registered command. If a command partially matches (only the > first tokens), the buffer is modified. So the next one will start with a > dirty buffer. > > I suggest to put the memset() in match_inst() instead. Something like this: > > if (resbuf != NULL) > memset(resbuf, 0, resbuf_size); > > > It will reset the buffer before using it. It was there for performance concern, since the buffer could be tainted, I'll upload a new version according to your suggestion, appreciate your suggestion. > > Olivier
