[v3,5/5] eal: replace out of bounds VLA with static_assert

  Both Gcc, clang and MSVC have better way to do compile time
assertions rather than using out of bounds array access.
The old method would fail if -Wvla is enabled because compiler
can't determine size in that code.  Also, the use of new
_Static_assert will catch broken code that is passing non-constant
expression to RTE_BUILD_BUG_ON().

Need to add brackets {} around the static_assert() to workaround
a bug in clang. Clang was not handling static_assert() in
a switch case properly.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 lib/eal/include/rte_common.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
  

On 1/16/24 21:41, Stephen Hemminger wrote:
> Both Gcc, clang and MSVC have better way to do compile time
> assertions rather than using out of bounds array access.
> The old method would fail if -Wvla is enabled because compiler
> can't determine size in that code.  Also, the use of new
> _Static_assert will catch broken code that is passing non-constant
> expression to RTE_BUILD_BUG_ON().
> 
> Need to add brackets {} around the static_assert() to workaround
> a bug in clang. Clang was not handling static_assert() in
> a switch case properly.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

Does it imply any limitations on Gcc / Clang versions to
be used? Is it documented somewhere in DPDK?
  

On 2024-01-16 19:41, Stephen Hemminger wrote:
> Both Gcc, clang and MSVC have better way to do compile time
> assertions rather than using out of bounds array access.
> The old method would fail if -Wvla is enabled because compiler
> can't determine size in that code.  Also, the use of new
> _Static_assert will catch broken code that is passing non-constant
> expression to RTE_BUILD_BUG_ON().
> 
> Need to add brackets {} around the static_assert() to workaround
> a bug in clang. Clang was not handling static_assert() in
> a switch case properly.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
>   lib/eal/include/rte_common.h | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
> index c1ba32d00e47..413bed23cb73 100644
> --- a/lib/eal/include/rte_common.h
> +++ b/lib/eal/include/rte_common.h
> @@ -16,6 +16,7 @@
>   extern "C" {
>   #endif
>   
> +#include <assert.h>
>   #include <stdint.h>
>   #include <limits.h>
>   
> @@ -495,7 +496,7 @@ rte_is_aligned(const void * const __rte_restrict ptr, const unsigned int align)
>   /**
>    * Triggers an error at compilation time if the condition is true.
>    */
> -#define RTE_BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> +#define RTE_BUILD_BUG_ON(condition) { static_assert(!(condition), #condition); }
>   
>   /*********** Cache line related macros ********/
>   

Should one use RTE_BUILD_BUG_ON() or static_assert() in new DPDK code?

If static_assert() occasionally needs a work-around, it sounds like 
keeping using RTE_BUILD_BUG_ON() everywhere is the better choice.
  

On Wed, 17 Jan 2024 08:53:44 +0100
Mattias Rönnblom <hofors@lysator.liu.se> wrote:

> >    * Triggers an error at compilation time if the condition is true.
> >    */
> > -#define RTE_BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> > +#define RTE_BUILD_BUG_ON(condition) { static_assert(!(condition), #condition); }
> >   
> >   /*********** Cache line related macros ********/
> >     
> 
> Should one use RTE_BUILD_BUG_ON() or static_assert() in new DPDK code?
> 
> If static_assert() occasionally needs a work-around, it sounds like 
> keeping using RTE_BUILD_BUG_ON() everywhere is the better choice.

Either choice is the same. Keeping the macro instead of directly using
static_assert will reduce the effort to change when the next C standard
introduces something different.

But using static_assert() can allow for a better warning message.
  

On Wed, 17 Jan 2024 10:52:40 +0300
Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> wrote:

> On 1/16/24 21:41, Stephen Hemminger wrote:
> > Both Gcc, clang and MSVC have better way to do compile time
> > assertions rather than using out of bounds array access.
> > The old method would fail if -Wvla is enabled because compiler
> > can't determine size in that code.  Also, the use of new
> > _Static_assert will catch broken code that is passing non-constant
> > expression to RTE_BUILD_BUG_ON().
> > 
> > Need to add brackets {} around the static_assert() to workaround
> > a bug in clang. Clang was not handling static_assert() in
> > a switch case properly.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > Acked-by: Morten Brørup <mb@smartsharesystems.com>
> > Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>  
> 
> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> 
> Does it imply any limitations on Gcc / Clang versions to
> be used? Is it documented somewhere in DPDK?

No new language versions problems.  There are already direct
usages of static_assert() in some drivers.