[ovs-dev] [PATCH 1/2] cmap, classifier: Avoid unsafe aliasing in iterators.

Jarno Rajahalme jrajahalme at nicira.com
Mon Jul 21 22:25:58 UTC 2014


As you noted earlier, CMAP_FOR_EACH and CMAP_FOR_EACH_SAFE now are basically the same thing, the only difference being on that safe does the advance before calling the body. You considered merging them before?

Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>

On Jul 19, 2014, at 10:28 AM, Ben Pfaff <blp at nicira.com> wrote:

> CMAP_FOR_EACH and CLS_FOR_EACH and their variants tried to use void ** as
> a "pointer to any kind of pointer".  That is a violation of the aliasing
> rules in ISO C which technically yields undefined behavior.  With GCC 4.1,
> it causes both warnings and actual misbehavior.  One option would to add
> -fno-strict-aliasing to the compiler flags, but that would only help with
> GCC; who knows whether this can be worked around with other compilers.
> 
> Instead, this commit rewrites the iterators to avoid disallowed pointer
> aliasing.
> 
> VMware-BZ: #1287651
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
> lib/classifier.c        | 44 +++++++++++------------
> lib/classifier.h        | 71 +++++++++++++-------------------------
> lib/cmap.c              | 45 +++++++++++-------------
> lib/cmap.h              | 92 ++++++++++++++++++++-----------------------------
> lib/dpif-netdev.c       |  4 +--
> ofproto/ofproto-dpif.c  |  4 +--
> ofproto/ofproto.c       |  4 +--
> tests/test-classifier.c |  8 ++---
> utilities/ovs-ofctl.c   |  4 +--
> 9 files changed, 115 insertions(+), 161 deletions(-)
> 
> diff --git a/lib/classifier.c b/lib/classifier.c
> index 332e05a..2ff539d 100644
> --- a/lib/classifier.c
> +++ b/lib/classifier.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
> + * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
>  *
>  * Licensed under the Apache License, Version 2.0 (the "License");
>  * you may not use this file except in compliance with the License.
> @@ -476,8 +476,8 @@ classifier_destroy(struct classifier *cls)
>     OVS_EXCLUDED(cls->mutex)
> {
>     if (cls) {
> -        struct cls_partition *partition, *next_partition;
> -        struct cls_subtable *subtable, *next_subtable;
> +        struct cls_partition *partition;
> +        struct cls_subtable *subtable;
>         int i;
> 
>         ovs_mutex_lock(&cls->mutex);
> @@ -485,14 +485,12 @@ classifier_destroy(struct classifier *cls)
>             trie_destroy(&cls->tries[i].root);
>         }
> 
> -        CMAP_FOR_EACH_SAFE (subtable, next_subtable, cmap_node,
> -                            &cls->subtables_map) {
> +        CMAP_FOR_EACH_SAFE (subtable, cmap_node, &cls->subtables_map) {
>             destroy_subtable(cls, subtable);
>         }
>         cmap_destroy(&cls->subtables_map);
> 
> -        CMAP_FOR_EACH_SAFE (partition, next_partition, cmap_node,
> -                            &cls->partitions) {
> +        CMAP_FOR_EACH_SAFE (partition, cmap_node, &cls->partitions) {
>             ovsrcu_postpone(free, partition);
>         }
>         cmap_destroy(&cls->partitions);
> @@ -1219,18 +1217,18 @@ search_subtable(const struct cls_subtable *subtable,
>  *       such that cls_rule_is_loose_match(rule, target) returns true.
>  *
>  * Ignores target->priority. */
> -struct cls_cursor cls_cursor_init(const struct classifier *cls,
> -                                  const struct cls_rule *target,
> -                                  void **pnode, const void *offset, bool safe)
> +struct cls_cursor cls_cursor_start(const struct classifier *cls,
> +                                   const struct cls_rule *target,
> +                                   bool safe)
>     OVS_NO_THREAD_SAFETY_ANALYSIS
> {
>     struct cls_cursor cursor;
>     struct cls_subtable *subtable;
> -    struct cls_rule *cls_rule = NULL;
> 
>     cursor.safe = safe;
>     cursor.cls = cls;
>     cursor.target = target && !cls_rule_is_catchall(target) ? target : NULL;
> +    cursor.rule = NULL;
> 
>     /* Find first rule. */
>     ovs_mutex_lock(&cursor.cls->mutex);
> @@ -1240,14 +1238,13 @@ struct cls_cursor cls_cursor_init(const struct classifier *cls,
> 
>         if (rule) {
>             cursor.subtable = subtable;
> -            cls_rule = rule->cls_rule;
> +            cursor.rule = rule->cls_rule;
>             break;
>         }
>     }
> -    *pnode = (char *)cls_rule + (ptrdiff_t)offset;
> 
>     /* Leave locked if requested and have a rule. */
> -    if (safe || !cls_rule) {
> +    if (safe || !cursor.rule) {
>         ovs_mutex_unlock(&cursor.cls->mutex);
>     }
>     return cursor;
> @@ -1258,18 +1255,19 @@ cls_cursor_next_unlock(struct cls_cursor *cursor, struct cls_rule *rule)
>     OVS_NO_THREAD_SAFETY_ANALYSIS
> {
>     /* Release the mutex if no rule, or 'safe' mode. */
> +    cursor->rule = rule;
>     if (!rule || cursor->safe) {
>         ovs_mutex_unlock(&cursor->cls->mutex);
>     }
> }
> 
> -/* Returns the next matching cls_rule in 'cursor''s iteration, or a null
> - * pointer if there are no more matches. */
> -struct cls_rule *
> -cls_cursor_next(struct cls_cursor *cursor, const struct cls_rule *rule_)
> +/* Sets 'cursor->rule' to the next matching cls_rule in 'cursor''s iteration,
> + * or to null if all matching rules have been visited. */
> +void
> +cls_cursor_advance(struct cls_cursor *cursor)
>     OVS_NO_THREAD_SAFETY_ANALYSIS
> {
> -    struct cls_match *rule = CONST_CAST(struct cls_match *, rule_->cls_match);
> +    struct cls_match *rule = cursor->rule->cls_match;
>     const struct cls_subtable *subtable;
>     struct cls_match *next;
> 
> @@ -1281,7 +1279,7 @@ cls_cursor_next(struct cls_cursor *cursor, const struct cls_rule *rule_)
>     next = next_rule_in_list__(rule);
>     if (next->priority < rule->priority) {
>         cls_cursor_next_unlock(cursor, next->cls_rule);
> -        return next->cls_rule;
> +        return;
>     }
> 
>     /* 'next' is the head of the list, that is, the rule that is included in
> @@ -1291,7 +1289,7 @@ cls_cursor_next(struct cls_cursor *cursor, const struct cls_rule *rule_)
>     CMAP_CURSOR_FOR_EACH_CONTINUE (rule, cmap_node, &cursor->rules) {
>         if (rule_matches(rule, cursor->target)) {
>             cls_cursor_next_unlock(cursor, rule->cls_rule);
> -            return rule->cls_rule;
> +            return;
>         }
>     }
> 
> @@ -1301,12 +1299,12 @@ cls_cursor_next(struct cls_cursor *cursor, const struct cls_rule *rule_)
>         if (rule) {
>             cursor->subtable = subtable;
>             cls_cursor_next_unlock(cursor, rule->cls_rule);
> -            return rule->cls_rule;
> +            return;
>         }
>     }
> 
>     ovs_mutex_unlock(&cursor->cls->mutex);
> -    return NULL;
> +    cursor->rule = NULL;
> }
> 
> static struct cls_subtable *
> diff --git a/lib/classifier.h b/lib/classifier.h
> index 1159610..4203eb8 100644
> --- a/lib/classifier.h
> +++ b/lib/classifier.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
> + * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
>  *
>  * Licensed under the Apache License, Version 2.0 (the "License");
>  * you may not use this file except in compliance with the License.
> @@ -316,63 +316,40 @@ struct cls_cursor {
>     const struct cls_rule *target;
>     struct cmap_cursor subtables;
>     struct cmap_cursor rules;
> +    struct cls_rule *rule;
>     bool safe;
> };
> 
> /* Iteration requires mutual exclusion of writers.  We do this by taking
>  * a mutex for the duration of the iteration, except for the
>  * 'SAFE' variant, where we release the mutex for the body of the loop. */
> -struct cls_cursor cls_cursor_init(const struct classifier *cls,
> -                                  const struct cls_rule *target,
> -                                  void **pnode, const void *offset, bool safe);
> +struct cls_cursor cls_cursor_start(const struct classifier *cls,
> +                                   const struct cls_rule *target,
> +                                   bool safe);
> 
> -struct cls_rule *cls_cursor_next(struct cls_cursor *cursor,
> -                                 const struct cls_rule *);
> -
> -#define CLS_CURSOR_START(RULE, MEMBER, CLS, TARGET)                     \
> -    cls_cursor_init(CLS, (TARGET), (void **)&(RULE),                    \
> -                    OBJECT_CONTAINING(NULL, RULE, MEMBER), false)
> -
> -#define CLS_CURSOR_START_SAFE(RULE, MEMBER, CLS, TARGET)                \
> -    cls_cursor_init(CLS, (TARGET), (void **)&(RULE),                    \
> -                    OBJECT_CONTAINING(NULL, RULE, MEMBER), true)
> -
> -#define CLS_FOR_EACH(RULE, MEMBER, CLS)                                 \
> -    for (struct cls_cursor cursor__ = CLS_CURSOR_START(RULE, MEMBER, CLS, \
> -                                                       NULL);           \
> -         RULE != OBJECT_CONTAINING(NULL, RULE, MEMBER);                 \
> -         ASSIGN_CONTAINER(RULE, cls_cursor_next(&cursor__, &(RULE)->MEMBER), \
> -                          MEMBER))
> +void cls_cursor_advance(struct cls_cursor *);
> 
> +#define CLS_FOR_EACH(RULE, MEMBER, CLS) \
> +    CLS_FOR_EACH_TARGET(RULE, MEMBER, CLS, NULL)
> #define CLS_FOR_EACH_TARGET(RULE, MEMBER, CLS, TARGET)                  \
> -    for (struct cls_cursor cursor__ = CLS_CURSOR_START(RULE, MEMBER, CLS, \
> -                                                       TARGET);         \
> -         RULE != OBJECT_CONTAINING(NULL, RULE, MEMBER);                 \
> -         ASSIGN_CONTAINER(RULE, cls_cursor_next(&cursor__, &(RULE)->MEMBER), \
> -                          MEMBER))
> -
> -/* This form allows classifier_remove() to be called within the loop. */
> -#define CLS_FOR_EACH_SAFE(RULE, NEXT, MEMBER, CLS)                      \
> -    for (struct cls_cursor cursor__ = CLS_CURSOR_START_SAFE(RULE, MEMBER, \
> -                                                            CLS, NULL); \
> -         (RULE != OBJECT_CONTAINING(NULL, RULE, MEMBER)                 \
> -          ? ASSIGN_CONTAINER(NEXT, cls_cursor_next(&cursor__,           \
> -                                                   &(RULE)->MEMBER),    \
> -                             MEMBER), true                              \
> +    for (struct cls_cursor cursor__ = cls_cursor_start(CLS, TARGET, false); \
> +         (cursor__.rule                                                 \
> +          ? (ASSIGN_CONTAINER(RULE, cursor__.rule, MEMBER),            \
> +             true)                                                      \
>           : false);                                                     \
> -         (RULE) = (NEXT))
> -
> -/* This form allows classifier_remove() to be called within the loop. */
> -#define CLS_FOR_EACH_TARGET_SAFE(RULE, NEXT, MEMBER, CLS, TARGET)       \
> -    for (struct cls_cursor cursor__ = CLS_CURSOR_START_SAFE(RULE, MEMBER, \
> -                                                            CLS, TARGET); \
> -         (RULE != OBJECT_CONTAINING(NULL, RULE, MEMBER)                 \
> -          ? ASSIGN_CONTAINER(NEXT, cls_cursor_next(&cursor__,           \
> -                                                   &(RULE)->MEMBER),    \
> -                             MEMBER), true                              \
> +         cls_cursor_advance(&cursor__))
> +
> +/* These forms allows classifier_remove() to be called within the loop. */
> +#define CLS_FOR_EACH_SAFE(RULE, MEMBER, CLS) \
> +    CLS_FOR_EACH_TARGET_SAFE(RULE, MEMBER, CLS, NULL)
> +#define CLS_FOR_EACH_TARGET_SAFE(RULE, MEMBER, CLS, TARGET)             \
> +    for (struct cls_cursor cursor__ = cls_cursor_start(CLS, TARGET, true); \
> +         (cursor__.rule                                                 \
> +          ? (ASSIGN_CONTAINER(RULE, cursor__.rule, MEMBER),            \
> +             cls_cursor_advance(&cursor__),                             \
> +             true)                                                      \
>           : false);                                                     \
> -         (RULE) = (NEXT))
> -
> +        )                                                               \
> 
> #ifdef __cplusplus
> }
> diff --git a/lib/cmap.c b/lib/cmap.c
> index 5d6dbcf..ba744cc 100644
> --- a/lib/cmap.c
> +++ b/lib/cmap.c
> @@ -784,31 +784,29 @@ cmap_rehash(struct cmap *cmap, uint32_t mask)
>     return new;
> }
> 
> -/* Initializes 'cursor' for iterating through 'cmap'.
> - *
> - * Use via CMAP_FOR_EACH. */
> -void
> -cmap_cursor_init(struct cmap_cursor *cursor, const struct cmap *cmap)
> +struct cmap_cursor
> +cmap_cursor_start(const struct cmap *cmap)
> {
> -    cursor->impl = cmap_get_impl(cmap);
> -    cursor->bucket_idx = 0;
> -    cursor->entry_idx = 0;
> +    struct cmap_cursor cursor;
> +
> +    cursor.impl = cmap_get_impl(cmap);
> +    cursor.bucket_idx = 0;
> +    cursor.entry_idx = 0;
> +    cursor.node = NULL;
> +    cmap_cursor_advance(&cursor);
> +
> +    return cursor;
> }
> 
> -/* Returns the next node for 'cursor' to visit, following 'node', or NULL if
> - * the last node has been visited.
> - *
> - * Use via CMAP_FOR_EACH. */
> -struct cmap_node *
> -cmap_cursor_next(struct cmap_cursor *cursor, const struct cmap_node *node)
> +void
> +cmap_cursor_advance(struct cmap_cursor *cursor)
> {
>     const struct cmap_impl *impl = cursor->impl;
> 
> -    if (node) {
> -        struct cmap_node *next = cmap_node_next(node);
> -
> -        if (next) {
> -            return next;
> +    if (cursor->node) {
> +        cursor->node = cmap_node_next(cursor->node);
> +        if (cursor->node) {
> +            return;
>         }
>     }
> 
> @@ -816,18 +814,15 @@ cmap_cursor_next(struct cmap_cursor *cursor, const struct cmap_node *node)
>         const struct cmap_bucket *b = &impl->buckets[cursor->bucket_idx];
> 
>         while (cursor->entry_idx < CMAP_K) {
> -            struct cmap_node *node = cmap_node_next(&b->nodes[cursor->entry_idx++]);
> -
> -            if (node) {
> -                return node;
> +            cursor->node = cmap_node_next(&b->nodes[cursor->entry_idx++]);
> +            if (cursor->node) {
> +                return;
>             }
>         }
> 
>         cursor->bucket_idx++;
>         cursor->entry_idx = 0;
>     }
> -
> -    return NULL;
> }
> 
> /* Returns the next node in 'cmap' in hash order, or NULL if no nodes remain in
> diff --git a/lib/cmap.h b/lib/cmap.h
> index b7569f5..87a1d53 100644
> --- a/lib/cmap.h
> +++ b/lib/cmap.h
> @@ -168,70 +168,54 @@ struct cmap_node *cmap_find_protected(const struct cmap *, uint32_t hash);
>  * has already expired.
>  */
> 
> -#define CMAP_CURSOR_FOR_EACH(NODE, MEMBER, CURSOR, CMAP)                \
> -    for ((cmap_cursor_init(CURSOR, CMAP),                               \
> -          ASSIGN_CONTAINER(NODE, cmap_cursor_next(CURSOR, NULL), MEMBER)); \
> -         NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER);                 \
> -         ASSIGN_CONTAINER(NODE, cmap_cursor_next(CURSOR, &(NODE)->MEMBER), \
> -                          MEMBER))
> -
> -#define CMAP_CURSOR_FOR_EACH_SAFE(NODE, NEXT, MEMBER, CURSOR, CMAP)     \
> -    for ((cmap_cursor_init(CURSOR, CMAP),                               \
> -          ASSIGN_CONTAINER(NODE, cmap_cursor_next(CURSOR, NULL), MEMBER)); \
> -         (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)                  \
> -          ? ASSIGN_CONTAINER(NEXT, cmap_cursor_next(CURSOR, &(NODE)->MEMBER), \
> -                             MEMBER), true                              \
> -          : false);                                                     \
> -         (NODE) = (NEXT))
> -
> -#define CMAP_CURSOR_FOR_EACH_CONTINUE(NODE, MEMBER, CURSOR)             \
> -    for (ASSIGN_CONTAINER(NODE, cmap_cursor_next(CURSOR, &(NODE)->MEMBER), \
> -                          MEMBER);                                      \
> -         NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER);                 \
> -         ASSIGN_CONTAINER(NODE, cmap_cursor_next(CURSOR, &(NODE)->MEMBER), \
> -                          MEMBER))
> +#define CMAP_CURSOR_FOR_EACH(NODE, MEMBER, CURSOR, CMAP)            \
> +    for (*(CURSOR) = cmap_cursor_start(CMAP);                       \
> +         ((CURSOR)->node                                            \
> +          ? (ASSIGN_CONTAINER(NODE, (CURSOR)->node, MEMBER), true)  \
> +          : false);                                                 \
> +         cmap_cursor_advance(CURSOR))
> +
> +#define CMAP_CURSOR_FOR_EACH_SAFE(NODE, MEMBER, CURSOR, CMAP)   \
> +    for (*(CURSOR) = cmap_cursor_start(CMAP);                   \
> +         ((CURSOR)->node                                        \
> +          ? (ASSIGN_CONTAINER(NODE, (CURSOR)->node, MEMBER),    \
> +             cmap_cursor_advance(CURSOR),                       \
> +             true)                                              \
> +          : false);                                             \
> +        )
> +
> +#define CMAP_CURSOR_FOR_EACH_CONTINUE(NODE, MEMBER, CURSOR)         \
> +    for (cmap_cursor_advance(CURSOR);                               \
> +         ((CURSOR)->node                                            \
> +          ? (ASSIGN_CONTAINER(NODE, (CURSOR)->node, MEMBER), true)  \
> +          : false);                                                 \
> +         cmap_cursor_advance(CURSOR))
> 
> struct cmap_cursor {
>     const struct cmap_impl *impl;
>     uint32_t bucket_idx;
>     int entry_idx;
> +    struct cmap_node *node;
> };
> 
> -void cmap_cursor_init(struct cmap_cursor *, const struct cmap *);
> -struct cmap_node *cmap_cursor_next(struct cmap_cursor *,
> -                                   const struct cmap_node *);
> -
> -
> -static inline struct cmap_cursor cmap_cursor_start(const struct cmap *cmap,
> -                                                   void **pnode,
> -                                                   const void *offset)
> -{
> -    struct cmap_cursor cursor;
> -
> -    cmap_cursor_init(&cursor, cmap);
> -    *pnode = (char *)cmap_cursor_next(&cursor, NULL) + (ptrdiff_t)offset;
> -
> -    return cursor;
> -}
> -
> -#define CMAP_CURSOR_START(NODE, MEMBER, CMAP)                   \
> -    cmap_cursor_start(CMAP, (void **)&(NODE),                   \
> -                      OBJECT_CONTAINING(NULL, NODE, MEMBER))
> +struct cmap_cursor cmap_cursor_start(const struct cmap *);
> +void cmap_cursor_advance(struct cmap_cursor *);
> 
> #define CMAP_FOR_EACH(NODE, MEMBER, CMAP)                               \
> -    for (struct cmap_cursor cursor__ = CMAP_CURSOR_START(NODE, MEMBER, CMAP); \
> -         NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER);                 \
> -         ASSIGN_CONTAINER(NODE, cmap_cursor_next(&cursor__, &(NODE)->MEMBER), \
> -                          MEMBER))
> -
> -#define CMAP_FOR_EACH_SAFE(NODE, NEXT, MEMBER, CMAP)                    \
> -    for (struct cmap_cursor cursor__ = CMAP_CURSOR_START(NODE, MEMBER, CMAP); \
> -         (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)                 \
> -          ? ASSIGN_CONTAINER(NEXT,                                      \
> -                             cmap_cursor_next(&cursor__, &(NODE)->MEMBER), \
> -                             MEMBER), true                              \
> +    for (struct cmap_cursor cursor__ = cmap_cursor_start(CMAP); \
> +         (cursor__.node                                                 \
> +          ? (ASSIGN_CONTAINER(NODE, cursor__.node, MEMBER), true)       \
> +          : false);                                                     \
> +         cmap_cursor_advance(&cursor__))
> +
> +#define CMAP_FOR_EACH_SAFE(NODE, MEMBER, CMAP)                          \
> +    for (struct cmap_cursor cursor__ = cmap_cursor_start(CMAP); \
> +         (cursor__.node                                                 \
> +          ? (ASSIGN_CONTAINER(NODE, cursor__.node, MEMBER),             \
> +             cmap_cursor_advance(&cursor__),                            \
> +             true)                                                      \
>           : false);                                                     \
> -         (NODE) = (NEXT))
> +        )
> 
> static inline struct cmap_node *cmap_first(const struct cmap *);
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 90445d1..9fbe4da 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -963,10 +963,10 @@ dp_netdev_remove_flow(struct dp_netdev *dp, struct dp_netdev_flow *flow)
> static void
> dp_netdev_flow_flush(struct dp_netdev *dp)
> {
> -    struct dp_netdev_flow *netdev_flow, *next;
> +    struct dp_netdev_flow *netdev_flow;
> 
>     ovs_mutex_lock(&dp->flow_mutex);
> -    CMAP_FOR_EACH_SAFE (netdev_flow, next, node, &dp->flow_table) {
> +    CMAP_FOR_EACH_SAFE (netdev_flow, node, &dp->flow_table) {
>         dp_netdev_remove_flow(dp, netdev_flow);
>     }
>     ovs_mutex_unlock(&dp->flow_mutex);
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 10b0cd4..0f19026 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -1280,8 +1280,8 @@ static void
> destruct(struct ofproto *ofproto_)
> {
>     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
> -    struct rule_dpif *rule, *next_rule;
>     struct ofproto_packet_in *pin, *next_pin;
> +    struct rule_dpif *rule;
>     struct oftable *table;
>     struct list pins;
> 
> @@ -1297,7 +1297,7 @@ destruct(struct ofproto *ofproto_)
>     hmap_remove(&all_ofproto_dpifs, &ofproto->all_ofproto_dpifs_node);
> 
>     OFPROTO_FOR_EACH_TABLE (table, &ofproto->up) {
> -        CLS_FOR_EACH_SAFE (rule, next_rule, up.cr, &table->cls) {
> +        CLS_FOR_EACH_SAFE (rule, up.cr, &table->cls) {
>             ofproto_rule_delete(&ofproto->up, &rule->up);
>         }
>     }
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index c9a8000..fca7d09 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -1283,13 +1283,13 @@ ofproto_flush__(struct ofproto *ofproto)
> 
>     ovs_mutex_lock(&ofproto_mutex);
>     OFPROTO_FOR_EACH_TABLE (table, ofproto) {
> -        struct rule *rule, *next_rule;
> +        struct rule *rule;
> 
>         if (table->flags & OFTABLE_HIDDEN) {
>             continue;
>         }
> 
> -        CLS_FOR_EACH_SAFE (rule, next_rule, cr, &table->cls) {
> +        CLS_FOR_EACH_SAFE (rule, cr, &table->cls) {
>             ofproto_rule_delete__(rule, OFPRR_DELETE);
>         }
>     }
> diff --git a/tests/test-classifier.c b/tests/test-classifier.c
> index 253e9b3..0dfa910 100644
> --- a/tests/test-classifier.c
> +++ b/tests/test-classifier.c
> @@ -452,9 +452,9 @@ compare_classifiers(struct classifier *cls, struct tcls *tcls)
> static void
> destroy_classifier(struct classifier *cls)
> {
> -    struct test_rule *rule, *next_rule;
> +    struct test_rule *rule;
> 
> -    CLS_FOR_EACH_SAFE (rule, next_rule, cls_rule, cls) {
> +    CLS_FOR_EACH_SAFE (rule, cls_rule, cls) {
>         classifier_remove(cls, &rule->cls_rule);
>         free_rule(rule);
>     }
> @@ -1069,12 +1069,12 @@ test_many_rules_in_n_tables(int n_tables)
>         }
> 
>         while (!classifier_is_empty(&cls)) {
> -            struct test_rule *rule, *next_rule;
>             struct test_rule *target;
> +            struct test_rule *rule;
> 
>             target = clone_rule(tcls.rules[random_range(tcls.n_rules)]);
> 
> -            CLS_FOR_EACH_TARGET_SAFE (rule, next_rule, cls_rule, &cls,
> +            CLS_FOR_EACH_TARGET_SAFE (rule, cls_rule, &cls,
>                                       &target->cls_rule) {
>                 classifier_remove(&cls, &rule->cls_rule);
>                 free_rule(rule);
> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> index de68f47..d3aad0f 100644
> --- a/utilities/ovs-ofctl.c
> +++ b/utilities/ovs-ofctl.c
> @@ -2316,9 +2316,9 @@ fte_free(struct fte *fte)
> static void
> fte_free_all(struct classifier *cls)
> {
> -    struct fte *fte, *next;
> +    struct fte *fte;
> 
> -    CLS_FOR_EACH_SAFE (fte, next, rule, cls) {
> +    CLS_FOR_EACH_SAFE (fte, rule, cls) {
>         classifier_remove(cls, &fte->rule);
>         fte_free(fte);
>     }
> -- 
> 1.9.1
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev




More information about the dev mailing list