No problem, make enough sense for v3.
> -----Original Message----- > From: Adrien Mazarguil [mailto:adrien.mazarg...@6wind.com] > Sent: Monday, December 18, 2017 6:51 PM > To: Olivier MATZ <olivier.m...@6wind.com> > Cc: Xueming(Steven) Li <xuemi...@mellanox.com>; dev@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 <xuemi...@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: sta...@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: sta...@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
