[PATCH v16 00/60] remove use of VLAs for Windows

[David Marchand]

On Tue, Jan 14, 2025 at 3:32 AM Andre Muezerie
<andremue at linux.microsoft.com> wrote:
>
> As per guidance technical board meeting 2024/04/17. This series
> removes the use of VLAs from code built for Windows for all 3
> toolchains. If there are additional opportunities to convert VLAs
> to regular C arrays please provide the details for incorporation
> into the series.
>
> MSVC does not support VLAs, replace VLAs with standard C arrays
> or alloca(). alloca() is available for all toolchain/platform
> combinations officially supported by DPDK.
>
> v16:
>   * remove -Wvla from drivers/common/mlx5/meson.build and
>     drivers/common/qat/meson.build
>
> v15:
>   * inverted some of the logic added during v14:
>     add -Wvla to meson build files in app and lib directories, adding
>     -Wno-vla to the few subdirectories which are not yet VLA free
>
> v14:
>   * add -Wvla to meson build for directories that are VLA free
>     under app, lib, drivers. This is to ensure that new VLAs are
>     not added to these directories in the future.

Thanks for working on this topic.

I see there is some back and forth on the topic of passing -Wvla.
It would be less fragile to put a -Wla in a upper level meson.build
(like config/meson.build for example), then disable explicitly in the
parts that are not ready.

Something like:
diff --git a/config/meson.build b/config/meson.build
index 6aaad6d8a4..be603bd45b 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -348,6 +348,17 @@ foreach arg: warning_flags
     endif
 endforeach

+if cc.has_argument('-Wvla')
+    add_project_arguments('-Wvla', language: 'c')
+    if not is_windows
+        no_vla_cflag = '-Wno-vla'
+    else
+        no_vla_cflag = []
+    endif
+else
+    no_vla_cflag = []
+endif
+
 # set other values pulled from the build options
 dpdk_conf.set('RTE_MAX_ETHPORTS', get_option('max_ethports'))
 dpdk_conf.set('RTE_LIBEAL_USE_HPET', get_option('use_hpet'))


This has the benefit of avoiding repeating those if cc.has_argument()
loops in all meson.build.
Disabling becomes simply a matter of adding cflags += no_vla_cflag.

This also enforces -Wvla for code that is built on windows (with mingw build).


I had a try, and flagged all remaining components that have VLA in them.
You can have a look at:
https://github.com/david-marchand/dpdk/commit/vla_v16_dma

This helped me catch a new VLA in the recently merged soring test code:
https://github.com/david-marchand/dpdk/commit/vla_v16_dma~1


WDYT?

-- 
David Marchand