tidying up osdep.h

tidying up osdep.h

[Peter Maydell]

(cc'ing people related to the recent 'extern "C"' patches and also
randomly Markus as somebody who's had opinions on header cleanups
in the past...)

osdep.h as it stands today is a mix of two things:
 (1) it has the "must be included by everybody" items:
   (a) config-host.h, poison.h, compiler.h
   (b) things which must be done before any system header is included
       (like defining __STDC_CONSTANT_MACROS or WIN32_LEAN_AND_MEAN)
   (c) includes of system headers which we need to then fix up for
       portability issues (eg redefining assert on mingw, defining
       fallback versions of missing macros)
 (2) it has declarations for a library of QEMU functions, some of which
     typically wrap and abstract away OS specifics (like qemu_create(),
     qemu_unlink()), and some of which seem to have just been dumped
     in here for convenience (like qemu_hw_version())

Every file needs (1), which is why we mandate osdep.h as the first
include; most files don't need a lot of the things in (2) (for instance
qemu_hw_version() is used in just half a dozen .c files). Is it worth
trying to split some of the type (2) items out into their own header files?

I suspect that the advantages would be primarily just making osdep.h
a bit clearer to read and less of an "attractive nuisance" for new
additions; I imagine the bulk of the extra compilation time represented
by osdep.h is going to be because it pulls in dozens of system
headers, most of which are going to be required under heading (1).

thanks
-- PMM

Re: tidying up osdep.h

[Philippe Mathieu-Daudé]

On 4/14/21 9:17 PM, Peter Maydell wrote:
> (cc'ing people related to the recent 'extern "C"' patches and also
> randomly Markus as somebody who's had opinions on header cleanups
> in the past...)
> 
> osdep.h as it stands today is a mix of two things:
>  (1) it has the "must be included by everybody" items:
>    (a) config-host.h, poison.h, compiler.h
>    (b) things which must be done before any system header is included
>        (like defining __STDC_CONSTANT_MACROS or WIN32_LEAN_AND_MEAN)
>    (c) includes of system headers which we need to then fix up for
>        portability issues (eg redefining assert on mingw, defining
>        fallback versions of missing macros)
>  (2) it has declarations for a library of QEMU functions, some of which
>      typically wrap and abstract away OS specifics (like qemu_create(),
>      qemu_unlink()), and some of which seem to have just been dumped
>      in here for convenience (like qemu_hw_version())
> 
> Every file needs (1), which is why we mandate osdep.h as the first
> include; most files don't need a lot of the things in (2) (for instance
> qemu_hw_version() is used in just half a dozen .c files). Is it worth
> trying to split some of the type (2) items out into their own header files?
> 
> I suspect that the advantages would be primarily just making osdep.h
> a bit clearer to read and less of an "attractive nuisance" for new
> additions; I imagine the bulk of the extra compilation time represented
> by osdep.h is going to be because it pulls in dozens of system
> headers, most of which are going to be required under heading (1).

What about:

- extract qemu_hw_version() to "qemu/legacy_api.h"?

- extract (1) from osdep.h as osinc.h (because described as
  "OS includes and handling of OS dependencies")?
  Or KISS as "qemu-first-include.h"...

- osdep.h now contains the declarations for osdep.c

Regards,

Phil.

Re: tidying up osdep.h

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 4/14/21 9:17 PM, Peter Maydell wrote:
>> (cc'ing people related to the recent 'extern "C"' patches and also
>> randomly Markus as somebody who's had opinions on header cleanups
>> in the past...)
>> 
>> osdep.h as it stands today is a mix of two things:
>>  (1) it has the "must be included by everybody" items:
>>    (a) config-host.h, poison.h, compiler.h
>>    (b) things which must be done before any system header is included
>>        (like defining __STDC_CONSTANT_MACROS or WIN32_LEAN_AND_MEAN)
>>    (c) includes of system headers which we need to then fix up for
>>        portability issues (eg redefining assert on mingw, defining
>>        fallback versions of missing macros)
>>  (2) it has declarations for a library of QEMU functions, some of which
>>      typically wrap and abstract away OS specifics (like qemu_create(),
>>      qemu_unlink()), and some of which seem to have just been dumped
>>      in here for convenience (like qemu_hw_version())
>> 
>> Every file needs (1), which is why we mandate osdep.h as the first
>> include; most files don't need a lot of the things in (2) (for instance
>> qemu_hw_version() is used in just half a dozen .c files). Is it worth
>> trying to split some of the type (2) items out into their own header files?
>> 
>> I suspect that the advantages would be primarily just making osdep.h
>> a bit clearer to read and less of an "attractive nuisance" for new
>> additions; I imagine the bulk of the extra compilation time represented
>> by osdep.h is going to be because it pulls in dozens of system
>> headers, most of which are going to be required under heading (1).
>
> What about:
>
> - extract qemu_hw_version() to "qemu/legacy_api.h"?
>
> - extract (1) from osdep.h as osinc.h (because described as
>   "OS includes and handling of OS dependencies")?
>   Or KISS as "qemu-first-include.h"...

For what it's worth, the autoconf convention is to call it "config.h".

> - osdep.h now contains the declarations for osdep.c
>
> Regards,
>
> Phil.

Re: tidying up osdep.h

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> (cc'ing people related to the recent 'extern "C"' patches and also
> randomly Markus as somebody who's had opinions on header cleanups
> in the past...)
>
> osdep.h as it stands today is a mix of two things:
>  (1) it has the "must be included by everybody" items:
>    (a) config-host.h, poison.h, compiler.h
>    (b) things which must be done before any system header is included
>        (like defining __STDC_CONSTANT_MACROS or WIN32_LEAN_AND_MEAN)
>    (c) includes of system headers which we need to then fix up for
>        portability issues (eg redefining assert on mingw, defining
>        fallback versions of missing macros)
>  (2) it has declarations for a library of QEMU functions, some of which
>      typically wrap and abstract away OS specifics (like qemu_create(),
>      qemu_unlink()), and some of which seem to have just been dumped
>      in here for convenience (like qemu_hw_version())

It's such a convenient dumping ground :)

> Every file needs (1), which is why we mandate osdep.h as the first
> include; most files don't need a lot of the things in (2) (for instance
> qemu_hw_version() is used in just half a dozen .c files). Is it worth
> trying to split some of the type (2) items out into their own header files?
>
> I suspect that the advantages would be primarily just making osdep.h
> a bit clearer to read and less of an "attractive nuisance" for new
> additions; I imagine the bulk of the extra compilation time represented
> by osdep.h is going to be because it pulls in dozens of system
> headers, most of which are going to be required under heading (1).

I agree that keeping it focused on (1) would be cleaner, and that such a
cleanup probably won't speed up builds.  Regarding your question whether
the cleanup is worth the bother: I guess if somebody posts patches, it
is.