On Thu, Jan 23, 2025 at 12:43:04PM +0000, Bruce Richardson wrote:
> On Thu, Jan 23, 2025 at 12:58:49PM +0100, David Marchand wrote:
> > On Tue, Jan 14, 2025 at 3:32 AM Andre Muezerie
> > <andre...@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
> > +
>
> Minor simplification suggestion, put "no_vla_cflag = []" outside the
> conditionals at the start, as the default value. Save having multiple
> copies of that assignment, and having to do "else" legs.
>
> /Bruce
These look like great improvements. I especially like the idea of using -Wvla
from the very top.
