On Mon Jun 2, 2025 at 9:53 AM CEST, Jan Beulich wrote:
> On 30.05.2025 14:02, Alejandro Vallejo wrote:> ---
> a/xen/include/xen/grant_table.h
>> +++ b/xen/include/xen/grant_table.h
>> @@ -27,7 +27,7 @@
>> #include <xen/rwlock.h>
>> #include <public/grant_table.h>
>>
>> -#ifdef CONFIG_GRANT_TABLE
>> +#if __has_include("asm/grant_table.h")
>> #include <asm/grant_table.h>
>> #endif
>
> This change looks wrong (or otherwise is lacking justification): With
> GRANT_TABLE=n
> the arch header isn't supposed to be included.
>
> Jan
It's not equivalent to the previous code; but that's a feature, not a bug.
Not including the header with GRANT_TABLE=n was the best we could with
the older toolchains in order to not try to include a header that might not
exist. The high number of sequential inclusions of xen/grant_table.h and
asm/grant_table.h seem to attest to that.
I can ammend the commit message to be clearer, but IMO this is what it was
always
meant to be. I can replace the current commit message with:
"The previous toolchain base version didn't provide __has_include(), which
allows conditional inclusion based on a header's existence. Lacking that
feature the inclusion was guarded by the GRANT_TABLE option being present
but even then sometimes the arch-specific header is required even when
the option is not selected. This causes inclusion sites to needlessly
include both asm/grant_table.h and xen/grant_table.h.
Using __has_include() removes this requirement at inclusion sites."
Thoughts?
Cheers,
Alejandro
