Re: [PATCH v2] lib/cmdline: init CLI parsing memory

[Adrien Mazarguil <adrien.mazarguil@6wind.com>]

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@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@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@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