[ovs-dev] [PATCH 3/3] Avoid uninitialized variable warnings with OBJECT_OFFSETOF() in MSVC.
Eitan Eliahu
eliahue at vmware.com
Thu Sep 11 08:57:37 UTC 2014
You have a point. Probably, not all developers would like to have MSVC pragams inside the code. But, still, I don't like the idea of adding instructions to suppress warnings.
Eitan
-----Original Message-----
From: Alin Serdean [mailto:aserdean at cloudbasesolutions.com]
Sent: Thursday, September 11, 2014 1:32 AM
To: Eitan Eliahu; Gurucharan Shetty; dev at openvswitch.org
Cc: Gurucharan Shetty
Subject: RE: [ovs-dev] [PATCH 3/3] Avoid uninitialized variable warnings with OBJECT_OFFSETOF() in MSVC.
Hi Eitan,
For a limited scope just to suppress the warnings the code will get very cumbersome.
For some variable initialization I do not see what is so inefficient.
Alin.
-----Mesaj original-----
De la: Eitan Eliahu [mailto:eliahue at vmware.com]
Trimis: Thursday, September 11, 2014 1:19 AM
Către: Alin Serdean; Gurucharan Shetty; dev at openvswitch.org
Cc: Gurucharan Shetty
Subiect: RE: [ovs-dev] [PATCH 3/3] Avoid uninitialized variable warnings with OBJECT_OFFSETOF() in MSVC.
Hi Alin,
I would prefer to disable the warning in a very limited scope rather than disabling them across all source code.
Adding instructions just for suppressing warning would be inefficient.
Eitan
-----Original Message-----
From: Alin Serdean [mailto:aserdean at cloudbasesolutions.com]
Sent: Thursday, September 11, 2014 1:07 AM
To: Eitan Eliahu; Gurucharan Shetty; dev at openvswitch.org
Cc: Gurucharan Shetty
Subject: RE: [ovs-dev] [PATCH 3/3] Avoid uninitialized variable warnings with OBJECT_OFFSETOF() in MSVC.
Hi Eitan,
I think putting in pragmas to disable warnings is a bit over our hand.
Either we just disable them in a common include header or we leave them and actually fix the warnings.
Alin.
-----Mesaj original-----
De la: dev [mailto:dev-bounces at openvswitch.org] În numele Eitan Eliahu
Trimis: Wednesday, September 10, 2014 10:21 PM
Către: Gurucharan Shetty; dev at openvswitch.org
Cc: Gurucharan Shetty
Subiect: Re: [ovs-dev] [PATCH 3/3] Avoid uninitialized variable warnings with OBJECT_OFFSETOF() in MSVC.
Hi Guru,
You can just suppress the warning by wrapping the code in a pragama warning block:
#pragma warning( push )
#pragma warning( disable : 4707 )
// Some code
#pragma warning( pop )
You don't want to initialize to NULL just for suppressing the warning.
Thanks,
Eitan
-----Original Message-----
From: dev [mailto:dev-bounces at openvswitch.org] On Behalf Of Gurucharan Shetty
Sent: Wednesday, September 10, 2014 12:56 PM
To: dev at openvswitch.org
Cc: Gurucharan Shetty
Subject: [ovs-dev] [PATCH 3/3] Avoid uninitialized variable warnings with OBJECT_OFFSETOF() in MSVC.
Implementation of OBJECT_OFFSETOF() for non-GNUC compilers like MSVC causes "uninitialized variable" warnings. Since OBJECT_OFFSETOF() is indirectly used through all the *_FOR_EACH() (through ASSIGN_CONTAINER() and OBJECT_CONTAINING()) macros, the OVS build on Windows gets littered with "uninitialized variable" warnings.
This patch attempts to workaround the problem.
Signed-off-by: Gurucharan Shetty <gshetty at nicira.com>
---
lib/classifier.c | 5 +++--
lib/classifier.h | 4 ++--
lib/cmap.h | 6 +++---
lib/heap.h | 2 +-
lib/hindex.h | 10 +++++-----
lib/hmap.h | 10 +++++-----
lib/list.h | 10 +++++-----
lib/util.h | 7 +++++++
8 files changed, 31 insertions(+), 23 deletions(-)
diff --git a/lib/classifier.c b/lib/classifier.c index ae03251..ee737a7 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -1561,7 +1561,7 @@ find_match_wc(const struct cls_subtable *subtable, const struct flow *flow,
struct flow_wildcards *wc) {
uint32_t basis = 0, hash;
- struct cls_match *rule;
+ struct cls_match *rule = NULL;
int i;
struct range ofs;
@@ -1767,7 +1767,8 @@ static struct cls_match * next_rule_in_list__(struct cls_match *rule)
OVS_NO_THREAD_SAFETY_ANALYSIS
{
- struct cls_match *next = OBJECT_CONTAINING(rule->list.next, next, list);
+ struct cls_match *next = NULL;
+ next = OBJECT_CONTAINING(rule->list.next, next, list);
return next;
}
diff --git a/lib/classifier.h b/lib/classifier.h index b394724..d6ab144 100644
--- a/lib/classifier.h
+++ b/lib/classifier.h
@@ -334,7 +334,7 @@ void cls_cursor_advance(struct cls_cursor *);
#define CLS_FOR_EACH_TARGET(RULE, MEMBER, CLS, TARGET) \
for (struct cls_cursor cursor__ = cls_cursor_start(CLS, TARGET, false); \
(cursor__.rule \
- ? (ASSIGN_CONTAINER(RULE, cursor__.rule, MEMBER), \
+ ? (INIT_CONTAINER(RULE, cursor__.rule, MEMBER), \
true) \
: false); \
cls_cursor_advance(&cursor__)) @@ -345,7 +345,7 @@ void cls_cursor_advance(struct cls_cursor *);
#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), \
+ ? (INIT_CONTAINER(RULE, cursor__.rule, MEMBER), \
cls_cursor_advance(&cursor__), \
true) \
: false); \
diff --git a/lib/cmap.h b/lib/cmap.h
index 038db6c..793202d 100644
--- a/lib/cmap.h
+++ b/lib/cmap.h
@@ -114,11 +114,11 @@ void cmap_replace(struct cmap *, struct cmap_node *old_node,
* to change during iteration. It may be very slightly faster.
*/
#define CMAP_FOR_EACH_WITH_HASH(NODE, MEMBER, HASH, CMAP) \
- for (ASSIGN_CONTAINER(NODE, cmap_find(CMAP, HASH), MEMBER); \
+ for (INIT_CONTAINER(NODE, cmap_find(CMAP, HASH), MEMBER); \
(NODE) != OBJECT_CONTAINING(NULL, NODE, MEMBER); \
ASSIGN_CONTAINER(NODE, cmap_node_next(&(NODE)->MEMBER), MEMBER))
#define CMAP_FOR_EACH_WITH_HASH_PROTECTED(NODE, MEMBER, HASH, CMAP) \
- for (ASSIGN_CONTAINER(NODE, cmap_find_locked(CMAP, HASH), MEMBER); \
+ for (INIT_CONTAINER(NODE, cmap_find_locked(CMAP, HASH), MEMBER); \
(NODE) != OBJECT_CONTAINING(NULL, NODE, MEMBER); \
ASSIGN_CONTAINER(NODE, cmap_node_next_protected(&(NODE)->MEMBER), \
MEMBER))
@@ -174,7 +174,7 @@ struct cmap_node *cmap_find_protected(const struct cmap *, uint32_t hash);
#define CMAP_CURSOR_FOR_EACH__(NODE, CURSOR, MEMBER) \
((CURSOR)->node \
- ? (ASSIGN_CONTAINER(NODE, (CURSOR)->node, MEMBER), \
+ ? (INIT_CONTAINER(NODE, (CURSOR)->node, MEMBER), \
cmap_cursor_advance(CURSOR), \
true) \
: false)
diff --git a/lib/heap.h b/lib/heap.h
index 870f582..8de9ea6 100644
--- a/lib/heap.h
+++ b/lib/heap.h
@@ -68,7 +68,7 @@ void heap_rebuild(struct heap *);
* element. */
#define HEAP_FOR_EACH(NODE, MEMBER, HEAP) \
for (((HEAP)->n > 0 \
- ? ASSIGN_CONTAINER(NODE, (HEAP)->array[1], MEMBER) \
+ ? INIT_CONTAINER(NODE, (HEAP)->array[1], MEMBER) \
: ((NODE) = NULL, (void) 0)); \
(NODE) != NULL; \
((NODE)->MEMBER.idx < (HEAP)->n \
diff --git a/lib/hindex.h b/lib/hindex.h index 631fd48..416da05 100644
--- a/lib/hindex.h
+++ b/lib/hindex.h
@@ -128,7 +128,7 @@ void hindex_remove(struct hindex *, struct hindex_node *);
* Evaluates HASH only once.
*/
#define HINDEX_FOR_EACH_WITH_HASH(NODE, MEMBER, HASH, HINDEX) \
- for (ASSIGN_CONTAINER(NODE, hindex_node_with_hash(HINDEX, HASH), MEMBER); \
+ for (INIT_CONTAINER(NODE, hindex_node_with_hash(HINDEX, HASH),
+ MEMBER); \
NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER); \
ASSIGN_CONTAINER(NODE, (NODE)->MEMBER.s, MEMBER))
@@ -149,16 +149,16 @@ hindex_node_with_hash(const struct hindex *hindex, size_t hash)
/* Iterates through every node in HINDEX. */
#define HINDEX_FOR_EACH(NODE, MEMBER, HINDEX) \
- for (ASSIGN_CONTAINER(NODE, hindex_first(HINDEX), MEMBER); \
+ for (INIT_CONTAINER(NODE, hindex_first(HINDEX), MEMBER); \
NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER); \
ASSIGN_CONTAINER(NODE, hindex_next(HINDEX, &(NODE)->MEMBER), MEMBER))
/* Safe when NODE may be freed (not needed when NODE may be removed from the
* hash index but its members remain accessible and intact). */
-#define HINDEX_FOR_EACH_SAFE(NODE, NEXT, MEMBER, HINDEX) \
- for (ASSIGN_CONTAINER(NODE, hindex_first(HINDEX), MEMBER); \
+#define HINDEX_FOR_EACH_SAFE(NODE, NEXT, MEMBER, HINDEX) \
+ for (INIT_CONTAINER(NODE, hindex_first(HINDEX), MEMBER); \
(NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER) \
- ? ASSIGN_CONTAINER(NEXT, hindex_next(HINDEX, &(NODE)->MEMBER), MEMBER), 1 \
+ ? INIT_CONTAINER(NEXT, hindex_next(HINDEX, &(NODE)->MEMBER),
+ MEMBER), 1 \
: 0); \
(NODE) = (NEXT))
diff --git a/lib/hmap.h b/lib/hmap.h
index 9fb83d5..5fcb7a2 100644
--- a/lib/hmap.h
+++ b/lib/hmap.h
@@ -126,12 +126,12 @@ struct hmap_node *hmap_random_node(const struct hmap *);
* HASH is only evaluated once.
*/
#define HMAP_FOR_EACH_WITH_HASH(NODE, MEMBER, HASH, HMAP) \
- for (ASSIGN_CONTAINER(NODE, hmap_first_with_hash(HMAP, HASH), MEMBER); \
+ for (INIT_CONTAINER(NODE, hmap_first_with_hash(HMAP, HASH),
+ MEMBER); \
NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER); \
ASSIGN_CONTAINER(NODE, hmap_next_with_hash(&(NODE)->MEMBER), \
MEMBER))
#define HMAP_FOR_EACH_IN_BUCKET(NODE, MEMBER, HASH, HMAP) \
- for (ASSIGN_CONTAINER(NODE, hmap_first_in_bucket(HMAP, HASH), MEMBER); \
+ for (INIT_CONTAINER(NODE, hmap_first_in_bucket(HMAP, HASH),
+ MEMBER); \
NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER); \
ASSIGN_CONTAINER(NODE, hmap_next_in_bucket(&(NODE)->MEMBER), MEMBER))
@@ -148,16 +148,16 @@ bool hmap_contains(const struct hmap *, const struct hmap_node *);
/* Iterates through every node in HMAP. */
#define HMAP_FOR_EACH(NODE, MEMBER, HMAP) \
- for (ASSIGN_CONTAINER(NODE, hmap_first(HMAP), MEMBER); \
+ for (INIT_CONTAINER(NODE, hmap_first(HMAP), MEMBER); \
NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER); \
ASSIGN_CONTAINER(NODE, hmap_next(HMAP, &(NODE)->MEMBER), MEMBER))
/* Safe when NODE may be freed (not needed when NODE may be removed from the
* hash map but its members remain accessible and intact). */
#define HMAP_FOR_EACH_SAFE(NODE, NEXT, MEMBER, HMAP) \
- for (ASSIGN_CONTAINER(NODE, hmap_first(HMAP), MEMBER); \
+ for (INIT_CONTAINER(NODE, hmap_first(HMAP), MEMBER); \
(NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER) \
- ? ASSIGN_CONTAINER(NEXT, hmap_next(HMAP, &(NODE)->MEMBER), MEMBER), 1 \
+ ? INIT_CONTAINER(NEXT, hmap_next(HMAP, &(NODE)->MEMBER),
+ MEMBER), 1 \
: 0); \
(NODE) = (NEXT))
diff --git a/lib/list.h b/lib/list.h
index 0da082e..ef6a9db 100644
--- a/lib/list.h
+++ b/lib/list.h
@@ -58,15 +58,15 @@ bool list_is_singleton(const struct list *); bool list_is_short(const struct list *);
#define LIST_FOR_EACH(ITER, MEMBER, LIST) \
- for (ASSIGN_CONTAINER(ITER, (LIST)->next, MEMBER); \
+ for (INIT_CONTAINER(ITER, (LIST)->next, MEMBER); \
&(ITER)->MEMBER != (LIST); \
ASSIGN_CONTAINER(ITER, (ITER)->MEMBER.next, MEMBER))
#define LIST_FOR_EACH_CONTINUE(ITER, MEMBER, LIST) \
- for (ASSIGN_CONTAINER(ITER, (ITER)->MEMBER.next, MEMBER); \
+ for (INIT_CONTAINER(ITER, (ITER)->MEMBER.next, MEMBER); \
&(ITER)->MEMBER != (LIST); \
ASSIGN_CONTAINER(ITER, (ITER)->MEMBER.next, MEMBER))
#define LIST_FOR_EACH_REVERSE(ITER, MEMBER, LIST) \
- for (ASSIGN_CONTAINER(ITER, (LIST)->prev, MEMBER); \
+ for (INIT_CONTAINER(ITER, (LIST)->prev, MEMBER); \
&(ITER)->MEMBER != (LIST); \
ASSIGN_CONTAINER(ITER, (ITER)->MEMBER.prev, MEMBER))
#define LIST_FOR_EACH_REVERSE_CONTINUE(ITER, MEMBER, LIST) \
@@ -74,9 +74,9 @@ bool list_is_short(const struct list *);
&(ITER)->MEMBER != (LIST); \
ASSIGN_CONTAINER(ITER, (ITER)->MEMBER.prev, MEMBER))
#define LIST_FOR_EACH_SAFE(ITER, NEXT, MEMBER, LIST) \
- for (ASSIGN_CONTAINER(ITER, (LIST)->next, MEMBER); \
+ for (INIT_CONTAINER(ITER, (LIST)->next, MEMBER); \
(&(ITER)->MEMBER != (LIST) \
- ? ASSIGN_CONTAINER(NEXT, (ITER)->MEMBER.next, MEMBER), 1 \
+ ? INIT_CONTAINER(NEXT, (ITER)->MEMBER.next, MEMBER), 1 \
: 0); \
(ITER) = (NEXT))
diff --git a/lib/util.h b/lib/util.h
index 261b4b3..a2c6ee9 100644
--- a/lib/util.h
+++ b/lib/util.h
@@ -228,6 +228,13 @@ ovs_prefetch_range(const void *start, size_t size) #define ASSIGN_CONTAINER(OBJECT, POINTER, MEMBER) \
((OBJECT) = OBJECT_CONTAINING(POINTER, OBJECT, MEMBER), (void) 0)
+/* As explained in the comment above OBJECT_OFFSETOF(), non-GNUC
+compilers
+ * like MSVC will complain about un-initialized variables if OBJECT
+ * hasn't already been initialized. To prevent such warnings,
+INIT_CONTAINER()
+ * can be used as a wrapper around ASSIGN_CONTAINER. */ #define
+INIT_CONTAINER(OBJECT, POINTER, MEMBER) \
+ ((OBJECT) = NULL, ASSIGN_CONTAINER(OBJECT, POINTER, MEMBER))
+
/* Given ATTR, and TYPE, cast the ATTR to TYPE by first casting ATTR to
* (void *). This is to suppress the alignment warning issued by clang. */ #define ALIGNED_CAST(TYPE, ATTR) ((TYPE) (void *) (ATTR))
--
1.7.9.5
_______________________________________________
dev mailing list
dev at openvswitch.org
https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=yTvML8OxA42Jb6ViHe7fUXbvPVOYDPVq87w43doxtlY%3D%0A&m=vhaaiwGg2o4fdzo1Y4GXNiuZqtaokSkxaJCi51lfBLo%3D%0A&s=916b857d57af38bd52b9bf30cb0dda65b6f0ae970620dcd2800b619fe86a4dee
_______________________________________________
dev mailing list
dev at openvswitch.org
https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=yTvML8OxA42Jb6ViHe7fUXbvPVOYDPVq87w43doxtlY%3D%0A&m=uzgOHkSF0d95NbtVAcQIQXP0T4OJNQaJOAX%2FZCXrnbk%3D%0A&s=49cccc3419f982168d7a4c74f94787a148fdf7503c6362026faa8c02ae366aa5
More information about the dev
mailing list