On Thu, Jun 30, 2016 at 2:31 PM, William Tu <u9012...@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 <dlu...@gmail.com> wrote:
> >
> >
> > On Tue, Jun 28, 2016 at 10:02 PM, William Tu <u9012...@gmail.com> wrote:
> >>
> >> Variable 'error' has been free in line 1795.
> >>
> >> Signed-off-by: William Tu <u9012...@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@openvswitch.org
> >> http://openvswitch.org/mailman/listinfo/dev
> >
> >
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to