[ovs-dev] [PATCH v2] windows/lib: Fix Windows C++ compilation issues on common headers
Sairam Venugopal
vsairam at vmware.com
Mon Dec 18 21:19:33 UTC 2017
+Alin/Nithin for Windows.
Hi Ben,
Thanks for reviewing the changes. The BUILD_ASSERT change is required in this instance primarily because of its usage. Based on CPP reference, A static assert declaration may appear at block scope (as a block declaration) and inside a class body (as a member declaration). However, unlike sizeof, static_assert cannot be used as an expression and this is strictly enforced on MSVC.
In lib/unaligned.h, we call "return get_unaligned_u64(p);” which gets evaluated as following on Windows:
return ((static_assert(sizeof *(p) == 8, "sizeof *(p) == 8")), ((void) 0), (void) sizeof (*(p) % 1), get_unaligned_u64__((const uint64_t *) (p)));
This usage is deemed invalid by Windows Compiler - error C2059: syntax error : ‘static_assert’. If the solution is to prevent multiple #ifdef(s), then I need to get rid of the following get_unaligned_u64(P) definition and instead split this into individual statements. Updating the BUILD_ASSERT macro for Windows isn’t ideal since the other usages don’t encounter this error and this would end up creating a lambda for all existing usages. It felt simpler to add an ifdef for WIN32 && CPP just for this use-case instead of modifying the underlying libraries.
Definition:
#define get_unaligned_u64(P) \
(BUILD_ASSERT(sizeof *(P) == 8), \
BUILD_ASSERT_GCCONLY(!TYPE_IS_SIGNED(typeof(*(P)))), \
(void) sizeof (*(P) % 1), \
get_unaligned_u64__((const uint64_t *) (P)))
Please let me know how you would like for me to proceed. I can either get rid of the #define get_unaligned_u64(P) and instead convert them to statements or retain the current patch while updating the comments around why this workaround is required.
Thanks,
Sairam
On 12/14/17, 2:12 PM, "Ben Pfaff" <blp at ovn.org> wrote:
>On Wed, Dec 13, 2017 at 04:13:20PM -0800, Sairam Venugopal wrote:
>> Found when compiling the code with C++ binaries. Most of the issues are
>> due to missing explicit cast.
>>
>> Changes in PADDED_MEMBERS* is because MSVC does not allow to re-define
>> unnamed structure in union. Thus, this fix defines the struct outside of
>> the anonymous union in order to calculate the padded size.
>>
>> Signed-off-by: Sairam Venugopal <vsairam at vmware.com>
>> Signed-off-by: Shireesh Kumar Singh <shireeshkum at vmware.com>
>> Co-authored-by: Shireesh Kumar Singh <shireeshkum at vmware.com>
>
>Thanks for working on this.
>
>I think that this should actually be a series of patches:
>
> - PADDED_MEMBERS fix.
>
> - Added casts
>
> - Changes to types in the atomics header.
>
> - Initializer changes.
>
> - BUILD_ASSERT change. I think that this change should actually
> be a change to the BUILD_ASSERT macro. It does not make sense
> to change just one user.
>
>Thanks,
>
>Ben.
More information about the dev
mailing list