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:aserd...@cloudbasesolutions.com] Sent: Thursday, September 11, 2014 1:07 AM To: Eitan Eliahu; Gurucharan Shetty; dev@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-boun...@openvswitch.org] În numele Eitan Eliahu Trimis: Wednesday, September 10, 2014 10:21 PM Către: Gurucharan Shetty; dev@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-boun...@openvswitch.org] On Behalf Of Gurucharan Shetty Sent: Wednesday, September 10, 2014 12:56 PM To: dev@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 <gshe...@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@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@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 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
