[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