> -----Original Message----- > From: Philippe Mathieu-Daudé [mailto:phi...@redhat.com] > Sent: Thursday, July 18, 2019 6:54 PM > To: Peter Maydell <peter.mayd...@linaro.org> > Cc: Zhang, Chen <chen.zh...@intel.com>; Li Zhijian <lizhij...@cn.fujitsu.com>; > Jason Wang <jasow...@redhat.com>; qemu-dev <qemu-devel@nongnu.org>; > Zhang Chen <zhangc...@gmail.com> > Subject: Re: [Qemu-devel] [PATCH V3] net/colo-compare.c: Fix memory leak > and code style issue. > > On 7/18/19 12:37 PM, Peter Maydell wrote: > > On Thu, 18 Jul 2019 at 11:28, Philippe Mathieu-Daudé <phi...@redhat.com> > wrote: > >> > >> On 7/18/19 11:22 AM, Zhang Chen wrote: > >>> From: Zhang Chen <chen.zh...@intel.com> > >>> > >>> This patch to fix the origin "char *data" menory leak, code style > >>> issue > >> > >> "memory"
Oh, I will fix this typo in next version. > >> > >>> and add necessary check here. > >>> Reported-by: Coverity (CID 1402785) > >>> > >>> Signed-off-by: Zhang Chen <chen.zh...@intel.com> > >>> --- > >>> net/colo-compare.c | 28 +++++++++++++++++++++------- > >>> 1 file changed, 21 insertions(+), 7 deletions(-) > >>> > >>> diff --git a/net/colo-compare.c b/net/colo-compare.c index > >>> 909dd6c6eb..fcccb4c6f6 100644 > >>> --- a/net/colo-compare.c > >>> +++ b/net/colo-compare.c > >>> @@ -127,6 +127,17 @@ static int compare_chr_send(CompareState *s, > >>> uint32_t vnet_hdr_len, > >>> bool notify_remote_frame); > >>> > >>> +static bool packet_matches_str(const char *str, > >>> + uint8_t *buf, > >> > >> You can use 'uint8_t *buf'. > > > > ?? that seems to be the same as what is written... > > Oops sorry, I copy/pasted and did not noticed I removed the 'const' > word. So I meant: You can use 'const uint8_t *buf' OK. Thanks Zhang Chen > > >> > >>> + uint32_t packet_len) { > >>> + if (packet_len != strlen(str)) { > >>> + return false; > >>> + } > >>> + > >>> + return !memcmp(str, buf, strlen(str)); > >> > >> If you don't want to use a local variable to hold strlen(str), you > >> can reuse packet_len since it is the same value: > >> > >> return !memcmp(str, buf, packet_len); > >> > >> However it makes the review harder, so I'd prefer using a str_len local > >> var. > > > > I'm pretty sure the compiler is going to optimise the > > strlen() away into a compile time constant anyway, so this is somewhat > > unnecessary micro-optimisation I think. > > I was not sure, I'm glad to learn that :) > > Thanks, > > Phil.