> -----Original Message----- > From: Olivier MATZ [mailto:olivier.m...@6wind.com] > Sent: Friday, December 8, 2017 9:51 PM > To: Adrien Mazarguil <adrien.mazarg...@6wind.com> > Cc: Xueming(Steven) Li <xuemi...@mellanox.com>; Wenzhuo Lu > <wenzhuo...@intel.com>; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v1] lib/cmdline: init parse result memory > > On Fri, Dec 08, 2017 at 01:27:26PM +0100, Adrien Mazarguil wrote: > > On Fri, Dec 08, 2017 at 03:02:44PM +0800, Xueming Li wrote: > > > Initialize binary result memory before parsing to avoid garbage in > > > parsing result. > > > > > > Signed-off-by: Xueming Li <xuemi...@mellanox.com> > > > > Since you chose to move the break statement, maybe the original commit > > mentioned in my previous message (9b3fbb051d2e "cmdline: fix parsing") > > can be reverted afterward? I think it makes tmp_result redundant. > > > > Wenzhuo, as the author of that commit, can you confirm? > > > > Olivier, no problem with breaking the loop immediately after the first > > successful match_inst() call instead of the last one? (I don't see why > > it would be an issue but I may have missed something) > > Moving the break will change the behavior, it will never detect ambiguous > commands (i.e when several commands match the same input). > So I think we should not do it. > We don't have CLI performance issue all the time, but recently I'm using files to save hundreds of batch 'expect' test commands, execute with a 'load' command, so performance of CLI processing becomes important. CLI developer should make sure not defining a CLI that already exists and verify the new CLI working as expected.
> IMO, only the memset() is enough. > > Regards, > Olivier
