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
