Re: [Qemu-devel] [PATCH v2 04/19] Include qapi/error.h exactly where needed

[Markus Armbruster]

Cc: Gerd for an idea; please skip to bottom of the message.

Eric Blake <address@hidden> writes:

> On 01/31/2018 08:48 AM, Markus Armbruster wrote:
>> This cleanup makes the number of objects depending on qapi/error.h
>> drop from 1910 (out of 4743) to 1612 in my "build everything" tree.
>> 
>> While there, separate #include from file comment with a blank line,
>> and drop a useless comment on why qemu/osdep.h is included first.
>> 
>> Signed-off-by: Markus Armbruster <address@hidden>
>> Reviewed-by: Eric Blake <address@hidden>
>> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
>> ---
>
>> +++ b/block/write-threshold.c
>> @@ -16,6 +16,7 @@
>>  #include "block/write-threshold.h"
>>  #include "qemu/notify.h"
>>  #include "qapi-event.h"
>> +#include "qapi/error.h"
>>  #include "qmp-commands.h"
>>  
>>  
>
> Worth squashing a double blank line while here?

Yes.  Same for the other too.

>> +++ b/hw/pci-bridge/i82801b11.c
>> @@ -44,7 +44,6 @@
>>  #include "qemu/osdep.h"
>>  #include "hw/pci/pci.h"
>>  #include "hw/i386/ich9.h"
>> -#include "qapi/error.h"
>>  
>>  
>>  
>> /*****************************************************************************/
>
> And here
>
>> +++ b/include/crypto/random.h
>> @@ -22,7 +22,6 @@
>>  #define QCRYPTO_RANDOM_H
>>  
>>  #include "qemu-common.h"
>> -#include "qapi/error.h"
>>  
>>  
>>  /**
>> diff --git a/include/crypto/xts.h b/include/crypto/xts.h
>> index da32ab82b6..719971afb7 100644
>> --- a/include/crypto/xts.h
>> +++ b/include/crypto/xts.h
>> @@ -27,7 +27,6 @@
>>  #define QCRYPTO_XTS_H
>>  
>>  #include "qemu-common.h"
>> -#include "qapi/error.h"
>>  
>>  
>>  #define XTS_BLOCK_SIZE 16
>
> And here
>
>> +++ b/include/ui/console.h
>> @@ -6,7 +6,9 @@
>>  #include "qapi/qmp/qdict.h"
>>  #include "qemu/notify.h"
>>  #include "qemu/error-report.h"
>> +#ifndef CONFIG_VNC
>>  #include "qapi/error.h"
>> +#endif
>
> Interesting conditional; it's because of the static inline fallbacks
> that are also conditional.  Makes sense.  An alternative would be having
> the fallbacks be in a .c rather than static inline in the .h, but it
> doesn't bother me enough to worry about it.

In my opinion, they should really, really be made a stub so we don't
have to include qapi/error.h in any configuration, but right now this
simple ifdeffery is all I can do.

> My R-b stands whether or not you make more whitespace tweaks.

Thanks!