Applied, thanks.

On Sun, Mar 20, 2016 at 02:37:45AM +0000, Nithin Raju wrote:
> I am fine with the suggestion and the patch you sent. Thanks Ben.
> 
> -----Original Message-----
> From: Ben Pfaff <b...@ovn.org>
> Date: Friday, March 18, 2016 at 1:47 PM
> To: Nithin Raju <nit...@vmware.com>
> Cc: "dev@openvswitch.org" <dev@openvswitch.org>, Yin Lin <li...@vmware.com>
> Subject: Re: [ovs-dev] [PATCH] list.h: Define OVS_LIST_POISON statically
> 
> >On Fri, Mar 18, 2016 at 01:17:54PM -0700, Nithin Raju wrote:
> >> The previous definitions of these variables using designated
> >> initializers caused a variety of issues when attempting to
> >> compile with MSVC, particularly if including these headers from C++
> >> code. By defining them like this, we can appease MSVC and keep the
> >> definitions the same on all platforms.
> >> 
> >> Suggested-by: Yin Lin <li...@vmware.com>
> >> Signed-off-by: Nithin Raju <nit...@vmware.com>
> >> ---
> >>  lib/list.h | 12 ++++++++----
> >>  1 file changed, 8 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/lib/list.h b/lib/list.h
> >> index f9c9d85..24c57ba 100644
> >> --- a/lib/list.h
> >> +++ b/lib/list.h
> >> @@ -24,10 +24,14 @@
> >>  #include "openvswitch/list.h"
> >>  
> >>  /* "struct ovs_list" with pointers that will (probably) cause
> >>segfaults if
> >> - * dereferenced and, better yet, show up clearly in a debugger. */
> >> -#define OVS_LIST_POISON \
> >> -(struct ovs_list) { (struct ovs_list *) (uintptr_t)
> >>0xccccccccccccccccULL, \
> >> -                    (struct ovs_list *) (uintptr_t)
> >>0xccccccccccccccccULL }
> >> + * dereferenced and, better yet, show up clearly in a debugger.
> >> +
> >> + * MSVC2015 doesn't support designated initializers when compiling C++,
> >> + * and doesn't support ternary operators with non-designated
> >>initializers.
> >> + * So we use these static definitions rather than using initializer
> >>macros. */
> >> +static const struct ovs_list OVS_LIST_POISON =
> >> +    { (struct ovs_list *) (uintptr_t) 0xccccccccccccccccULL,
> >> +      (struct ovs_list *) (uintptr_t) 0xccccccccccccccccULL };
> >
> >This causes a lot of new warnings from sparse, e.g.:
> >
> >    ../lib/list.h:33:39: warning: cast truncates bits from constant value
> >(cccccccccccccccc becomes cccccccc)
> >    ../lib/list.h:34:39: warning: cast truncates bits from constant value
> >(cccccccccccccccc becomes cccccccc)
> >
> >How about the following?  It reuses a trick we previously have used in
> >rculist.h.
> >
> >--8<--------------------------cut here-------------------------->8--
> >
> >From: Nithin Raju <nit...@vmware.com>
> >Date: Fri, 18 Mar 2016 13:17:54 -0700
> >Subject: [PATCH] list.h: Define OVS_LIST_POISON statically
> >
> >The previous definitions of these variables using designated
> >initializers caused a variety of issues when attempting to
> >compile with MSVC, particularly if including these headers from C++
> >code. By defining them like this, we can appease MSVC and keep the
> >definitions the same on all platforms.
> >
> >Suggested-by: Yin Lin <li...@vmware.com>
> >Signed-off-by: Nithin Raju <nit...@vmware.com>
> >Signed-off-by: Ben Pfaff <b...@ovn.org>
> >---
> > lib/list.h | 14 +++++++++-----
> > 1 file changed, 9 insertions(+), 5 deletions(-)
> >
> >diff --git a/lib/list.h b/lib/list.h
> >index f9c9d85..96bbafd 100644
> >--- a/lib/list.h
> >+++ b/lib/list.h
> >@@ -1,5 +1,5 @@
> > /*
> >- * Copyright (c) 2008, 2009, 2010, 2011, 2013, 2015 Nicira, Inc.
> >+ * Copyright (c) 2008, 2009, 2010, 2011, 2013, 2015, 2016 Nicira, Inc.
> >  *
> >  * Licensed under the Apache License, Version 2.0 (the "License");
> >  * you may not use this file except in compliance with the License.
> >@@ -24,10 +24,14 @@
> > #include "openvswitch/list.h"
> > 
> > /* "struct ovs_list" with pointers that will (probably) cause segfaults
> >if
> >- * dereferenced and, better yet, show up clearly in a debugger. */
> >-#define OVS_LIST_POISON \
> >-(struct ovs_list) { (struct ovs_list *) (uintptr_t)
> >0xccccccccccccccccULL, \
> >-                    (struct ovs_list *) (uintptr_t)
> >0xccccccccccccccccULL }
> >+ * dereferenced and, better yet, show up clearly in a debugger.
> >+
> >+ * MSVC2015 doesn't support designated initializers when compiling C++,
> >+ * and doesn't support ternary operators with non-designated
> >initializers.
> >+ * So we use these static definitions rather than using initializer
> >macros. */
> >+static const struct ovs_list OVS_LIST_POISON =
> >+    { (struct ovs_list *) (UINTPTR_MAX / 0xf * 0xc),
> >+      (struct ovs_list *) (UINTPTR_MAX / 0xf * 0xc) };
> > 
> > static inline void list_init(struct ovs_list *);
> > static inline void list_poison(struct ovs_list *);
> >-- 
> >2.1.3
> >
> 
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to