You are right. There is no problem with patch.

Acked-by: Andy Zhou <[email protected]>



On Wed, Aug 7, 2013 at 10:09 AM, Ben Pfaff <[email protected]> wrote:

> On Mon, Aug 05, 2013 at 01:00:59PM -0700, Andy Zhou wrote:
> > On Thu, Aug 1, 2013 at 2:29 PM, Ben Pfaff <[email protected]> wrote:
> >
> > > This API change is necessary for thread safety, to be added in an
> upcoming
> > > commit.  Otherwise, the client would not be able to actually use any of
> > > the returned netdevs because they could already have been destroyed.
> > >
> > > Signed-off-by: Ben Pfaff <[email protected]>
> > > ---
> > >  lib/netdev-bsd.c   |   32 ++++++++++----------------------
> > >  lib/netdev-linux.c |    4 ++++
> > >  lib/netdev.c       |    5 +++--
> > >  3 files changed, 17 insertions(+), 24 deletions(-)
> > >
> > > diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
> > > index 6ff6b3e..d6a8631 100644
> > > --- a/lib/netdev-bsd.c
> > > +++ b/lib/netdev-bsd.c
> > > @@ -255,6 +255,7 @@ netdev_bsd_cache_cb(const struct rtbsd_change
> *change,
> > >                  dev->cache_valid = 0;
> > >                  netdev_bsd_changed(dev);
> > >              }
> > > +            netdev_close(base_dev);
> > >          }
> > >      } else {
> > >          /*
> > > @@ -271,6 +272,7 @@ netdev_bsd_cache_cb(const struct rtbsd_change
> *change,
> > >              dev = netdev_bsd_cast(netdev);
> > >              dev->cache_valid = 0;
> > >              netdev_bsd_changed(dev);
> > > +            netdev_close(dev);
> > >          }
> > >          shash_destroy(&device_shash);
> > >      }
> > > @@ -1200,9 +1202,10 @@ netdev_bsd_get_in6(const struct netdev *netdev_,
> > > struct in6_addr *in6)
> > >  }
> > >
> > >  #if defined(__NetBSD__)
> > > -static struct netdev *
> > > -find_netdev_by_kernel_name(const char *kernel_name)
> > > +static char *
> > > +netdev_bsd_kernel_name_to_ovs_name(const char *kernel_name)
> > >  {
> > > +    char *ovs_name = NULL;
> > >      struct shash device_shash;
> > >      struct shash_node *node;
> > >
> > > @@ -1213,24 +1216,14 @@ find_netdev_by_kernel_name(const char
> *kernel_name)
> > >          struct netdev_bsd * const dev = netdev_bsd_cast(netdev);
> > >
> > >          if (!strcmp(dev->kernel_name, kernel_name)) {
> > > -            shash_destroy(&device_shash);
> > > -            return &dev->up;
> > > +            free(ovs_name);
> > > +            ovs_name = xstrdup(netdev_get_name(&dev->up));
> > >
> > Would netdev_close to be called for &dev->up ?
>
> netdev == &dev->up here.  I guess the code could be more consistent
> about using one or the other.
>
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to