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