[ovs-dev] [PATCH 2/5] lib/cmap, lib/classifier: Avoid aliasing pointers.
Jarno Rajahalme
jrajahalme at nicira.com
Sat Jul 19 04:05:49 UTC 2014
Older GCC (e.g. 4.1.2) did not like the pointer casts used for
initializing the iteration cursors. This patch changes the code to
avoid the void casts for GCC, and makes the classifier interface more
similar to that of the cmap. These changes make the code work with
GCC 4.1.2 strict aliasing rules.
VMware-BZ: #1287651
Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
lib/classifier.c | 99 +++++++++++++++++++++++++-----------------------------
lib/classifier.h | 55 +++++++++++++++++++++---------
lib/cmap.h | 15 +++++++++
3 files changed, 100 insertions(+), 69 deletions(-)
diff --git a/lib/classifier.c b/lib/classifier.c
index 332e05a..102eb17 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -1210,8 +1210,7 @@ search_subtable(const struct cls_subtable *subtable,
return NULL;
}
-/* Initializes 'cursor' for iterating through rules in 'cls', and returns the
- * first matching cls_rule via '*pnode', or NULL if there are no matches.
+/* Initializes 'cursor' for iterating through rules in 'cls'.
*
* - If 'target' is null, the cursor will visit every rule in 'cls'.
*
@@ -1219,38 +1218,17 @@ 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)
+void cls_cursor_init(struct cls_cursor *cursor, 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;
-
- /* Find first rule. */
- ovs_mutex_lock(&cursor.cls->mutex);
- CMAP_CURSOR_FOR_EACH (subtable, cmap_node, &cursor.subtables,
- &cursor.cls->subtables_map) {
- struct cls_match *rule = search_subtable(subtable, &cursor);
-
- if (rule) {
- cursor.subtable = subtable;
- cls_rule = rule->cls_rule;
- break;
- }
- }
- *pnode = (char *)cls_rule + (ptrdiff_t)offset;
-
- /* Leave locked if requested and have a rule. */
- if (safe || !cls_rule) {
- ovs_mutex_unlock(&cursor.cls->mutex);
+ cursor->safe = safe;
+ cursor->cls = cls;
+ cursor->target = target && !cls_rule_is_catchall(target) ? target : NULL;
+ /* Leave locked if not 'safe' mode */
+ if (!safe) {
+ ovs_mutex_lock(&cls->mutex);
}
- return cursor;
}
static void
@@ -1269,7 +1247,7 @@ struct cls_rule *
cls_cursor_next(struct cls_cursor *cursor, const struct cls_rule *rule_)
OVS_NO_THREAD_SAFETY_ANALYSIS
{
- struct cls_match *rule = CONST_CAST(struct cls_match *, rule_->cls_match);
+ struct cls_match *rule;
const struct cls_subtable *subtable;
struct cls_match *next;
@@ -1278,33 +1256,46 @@ cls_cursor_next(struct cls_cursor *cursor, const struct cls_rule *rule_)
ovs_mutex_lock(&cursor->cls->mutex);
}
- next = next_rule_in_list__(rule);
- if (next->priority < rule->priority) {
- cls_cursor_next_unlock(cursor, next->cls_rule);
- return next->cls_rule;
- }
+ if (!rule_) { /* First rule. */
+ CMAP_CURSOR_FOR_EACH (subtable, cmap_node, &cursor->subtables,
+ &cursor->cls->subtables_map) {
+ rule = search_subtable(subtable, cursor);
- /* 'next' is the head of the list, that is, the rule that is included in
- * the subtable's map. (This is important when the classifier contains
- * rules that differ only in priority.) */
- rule = next;
- 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;
+ if (rule) {
+ cursor->subtable = subtable;
+ cls_cursor_next_unlock(cursor, rule->cls_rule);
+ return rule->cls_rule;
+ }
+ }
+ } else {
+ rule = CONST_CAST(struct cls_match *, rule_->cls_match);
+ next = next_rule_in_list__(rule);
+ if (next->priority < rule->priority) {
+ cls_cursor_next_unlock(cursor, next->cls_rule);
+ return next->cls_rule;
+ }
+
+ /* 'next' is the head of the list, that is, the rule that is included in
+ * the subtable's map. (This is important when the classifier contains
+ * rules that differ only in priority.) */
+ rule = next;
+ 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;
+ }
}
- }
- subtable = cursor->subtable;
- CMAP_CURSOR_FOR_EACH_CONTINUE (subtable, cmap_node, &cursor->subtables) {
- rule = search_subtable(subtable, cursor);
- if (rule) {
- cursor->subtable = subtable;
- cls_cursor_next_unlock(cursor, rule->cls_rule);
- return rule->cls_rule;
+ subtable = cursor->subtable;
+ CMAP_CURSOR_FOR_EACH_CONTINUE (subtable, cmap_node, &cursor->subtables) {
+ rule = search_subtable(subtable, cursor);
+ if (rule) {
+ cursor->subtable = subtable;
+ cls_cursor_next_unlock(cursor, rule->cls_rule);
+ return rule->cls_rule;
+ }
}
}
-
ovs_mutex_unlock(&cursor->cls->mutex);
return NULL;
}
diff --git a/lib/classifier.h b/lib/classifier.h
index 1159610..ada9668 100644
--- a/lib/classifier.h
+++ b/lib/classifier.h
@@ -322,39 +322,64 @@ struct cls_cursor {
/* 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);
+void cls_cursor_init(struct cls_cursor *, const struct classifier *,
+ 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)
+#if __GNUC__
-#define CLS_CURSOR_START_SAFE(RULE, MEMBER, CLS, TARGET) \
- cls_cursor_init(CLS, (TARGET), (void **)&(RULE), \
- OBJECT_CONTAINING(NULL, RULE, MEMBER), true)
+#define CLS_CURSOR_START(RULE, MEMBER, CLS, TARGET, SAFE) \
+ ({ \
+ struct cls_cursor cursor; \
+ \
+ cls_cursor_init(&cursor, (CLS), (TARGET), (SAFE)); \
+ ASSIGN_CONTAINER(RULE, cls_cursor_next(&cursor, NULL), MEMBER); \
+ \
+ cursor; \
+ })
+
+#else
+
+static inline struct cls_cursor cls_cursor_start(const struct classifier *cls,
+ const struct cls_rule *target,
+ bool safe,
+ void **pnode,
+ const void *offset)
+{
+ struct cls_cursor cursor;
+
+ cls_cursor_init(&cursor, cls, target, safe);
+ *pnode = (char *)cls_cursor_next(&cursor, NULL) + (ptrdiff_t)offset;
+
+ return cursor;
+}
+
+#define CLS_CURSOR_START(RULE, MEMBER, CLS, TARGET, SAFE) \
+ cls_cursor_start((CLS), (TARGET), (SAFE), (void **)&(RULE), \
+ OBJECT_CONTAINING(NULL, RULE, MEMBER))
+
+#endif
#define CLS_FOR_EACH(RULE, MEMBER, CLS) \
for (struct cls_cursor cursor__ = CLS_CURSOR_START(RULE, MEMBER, CLS, \
- NULL); \
+ NULL, false); \
RULE != OBJECT_CONTAINING(NULL, RULE, MEMBER); \
ASSIGN_CONTAINER(RULE, cls_cursor_next(&cursor__, &(RULE)->MEMBER), \
MEMBER))
#define CLS_FOR_EACH_TARGET(RULE, MEMBER, CLS, TARGET) \
for (struct cls_cursor cursor__ = CLS_CURSOR_START(RULE, MEMBER, CLS, \
- TARGET); \
+ TARGET, false); \
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); \
+ for (struct cls_cursor cursor__ = CLS_CURSOR_START(RULE, MEMBER, CLS, \
+ NULL, true); \
(RULE != OBJECT_CONTAINING(NULL, RULE, MEMBER) \
? ASSIGN_CONTAINER(NEXT, cls_cursor_next(&cursor__, \
&(RULE)->MEMBER), \
@@ -364,8 +389,8 @@ struct cls_rule *cls_cursor_next(struct cls_cursor *cursor,
/* 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); \
+ for (struct cls_cursor cursor__ = CLS_CURSOR_START(RULE, MEMBER, CLS, \
+ TARGET, true); \
(RULE != OBJECT_CONTAINING(NULL, RULE, MEMBER) \
? ASSIGN_CONTAINER(NEXT, cls_cursor_next(&cursor__, \
&(RULE)->MEMBER), \
diff --git a/lib/cmap.h b/lib/cmap.h
index b7569f5..62de013 100644
--- a/lib/cmap.h
+++ b/lib/cmap.h
@@ -201,6 +201,19 @@ void cmap_cursor_init(struct cmap_cursor *, const struct cmap *);
struct cmap_node *cmap_cursor_next(struct cmap_cursor *,
const struct cmap_node *);
+#if __GNUC__
+
+#define CMAP_CURSOR_START(NODE, MEMBER, CMAP) \
+ ({ \
+ struct cmap_cursor cursor; \
+ \
+ cmap_cursor_init(&cursor, (CMAP)); \
+ ASSIGN_CONTAINER(NODE, cmap_cursor_next(&cursor, NULL), MEMBER); \
+ \
+ cursor; \
+ })
+
+#else
static inline struct cmap_cursor cmap_cursor_start(const struct cmap *cmap,
void **pnode,
@@ -218,6 +231,8 @@ static inline struct cmap_cursor cmap_cursor_start(const struct cmap *cmap,
cmap_cursor_start(CMAP, (void **)&(NODE), \
OBJECT_CONTAINING(NULL, NODE, MEMBER))
+#endif
+
#define CMAP_FOR_EACH(NODE, MEMBER, CMAP) \
for (struct cmap_cursor cursor__ = CMAP_CURSOR_START(NODE, MEMBER, CMAP); \
NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER); \
--
1.7.10.4
More information about the dev
mailing list