[ovs-dev] [PATCH v4 4/5] classifier: Make insert and replace take a const rule, too.

[Jarno Rajahalme]

On Nov 14, 2014, at 1:42 PM, Ben Pfaff <blp at nicira.com> wrote:

> On Thu, Nov 13, 2014 at 11:56:16AM -0800, Jarno Rajahalme wrote:
>> classifier_remove() was recently changed to take a const struct
>> cls_rule *.  Make the corresponding change to classifier_replace() and
>> classifier_insert().  This simplifies existing calling sites in
>> ofproto.
>> 
>> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
> 
> The construct &CONST_CAST(...)->member looks odd to me, how about this incremental:
> 

I thought it was clever … but folded your incremental in anyway :-)

Thanks for the review!

Merging soon,

  Jarno

> diff --git a/lib/classifier.c b/lib/classifier.c
> index 4535f19..8fce7a9 100644
> --- a/lib/classifier.c
> +++ b/lib/classifier.c
> @@ -602,7 +602,7 @@ classifier_replace(struct classifier *cls, const struct cls_rule *rule)
>             /* No change in subtable's max priority or max count. */
> 
>             /* Make rule visible to iterators. */
> -            rculist_replace(&CONST_CAST(struct cls_rule *, rule)->node,
> +            rculist_replace(CONST_CAST(struct rculist *, &rule->node),
>                             &old->node);
> 
>             /* Return displaced rule.  Caller is responsible for keeping it
> @@ -613,7 +613,7 @@ classifier_replace(struct classifier *cls, const struct cls_rule *rule)
> 
>     /* Make rule visible to iterators. */
>     rculist_push_back(&subtable->rules_list,
> -                      &CONST_CAST(struct cls_rule *, rule)->node);
> +                      CONST_CAST(struct rculist *, &rule->node));
> 
>     /* Rule was added, not replaced.  Update 'subtable's 'max_priority' and
>      * 'max_count', if necessary.
> @@ -680,7 +680,7 @@ classifier_remove(struct classifier *cls, const struct cls_rule *rule)
>     CONST_CAST(struct cls_rule *, rule)->cls_match = NULL;
> 
>     /* Remove 'rule' from the subtable's rules list. */
> -    rculist_remove(&CONST_CAST(struct cls_rule *, rule)->node);
> +    rculist_remove(CONST_CAST(struct rculist *, &rule->node));
> 
>     INIT_CONTAINER(prev, rculist_back_protected(&cls_match->list), list);
>     INIT_CONTAINER(next, rculist_next(&cls_match->list), list);
> 
> Acked-by: Ben Pfaff <blp at nicira.com>