On 2/26/25 18:02, Ján Tomko wrote:
> On a Thursday in 2024, Andrea Bolognani wrote:
>> 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?
>
> I don't like all the extra additional meson code. Can it be somehow
> simplified?
>
> Calling:
> foreach function : functions
> conf.set('WITH_@0@'.format(function.to_upper()),
> cc.has_function(function))
> endforeach
> does not seem to work.
That's because conf.set(varname, value) behaves differently for
different types of $value:
if $value is boolean then "#define/#undef $varname" is generated,
if $value is int then "#define $varname $value" is generated,
and if $value is string then "#define $varname $value" is generated.
Long story short, we want .to_int():
foreach function : stat_functions
conf.set('WITH_@0@_DECL'.format(function.to_upper()),
cc.has_header_symbol('sys/stat.h', function).to_int())
endforeach
But this patch series fixes only a portion of the problem (that where
compiler complained about -Wundef). Similar pattern exists for other
$functions, e,g:
tests/virmock.h:#ifndef WITH___OPEN_2_DECL
I suggest turning at least those two foreach loops above and below too.
This still leaves us with plenty of:
if condition
conf.set('WITH_SOMETHING', 1)
endif
but well, who said this is trivial task?
Michal
