LGTM with one question below,

Acked-by: Jarno Rajahalme <[email protected]>

> On Mar 16, 2017, at 2:04 PM, Ben Pfaff <[email protected]> wrote:
> 
> Until now, the BUILD_ASSERT and BUILD_ASSERT_DECL macros have used OVS's
> home-grown build assertion strategy.  This commit switches them to using
> C11 build assertions with compilers that support them.  The semantics are
> the same, but C11 build assertions yield clearer error messages when they
> fail.
> 
> This commit also reorders the definitions a bit to make it easier to
> follow.
> 
> Signed-off-by: Ben Pfaff <[email protected]>
> ---
> include/openvswitch/compiler.h | 28 +++++++++++++++-------------
> 1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/include/openvswitch/compiler.h b/include/openvswitch/compiler.h
> index 6e779f38fd16..0dc8636add33 100644
> --- a/include/openvswitch/compiler.h
> +++ b/include/openvswitch/compiler.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2016 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2016, 2017 
> Nicira, Inc.
>  *
>  * Licensed under the Apache License, Version 2.0 (the "License");
>  * you may not use this file except in compliance with the License.
> @@ -236,26 +236,28 @@
> #define OVS_PREFETCH_WRITE(addr)
> #endif
> 
> -/* Build assertions. */
> +/* Build assertions.
> + *
> + * Use BUILD_ASSERT_DECL as a declaration or a statement, or BUILD_ASSERT as
> + * part of an expression. */
> #ifdef __CHECKER__
> #define BUILD_ASSERT(EXPR) ((void) 0)
> #define BUILD_ASSERT_DECL(EXPR) extern int (*build_assert(void))[1]
> -#elif !defined(__cplusplus)
> -/* Build-time assertion building block. */
> +#elif defined(__cplusplus)
> +#include <boost/static_assert.hpp>
> +#define BUILD_ASSERT BOOST_STATIC_ASSERT
> +#define BUILD_ASSERT_DECL BOOST_STATIC_ASSERT
> +#elif (__GNUC__ * 256 + __GNUC_MINOR__ >= 0x403 \
> +       || __has_extension(c_static_assert))
> +#define BUILD_ASSERT_DECL(EXPR) _Static_assert(EXPR, #EXPR)
> +#define BUILD_ASSERT(EXPR) (void) ({ _Static_assert(EXPR, #EXPR); })

Curly braces in a macro is a GCC feature, so is it possible that a compiler has 
the “c_static_assert” extension but not this one? I see that __has_extension() 
is defined as 0 if it is not defined, so if it it only ever defined for GCC or 
compatible compiler, then this question is moot.

  Jarno

> +#else
> #define BUILD_ASSERT__(EXPR) \
>         sizeof(struct { unsigned int build_assert_failed : (EXPR) ? 1 : -1; })
> -
> -/* Build-time assertion for use in a statement context. */
> #define BUILD_ASSERT(EXPR) (void) BUILD_ASSERT__(EXPR)
> -
> -/* Build-time assertion for use in a declaration context. */
> #define BUILD_ASSERT_DECL(EXPR) \
>         extern int (*build_assert(void))[BUILD_ASSERT__(EXPR)]
> -#else /* __cplusplus */
> -#include <boost/static_assert.hpp>
> -#define BUILD_ASSERT BOOST_STATIC_ASSERT
> -#define BUILD_ASSERT_DECL BOOST_STATIC_ASSERT
> -#endif /* __cplusplus */
> +#endif
> 
> #ifdef __GNUC__
> #define BUILD_ASSERT_GCCONLY(EXPR) BUILD_ASSERT(EXPR)
> -- 
> 2.10.2
> 
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to