Re: [Xen-devel] [PATCH v6 10/12] xen/arm: move arch specific grant table bits into grant_table.c

[Juergen Gross <jgross@xxxxxxxx>]

On 15/09/17 10:49, Julien Grall wrote:
> Hi Juergen,
> 
> On 15/09/2017 08:22, Juergen Gross wrote:
>> On 14/09/17 19:31, Julien Grall wrote:
>>> Hi Juergen,
>>>
>>> On 13/09/17 16:46, Juergen Gross wrote:
>>>> diff --git a/xen/include/xen/grant_table.h
>>>> b/xen/include/xen/grant_table.h
>>>> index df11b31264..f3f2fb9ebc 100644
>>>> --- a/xen/include/xen/grant_table.h
>>>> +++ b/xen/include/xen/grant_table.h
>>>> @@ -27,7 +27,8 @@
>>>>   #include <xen/rwlock.h>
>>>>   #include <public/grant_table.h>
>>>>   #include <asm/page.h>
>>>> -#include <asm/grant_table.h>
>>>
>>> This change looks a bit strange to me. This is the only place where
>>> asm/grant_table.h is pulled. Because you remove it, it now means that
>>> the prototype will not be defined first and may result to mismatch in
>>> the future.
>>>
>>> Ideally we should enforce, although it would require some work as we
>>> didn't really follow that rule in a few places.
>>
>> Aah, I missed the prototypes which relate to functions outside of
>> grant_table.c. Strange that the compiler didn't barf at me.
> 
> It is because we don't set -Wmissing-prototypes. This would break
> compilation on Xen because of quite a few missing prototypes within the
> source code.

Uuh, I wasn't aware of this!

>> All those functions seem to be rather small, so I can add them as inline
>> to asm-arm/grant_table.h, as they are used only by grant_table.c.
> 
> I think you would still need to include asm/grant_table.h for x86 to get
> the prototype defined for create_grant_p2m_mapping & co.
>
> To be honest, I am not sure to fully understand the rationale to not
> include asm/grant_table.h in grant_table.h.

The idea was that asm/grant_table.h only contains abstractions needed in
grant_table.c. Why include it in a Xen-global header when only one
source needs its contents? Additionally when I started this patch I
tried to use inline functions instead of #defines in asm/grant-table.h
leading to compile errors when struct grant_table was dereferenced.

But with the missing prototypes problem I think it might really be
better to keep the #include in grant_table.h.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel