Re: [Qemu-devel] [PATCH v2 01/29] include: Make headers more self-contained

[Markus Armbruster]

Alex Bennée <address@hidden> writes:

> Markus Armbruster <address@hidden> writes:
>
>> Alex Bennée <address@hidden> writes:
>>
>>> Markus Armbruster <address@hidden> writes:
>>>
>>>> Back in 2016, we discussed[1] rules for headers, and these were
>>>> generally liked:
>>>>
>>>> 1. Have a carefully curated header that's included everywhere first.  We
>>>>    got that already thanks to Peter: osdep.h.
>>>>
>>>> 2. Headers should normally include everything they need beyond osdep.h.
>>>>    If exceptions are needed for some reason, they must be documented in
>>>>    the header.  If all that's needed from a header is typedefs, put
>>>>    those into qemu/typedefs.h instead of including the header.
>>>>
>>>> 3. Cyclic inclusion is forbidden.
>>>>
>>>> This patch gets include/ closer to obeying 2.
>>>>
>>>> It's actually extracted from my "[RFC] Baby steps towards saner
>>>> headers" series[2], which demonstrates a possible path towards
>>>> checking 2 automatically.  It passes the RFC test there.
>>>>
>>>> [1] Message-ID: <address@hidden>
>>>>     https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg03345.html
>>>> [2] Message-Id: <address@hidden>
>>>>     https://lists.nongnu.org/archive/html/qemu-devel/2019-07/msg02715.html
>>>>
>>>> Signed-off-by: Markus Armbruster <address@hidden>
>>>> Reviewed-by: Alistair Francis <address@hidden>
[...]
>>>> diff --git a/include/fpu/softfloat-macros.h 
>>>> b/include/fpu/softfloat-macros.h
>>>> index c55aa6d174..be83a833ec 100644
>>>> --- a/include/fpu/softfloat-macros.h
>>>> +++ b/include/fpu/softfloat-macros.h
>>>> @@ -82,6 +82,8 @@ this code that are retained.
>>>>  #ifndef FPU_SOFTFLOAT_MACROS_H
>>>>  #define FPU_SOFTFLOAT_MACROS_H
>>>>
>>>> +#include "fpu/softfloat.h"
>>>> +
>>>
>>> What does softfloat-macros actually need from the core softfloat API?
>>> These are lower level functions used by softfloat itself (and m68k for
>>> it's own bit fiddling).
>>
>> I extracted this patch out of "[PATCH RFC v5 0/3] Baby steps towards
>> saner headers".  PATCH 1/3 creates make target "check-source", which is
>> what I used to find headers that aren't self-contained.  In this case:
>>
>>   CC      cris-softmmu/tests/headers-tgt/include/fpu/softfloat.o
>> In file included from tests/headers-tgt/include/fpu/softfloat-macros.c:21:
>> /work/armbru/qemu/include/fpu/softfloat-macros.h: In function 
>> ‘estimateDiv128To64’:
>> /work/armbru/qemu/include/fpu/softfloat-macros.h:623:27: error: implicit 
>> declaration of function ‘LIT64’ [-Werror=implicit-function-declaration]
>>   623 |     if ( b <= a0 ) return LIT64( 0xFFFFFFFFFFFFFFFF );
>
> The LIT64 definition should be moved to softfloat-types.h which is
> already included by softfloat.h unless we already have a QEMU expansion
> we should be using. The softfloat-macros.h can include softfloat-types.h
> as well and we should only include the full softfloat.h if they need it.
>
> Do you want me to spin up a patch?

Yes, please!

[...]