[ovs-dev] [ofproto tests 07/29] dpif: Use caller-provided dpif_class in dp_register_provider().

Justin Pettit jpettit at nicira.com
Thu Nov 18 01:18:07 UTC 2010


Looks good.

--Justin


On Nov 16, 2010, at 11:20 AM, Ben Pfaff wrote:

> I don't see a reason here to copy the dpif_class instead of using the
> pointer provided by the caller.  Using the caller's pointer allows the
> caller to compare a dpif's 'class' member against the address of its
> class structure, which seems like a reasonable thing to do.
> ---
> lib/dpif.c |   20 ++++++++++----------
> 1 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/dpif.c b/lib/dpif.c
> index e117bdf..673eac4 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -50,7 +50,7 @@ static const struct dpif_class *base_dpif_classes[] = {
> };
> 
> struct registered_dpif_class {
> -    struct dpif_class dpif_class;
> +    const struct dpif_class *dpif_class;
>     int refcount;
> };
> static struct shash dpif_classes = SHASH_INITIALIZER(&dpif_classes);
> @@ -97,8 +97,8 @@ dp_run(void)
>     struct shash_node *node;
>     SHASH_FOR_EACH(node, &dpif_classes) {
>         const struct registered_dpif_class *registered_class = node->data;
> -        if (registered_class->dpif_class.run) {
> -            registered_class->dpif_class.run();
> +        if (registered_class->dpif_class->run) {
> +            registered_class->dpif_class->run();
>         }
>     }
> }
> @@ -113,8 +113,8 @@ dp_wait(void)
>     struct shash_node *node;
>     SHASH_FOR_EACH(node, &dpif_classes) {
>         const struct registered_dpif_class *registered_class = node->data;
> -        if (registered_class->dpif_class.wait) {
> -            registered_class->dpif_class.wait();
> +        if (registered_class->dpif_class->wait) {
> +            registered_class->dpif_class->wait();
>         }
>     }
> }
> @@ -133,8 +133,7 @@ dp_register_provider(const struct dpif_class *new_class)
>     }
> 
>     registered_class = xmalloc(sizeof *registered_class);
> -    memcpy(&registered_class->dpif_class, new_class,
> -           sizeof registered_class->dpif_class);
> +    registered_class->dpif_class = new_class;
>     registered_class->refcount = 0;
> 
>     shash_add(&dpif_classes, new_class->type, registered_class);
> @@ -182,7 +181,7 @@ dp_enumerate_types(struct svec *types)
> 
>     SHASH_FOR_EACH(node, &dpif_classes) {
>         const struct registered_dpif_class *registered_class = node->data;
> -        svec_add(types, registered_class->dpif_class.type);
> +        svec_add(types, registered_class->dpif_class->type);
>     }
> }
> 
> @@ -208,7 +207,7 @@ dp_enumerate_names(const char *type, struct svec *names)
>         return EAFNOSUPPORT;
>     }
> 
> -    dpif_class = &registered_class->dpif_class;
> +    dpif_class = registered_class->dpif_class;
>     error = dpif_class->enumerate ? dpif_class->enumerate(names) : 0;
> 
>     if (error) {
> @@ -259,7 +258,8 @@ do_open(const char *name, const char *type, bool create, struct dpif **dpifp)
>         goto exit;
>     }
> 
> -    error = registered_class->dpif_class.open(name, type, create, &dpif);
> +    error = registered_class->dpif_class->open(registered_class->dpif_class,
> +                                               name, create, &dpif);
>     if (!error) {
>         registered_class->refcount++;
>     }
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org





More information about the dev mailing list