[ovs-dev] [PATCH 5/7] ovn-nbctl: Fix double memory free reported by clang.

[Darrell Ball]

On Thu, Jun 30, 2016 at 2:31 PM, William Tu <u9012063 at gmail.com> wrote:

> Hi Darrell,
>
> Thanks, I think we should print the error in VLOG_WARN and free it .
>
> Regards,
> William
>

Thanks
I only commented on grabbing the error return value and allowing it to be
freed instead
of removing the free() call to satisfy the clang warning.

If you think also adding the error returned is helpful to add to the
existing below warn
log, then probably best to check with the author of the code, but I agree
with you.

>                 VLOG_WARN("router "UUID_FMT" (%s) has invalid prefix: %s",
>                           UUID_ARGS(&lr->header_.uuid), lr->name,
>                           route->ip_prefix);




>
> On Wed, Jun 29, 2016 at 10:08 AM, Darrell Ball <dlu998 at gmail.com> wrote:
> >
> >
> > On Tue, Jun 28, 2016 at 10:02 PM, William Tu <u9012063 at gmail.com> wrote:
> >>
> >> Variable 'error' has been free in line 1795.
> >>
> >> Signed-off-by: William Tu <u9012063 at gmail.com>
> >> ---
> >>  ovn/utilities/ovn-nbctl.c | 1 -
> >>  1 file changed, 1 deletion(-)
> >>
> >> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> >> index 345647a..3228a03 100644
> >> --- a/ovn/utilities/ovn-nbctl.c
> >> +++ b/ovn/utilities/ovn-nbctl.c
> >> @@ -1805,7 +1805,6 @@ nbctl_lr_route_list(struct ctl_context *ctx)
> >>                  VLOG_WARN("router "UUID_FMT" (%s) has invalid prefix:
> >> %s",
> >>                            UUID_ARGS(&lr->header_.uuid), lr->name,
> >>                            route->ip_prefix);
> >> -                free(error);
> >>                  continue;
> >>              }
> >>          }
> >
> >
> >
> > Possibly, the intention was to check for parse error from ipv6 as well as
> > ipv4.
> > The first free is for the ipv4 case.
> > The second free is for the ipv6 case.
> >
> >         error = ip_parse_cidr(route->ip_prefix, &ipv4, &plen);
> >         if (!error) {
> >             ipv4_routes[n_ipv4_routes].plen = plen;
> >             ipv4_routes[n_ipv4_routes].addr = ipv4;
> >             ipv4_routes[n_ipv4_routes].route = route;
> >             n_ipv4_routes++;
> >         } else {
> >             free(error);
> >
> >              struct in6_addr ipv6;
> > -            if (!ipv6_parse_cidr(route->ip_prefix, &ipv6, &plen)) {
> > +           error = ipv6_parse_cidr(route->ip_prefix, &ipv6, &plen);
> > +           if (!error) {
> >                  ipv6_routes[n_ipv6_routes].plen = plen;
> >                  ipv6_routes[n_ipv6_routes].addr = ipv6;
> >                  ipv6_routes[n_ipv6_routes].route = route;
> >                  n_ipv6_routes++;
> >             } else {
> >                 /* Invalid prefix. */
> >                 VLOG_WARN("router "UUID_FMT" (%s) has invalid prefix:
> %s",
> >                           UUID_ARGS(&lr->header_.uuid), lr->name,
> >                           route->ip_prefix);
> >                 free(error);
> >                 continue;
> >             }
> >
> >
> >
> >>
> >> --
> >> 2.5.0
> >>
> >> _______________________________________________
> >> dev mailing list
> >> dev at openvswitch.org
> >> http://openvswitch.org/mailman/listinfo/dev
> >
> >
>