Re: [PATCH for-6.1 4/4] configure: Poison all current target-specific #defines

[Thomas Huth]

On 15/04/2021 10.34, Philippe Mathieu-Daudé wrote:
> +Richard/Claudio
>
> On 4/14/21 1:20 PM, Thomas Huth wrote:
> > We are generating a lot of target-specific defines in the *-config-devices.h
> > and *-config-target.h files. Using them in common code is wrong and leads
> > to very subtle bugs since a "#ifdef CONFIG_SOMETHING" is not working there
> > as expected. To avoid these issues, we are already poisoning many of the
> > macros in include/exec/poison.h - but it's cumbersome to maintain this
> > list manually. Thus let's generate an additional list of poisoned macros
> > automatically from the current config switches - this should give us a
> > much better test coverage via the different CI configurations.
> >
> > Note that CONFIG_TCG (which is also defined in config-host.h) and
> > CONFIG_USER_ONLY are special, so we have to filter these out.
>
> I know if we poison CONFIG_TCG, almost nothing build, but I fail to
> see how it is different from the other accelerators. 

You could argue that TCG is not specific to a target, i.e. it is either 
available for all targets, or it is disabled for all targets, so this is 
rather like a host config option than a target specific option.

> > diff --git a/configure b/configure
> > index 4f374b4889..a0f0601e7e 100755
> > --- a/configure
> > +++ b/configure
> > @@ -6440,6 +6440,11 @@ if test -n "${deprecated_features}"; then
> >       echo "  features: ${deprecated_features}"
> >   fi
> >   
>
> Maybe a one line comment (but since it is obvious, I don't mind):
>
>     # Filter out CONFIG_TCG and CONFIG_USER_ONLY which are special

Ok, makes sense, I can add that.

> > +sed -n -e '/CONFIG_TCG/d' -e '/CONFIG_USER_ONLY/d' \
> > +    -e '/^#define / { s///; s/ .*//; s/^/#pragma GCC poison /p; }' \
> > +    *-config-devices.h *-config-target.h | \
> > +    sort -u > config-poison.h
> > +
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks!

  Thomas