On Fri, Aug 09, 2013 at 10:17:47PM -0700, Alex Wang wrote:
> > > + struct netdev *netdev_ =
> > netdev_from_name(change.ifname);
> > > > + if (netdev_ &&
> > > > is_netdev_linux_class(netdev_->netdev_class)) {
> > >
> > >
> > > Want to confirm, at this if statement, is there any case that netdev is
> > not
> > > linux class? (I don't think there is such case right?)
> >
> > Sure it could happen. If you create a dummy netdev with a particular
> > name then that would "hide" a linux netdev with the same name, but the
> > socket would still receive notifications for the linux netdev and we
> > wouldn't want to try to try the dummy netdev as a linux one here.
>
> so in this case, there is the possibility that
> "netdev_" may not be closed, right? Should we move the netdev_close() out?
You are right, there is a leak here. I sent out a fix for review:
http://openvswitch.org/pipermail/dev/2013-August/030585.html
> > > > + } else if (error != EAGAIN) {
> > > > + VLOG_WARN_RL(&rl, "error reading or parsing netlink (%s)",
> > > > + ovs_strerror(error));
> > > > + }
> > > > + ofpbuf_uninit(&buf);
> > > >
> > >
> > > This "ofpbuf_uninit()" is redundant.
> >
> > It is? I do not see any other code that does it, so I think that it is
> > necessary.
>
> I still feel confused. Since the "buf.source" is "OFPBUF_STUB", the
> "ofpbuf_uninit()"
> will have no effect.
The stub can expand into malloc()ed memory if the received message is
larger than the stub buffer. ofpbuf_uninit() will free the buffer in
that case.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
