Re: [Qemu-devel] [PATCH V3] net/colo-compare.c: Fix memory leak and code style issue.

[Peter Maydell <peter.maydell@linaro.org>]

On Thu, 18 Jul 2019 at 11:28, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> On 7/18/19 11:22 AM, Zhang Chen wrote:
> > From: Zhang Chen <chen.zhang@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.zhang@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...

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

thanks
-- PMM