+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" <[email protected]> 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 <[email protected]>
>> Signed-off-by: Shireesh Kumar Singh <[email protected]>
>> Co-authored-by: Shireesh Kumar Singh <[email protected]>
>
>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.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
