As an alternative I tried to use decltype() operator (which I thing is the
right way to do it):
#define OBJECT_OFFSETOF(OBJECT, MEMBER) \
((LONG)(LONG_PTR)&((decltype(OBJECT))0)->MEMBER);
This worked correctly in a standalone program I wrote but for some reason any
use decltypea s an l-value does not work in OVS source files.
Anyway, usage of the warning pragma is fine for now.
Thanks,
Eitan
-----Original Message-----
From: Gurucharan Shetty [mailto:shet...@nicira.com]
Sent: Thursday, September 11, 2014 8:30 AM
To: Eitan Eliahu
Cc: dev@openvswitch.org; Gurucharan Shetty
Subject: Re: [ovs-dev] [PATCH 3/3] Avoid uninitialized variable warnings with
OBJECT_OFFSETOF() in MSVC.
On Wed, Sep 10, 2014 at 10:20 PM, Eitan Eliahu <elia...@vmware.com> wrote:
> 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 )
For this particular case, I think what you suggest is a reasonable approach, if
it actually worked (or if you know how to get it working).
For e.g., the following diff applied on the tip of master should have solved
the problem for us.
diff --git a/lib/util.h b/lib/util.h
index dc34ee5..951aca8 100644
--- a/lib/util.h
+++ b/lib/util.h
@@ -201,7 +201,10 @@ ovs_prefetch_range(const void *start, size_t size)
#define OBJECT_OFFSETOF(OBJECT, MEMBER) offsetof(typeof(*(OBJECT)), MEMBER)
#else #define OBJECT_OFFSETOF(OBJECT, MEMBER) \
- ((char *) &(OBJECT)->MEMBER - (char *) (OBJECT))
+ __pragma (warning(push)) \
+ __pragma (warning(disable:4700)) \
+ ((char *) &(OBJECT)->MEMBER - (char *) (OBJECT)) \
+ __pragma (warning(pop))
#endif
But it doesn't. I think the reason it doesn't is because of the following
information in msdn documentation.
"For warning numbers in the range 4700-4999, which are the ones associated with
code generation, the state of the warning in effect when the compiler
encounters the open curly brace of a function will be in effect for the rest of
the function. Using the warning pragma in the function to change the state of a
warning that has a number larger than 4699 will only take effect after the end
of the function. The following example shows the correct placement of warning
pragmas to disable a code-generation warning message, and then to restore it."
Ref:
https://urldefense.proofpoint.com/v1/url?u=http://msdn.microsoft.com/en-us/library/2c8f766e.aspx&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=yTvML8OxA42Jb6ViHe7fUXbvPVOYDPVq87w43doxtlY%3D%0A&m=4G2z2MXJbEYppIl73yQrveJTD3D0erYuXZJkQ%2BTj8Lo%3D%0A&s=adcbafd1e3da873656b20a37dc68f6b7d2de3c66db71c7d7fc7e33ff11e8d26e
If what the above says is true, it is not really feasible to disable a warning
for every function that has a *_FOR_EACH macro call.
>
> 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/mail
> man/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=yTvML8OxA42Jb6V
> iHe7fUXbvPVOYDPVq87w43doxtlY%3D%0A&m=vhaaiwGg2o4fdzo1Y4GXNiuZqtaokSkxa
> JCi51lfBLo%3D%0A&s=916b857d57af38bd52b9bf30cb0dda65b6f0ae970620dcd2800
> b619fe86a4dee
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
