[ovs-dev] [PATCH] netdev: New function netdev_ref().

Ethan Jackson ethan at nicira.com
Wed May 22 17:42:32 UTC 2013


LGTM as well.  I plan to use this in the CFM patch so please go ahead
and push it.

Ethan

On Wed, May 22, 2013 at 8:50 AM, Ben Pfaff <blp at nicira.com> wrote:
> 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 <blp at 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 <ethan at nicira.com>
>> > Signed-off-by: Ben Pfaff <blp at 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 at openvswitch.org
>> > http://openvswitch.org/mailman/listinfo/dev
>> >
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list