A few days ago I have posted a patch[1] that addresses an issue
introduced when a meson check was dropped but some uses of the
corresponding WITH_ macro were not removed at the same time.
That got me thinking about what we can do to prevent such scenarios
from happening again in the future. I have come up with something
that I think would be effective, but since applying the approach
throughout the entire codebase would require a non-trivial amount of
work, I figured I'd ask for feedback before embarking on it.
The idea is that there are two types of macros we can use for
conditional compilation: external ones, coming from the OS or other
libraries, and internal ones, which are the result of meson tests.
The external ones (e.g. SIOCSIFFLAGS, __APPLE__) are usually only
defined if they apply, so it is correct to check for their presence
with #ifdef. Using #if will also work, as undefined macros evaluate
to zero, but it's not good practice to use them that way. If -Wundef
has been passed to the compiler, those incorrect uses will be
reported (only on platforms where they are not defined, of course).
The internal ones (e.g. WITH_QEMU, WITH_STRUCT_IFREQ) are similar,
but in this case we control their definition. This means that using
means that the feature is not available on the machine we're building
on, but it could also mean that we've removed the meson check and
forgot to update all users of the macro. In this case, -Wundef would
work 100% reliably to detect the issue: if the meson check doesn't
exist, neither will the macro, regardless of what platform we're
building on.
So the approach I'm suggesting is to use a syntax-check rule to
ensure that internal macros are only ever checked with #if instead of
Of course this requires a full sweep to fix all cases in which we're
not already doing things according to the proposal. Should be fairly
easy, if annoying. A couple of examples are included here for
demonstration purposes.
The bigger impact is going to be on the build system. Right now we
generally only define WITH_ macros if the check passed, but that will
have to change and the result is going to be quite a bit of
additional meson code I'm afraid.
Thoughts?
[1]
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/S...
Andrea Bolognani (4):
configmake: Check for WIN32 correctly
meson: Always define WITH_*_DECL macros
syntax-check: Ensure WITH_ macros are used correctly
meson: Enable -Wundef
build-aux/syntax-check.mk | 5 +++++
configmake.h.in | 2 +-
meson.build | 3 +++
tests/virmockstathelpers.c | 28 ++++++++++++++--------------
4 files changed, 23 insertions(+), 15 deletions(-)
--
2.43.2