Thanks for taking a look.  I won't push this unless Ethan says that he
thinks it is useful for his recent cfm patch.

I have two perspectives on your question.  The first one is that this
situation is analogous to strchr(), which takes a const char * and
returns a char *.  Its parameter is const because it does not modify its
argument, and its return value is non-const because that is more useful
to the caller, which in practice often passes in a non-const pointer and
wishes to use the result in a non-const way.  (In C++, there are two
versions of strchr(): one that takes and returns a const char *, one
that takes and returns a char *.  This is strictly better type-wise, but
C does not allow this.)

The second perspective is that this situation is analogous to a function
like strdup(), which takes a const char * (that it does not modify) and
returns a char * (that is newly allocated).  netdev_ref() is somewhat
like this, in that the caller can regard its return value as a new copy
of the netdev.  Even though the return value points to the same
location, it is still somewhat separate due to the additional reference
that it "owns".

On Tue, May 21, 2013 at 06:37:17PM -0700, Alex Wang wrote:
> Looks good to me. Want to ask what is the tradeoff between using "const
> struct netdev" and "struct netdev" as input argument.
> 
> Thanks,
> 
> On Tue, May 21, 2013 at 3:42 PM, Ben Pfaff <b...@nicira.com> wrote:
> 
> > I suspect that this makes it easier to make sure that a netdev stays open
> > as long as needed in some cases where a module needs access to a netdev
> > opened by some higher-level module.
> >
> > CC: Ethan Jackson <et...@nicira.com>
> > Signed-off-by: Ben Pfaff <b...@nicira.com>
> > ---
> >  lib/netdev.c |   11 +++++++++++
> >  lib/netdev.h |    1 +
> >  2 files changed, 12 insertions(+), 0 deletions(-)
> >
> > diff --git a/lib/netdev.c b/lib/netdev.c
> > index 5c2e9f5..5aae01c 100644
> > --- a/lib/netdev.c
> > +++ b/lib/netdev.c
> > @@ -277,6 +277,17 @@ netdev_open(const char *name, const char *type,
> > struct netdev **netdevp)
> >      return 0;
> >  }
> >
> > +/* Returns a reference to 'netdev_' for the caller to own. */
> > +struct netdev *
> > +netdev_ref(const struct netdev *netdev_)
> > +{
> > +    struct netdev *netdev = CONST_CAST(struct netdev *, netdev_);
> > +
> > +    ovs_assert(netdev->ref_cnt > 0);
> > +    netdev->ref_cnt++;
> > +    return netdev;
> > +}
> > +
> >  /* Reconfigures the device 'netdev' with 'args'.  'args' may be empty
> >   * or NULL if none are needed. */
> >  int
> > diff --git a/lib/netdev.h b/lib/netdev.h
> > index c7f3c1d..b1cc319 100644
> > --- a/lib/netdev.h
> > +++ b/lib/netdev.h
> > @@ -110,6 +110,7 @@ bool netdev_is_reserved_name(const char *name);
> >
> >  /* Open and close. */
> >  int netdev_open(const char *name, const char *type, struct netdev **);
> > +struct netdev *netdev_ref(const struct netdev *);
> >  void netdev_close(struct netdev *);
> >
> >  void netdev_parse_name(const char *netdev_name, char **name, char **type);
> > --
> > 1.7.2.5
> >
> > _______________________________________________
> > 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