On Tue, Jan 16, 2024 at 09:03:01AM -0800, Stephen Hemminger wrote: > Ran into a corner case issue, so sending to mailing list for wider discussion. > > One improvement to DPDK code base planned is getting rid of variable length > arrays. > VLA's can cause bugs and are not supported by the Windows compiler. > Gcc and Clang have a flag to warn on use of VLA's (-Wvla). > > In DPDK one use of VLA's is in the RTE_BUILD_BUG_ON macro. > The best path is to replace the undefined array access with a better > static_assert() which is builtin to compilers. > > Using static_assert() is relatively easy, there are a few places where > it does detect use of non-constant expressions in existing code but these > are fixable. > > But there is one case where use static_assert() runs into a Clang bug > which is not fixed in distros: > > https://github.com/llvm/llvm-project/issues/55821 > > The code in question is in the sfc driver which for better or worse > has lots of assertions. The problem is that clang (except in unreleased trunk) > can not handle static_assert in a switch leg. > > switch (actions->type) { > case RTE_FLOW_ACTION_TYPE_VOID: > SFC_BUILD_SET_OVERFLOW(RTE_FLOW_ACTION_TYPE_VOID, > actions_set); > break; > > ../drivers/net/sfc/sfc_flow.c:1635:4: error: expected expression > SFC_BUILD_SET_OVERFLOW(RTE_FLOW_ACTION_TYPE_VOID, > ^ > ../drivers/net/sfc/sfc_flow.h:36:2: note: expanded from macro > 'SFC_BUILD_SET_OVERFLOW' > RTE_BUILD_BUG_ON((_action) >= sizeof(_set) * CHAR_BIT) > ^ > > > There are some workarounds: > 0. Ignore it, works on Gcc, and Clang fix is pending. > 1. Remove many of these RTE_BUILD_BUG_ON's. They are really not that > helpful. > 2. Add additional { } to these switch cases so they become basic blocks > which works around the bug. > 3. Move the assertions out of the switch > > My preference is #2 but open to other options.
+1 for #2 just make it a block.
