Re: [RFC PATCH] configure: Poison (almost) all target-specific #defines

[Claudio Fontana]

On 3/15/21 4:08 PM, Thomas Huth wrote:
> On 15/03/2021 15.07, Claudio Fontana wrote:
>> On 3/15/21 2:54 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 some of the
>>> macros in include/exec/poison.h - but maintaining this list manually is
>>> cumbersome. Thus let's generate the list of poisoned macros automatically
>>> instead.
>>> 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 have the impression that CONFIG_USER_ONLY should be poisoned too.
>>
>> A lot of the
>>
>> #ifndef CONFIG_USER_ONLY
>>
>> end up currently doing the wrong thing in common modules includes,
>> especially due to the inverted nature of the check.
> 
> Not sure about that ... do you have an example at hand?

it was the whole story around hw/core/cpu.h .

It contains CONFIG_USER_ONLY, with the unwanted behavior mentioned,
and seeing its existing use, I stopped short of introducing a bug:

https://www.mail-archive.com/qemu-devel@nongnu.org/msg768318.html

Other code in hw/core/cpu.c also uses CONFIG_USER_ONLY (See the XXX),

and the hw/core/cpu.h continues to carry CONFIG_USER_ONLY, with the potential 
to lead other people to misuse it
(putting in an extra prototype is harmless, but an extra field isn't).



> 
> Anyway, one thing is sure, if we want to poison CONFIG_USER_ONLY, this will 
> certainly cause a lot of clean up work first, since it is used all over the 
> place...
> 
>   Thomas

Yes, and probably a good idea.

Thanks,

Claudio