Re: [Qemu-devel] [PATCH v2 26/29] Clean up inclusion of sysemu/sysemu.h

["Alex Bennée" <alex.bennee@linaro.org>]

Markus Armbruster <armbru@redhat.com> writes:

> Alex Bennée <alex.bennee@linaro.org> writes:
>
>> Markus Armbruster <armbru@redhat.com> writes:
>>
>>> In my "build everything" tree, changing sysemu/sysemu.h triggers a
>>> recompile of some 5400 out of 6600 objects (not counting tests and
>>> objects that don't depend on qemu/osdep.h).
>>>
>>> 119 of 380 #include directives are actually superfluous.  Delete them.
>>> Downgrade two more to qapi/qapi-types-run-state.h, and move one from
>>> char/serial.h to char/serial.c.
>>>
>>> This doesn't reduce actual use much, as it's still included into
>>> widely included headers.  The next commit will tackle that.
>>>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>> <snip>
>>>  hw/semihosting/config.c             | 1 +
>> <snip>
>>>  stubs/semihost.c                    | 1 +
>> <snip>
>>> diff --git a/hw/semihosting/config.c b/hw/semihosting/config.c
>>> index 2a8e7e1045..9807f10cb0 100644
>>> --- a/hw/semihosting/config.c
>>> +++ b/hw/semihosting/config.c
>>> @@ -24,6 +24,7 @@
>>>  #include "qemu/error-report.h"
>>>  #include "hw/semihosting/semihost.h"
>>>  #include "chardev/char.h"
>>> +#include "sysemu/sysemu.h"
>>>
>>>  QemuOptsList qemu_semihosting_config_opts = {
>>>      .name = "semihosting-config",
>>> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
>>> index b8332150f1..9f3cff5fb6 100644
>> <snip>
>>>
>>> diff --git a/stubs/semihost.c b/stubs/semihost.c
>>> index 4d5b3c0653..f90589259c 100644
>>> --- a/stubs/semihost.c
>>> +++ b/stubs/semihost.c
>>> @@ -12,6 +12,7 @@
>>>  #include "qemu/option.h"
>>>  #include "qemu/error-report.h"
>>>  #include "hw/semihosting/semihost.h"
>>> +#include "sysemu/sysemu.h"
>> <snip>
>>
>> These additions seem out of place. If I comment them out I can still
>> build fine
>
> sysemu/sysemu.h declares qemu_semihosting_config_opts,
> hw/semihosting/config.c and stubs/semihost.c define it.
>
> Gcc warns when you do that for functions (-Wmissing-declarations
> -Wmissing-prototypes), but not for variables.  I like to include the
> header anyway, to make sure the compiler checks the declaration is
> consistent with the definition.
>
>>            - I think the only place that needs them is vl.c so it has a
>> typedef for the semihosting configure options. Arguably the extern
>> declaration could be moved into semihostings own headers to avoid
>> polluting sysemu.h more than it needs to?
>
> I'm not sure I'm following you.
>
> What would you like me to move where?

extern QemuOptsList qemu_semihosting_config_opts;

from sysemu.h to semihosting.h - but given other options externs are
there maybe that is the best place for it.

--
Alex Bennée