Does MSVC support decltype in C or only in C++? On Thu, Sep 11, 2014 at 10:41 AM, Eitan Eliahu <elia...@vmware.com> wrote: > 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
-- "I don't normally do acked-by's. I think it's my way of avoiding getting blamed when it all blows up." Andrew Morton _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
