[dpdk-dev] [PATCH v2] lib/cmdline: init CLI parsing memory

[Xueming(Steven) Li]

No problem, make enough sense for v3.

> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com]
> Sent: Monday, December 18, 2017 6:51 PM
> To: Olivier MATZ <olivier.matz at 6wind.com>
> Cc: Xueming(Steven) Li <xuemingl at mellanox.com>; dev at dpdk.org
> Subject: Re: [PATCH v2] lib/cmdline: init CLI parsing memory
> 
> On Thu, Dec 14, 2017 at 04:35:45PM +0100, Olivier MATZ wrote:
> > Hi Xueming,
> >
> > On Sat, Dec 09, 2017 at 11:39:23PM +0800, Xueming Li wrote:
> > > Initialize result memory every time before parsing. Also save
> > > successfully parsed result before further ambiguous command
> > > detection to avoid result being tainted by later parsing.
> > >
> > > Signed-off-by: Xueming Li <xuemingl at mellanox.com>
> >
> > I'm ok with the content of the patch, but this has 2 be split in 2
> > commits, which fixes different things.
> >
> > 1/ cmdline: fix dynamic tokens parsing
> >
> >    [contains what Adrien suggested = all your patch but memset]
> >
> >    When using dynamic tokens, the result buffer contains pointers
> >    to some location inside the result buffer. When the content of
> >    the temporary buffer is copied in the final one, these pointers
> >    still point to the temporary buffer.
> >
> >    This works until the temporary buffer is kept intact, but the
> >    next commit introduces a memset() that breaks this assumption.
> >
> >    This commit renames the buffers, and ensures that the pointers
> >    point to the valid location, by recopying the buffer before
> >    invoking f().
> >
> >    Fixes: 9b3fbb051d2e ("cmdline: fix parsing")
> >    Cc: stable at dpdk.org
> >
> >
> > 2/ cmdline: avoid garbage in unused fields of parsed result
> >
> >    [contains the memset() only]
> >
> >    The result buffer was not initialized before parsing, inducing
> >    garbage in unused fields or padding of the parsed structure.
> >
> >    Initialize the result buffer each time before parsing.
> >
> >    Fixes: af75078fece3 ("first public release")
> >    Cc: stable at dpdk.org
> >
> >
> > Thoughts?
> > Adrien, are you also ok?
> 
> Yes I fully agree, splitting this in two patches is also what I had in
> mind.
> Xueming, do you plan to submit v3 accordingly?
> 
> --
> Adrien Mazarguil
> 6WIND