[ovs-dev] [PATCH] list.h: Define OVS_LIST_POISON statically

[Nithin Raju]

I am fine with the suggestion and the patch you sent. Thanks Ben.

-----Original Message-----
From: Ben Pfaff <blp at ovn.org>
Date: Friday, March 18, 2016 at 1:47 PM
To: Nithin Raju <nithin at vmware.com>
Cc: "dev at openvswitch.org" <dev at openvswitch.org>, Yin Lin <linyi at 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 <linyi at vmware.com>
>> Signed-off-by: Nithin Raju <nithin at 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 <nithin at 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 <linyi at vmware.com>
>Signed-off-by: Nithin Raju <nithin at vmware.com>
>Signed-off-by: Ben Pfaff <blp at 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
>