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" >> >>> 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' >> >>> + 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.