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 <b...@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 <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
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
