Re: [Qemu-devel] [PATCH v4 11/29] typedefs: Separate incomplete types and function types

[Markus Armbruster]

Markus Armbruster <address@hidden> writes:

> Alex Bennée <address@hidden> writes:
>
>> Markus Armbruster <address@hidden> writes:
>>
>>> While there, drop the obsolete file comment.
>>>
>>> Signed-off-by: Markus Armbruster <address@hidden>
>>> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
>>> Tested-by: Philippe Mathieu-Daudé <address@hidden>
>>> ---
>>>  include/qemu/typedefs.h | 12 ++++++++----
>>>  1 file changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
>>> index fcdaae58c4..29346648d4 100644
>>> --- a/include/qemu/typedefs.h
>>> +++ b/include/qemu/typedefs.h
>>> @@ -1,10 +1,10 @@
>>>  #ifndef QEMU_TYPEDEFS_H
>>>  #define QEMU_TYPEDEFS_H
>>>
>>> -/* A load of opaque types so that device init declarations don't have to
>>> -   pull in all the real definitions.  */
>>> -
>>> -/* Please keep this list in case-insensitive alphabetical order */
>>> +/*
>>> + * Incomplete struct types
>>
>> Maybe expand this a little...
>>
>>   "Incomplete struct types for modules that don't need the complete
>>   definitions but still pass around typed variables."?
>
> If we explain proper use of qemu/typedefs.h in HACKING, as discussed in
> review of v1[*], we could point there.

Perhaps rewriting the obsolete file comment would be better.  Something
like this:

    /*
     * This header is for selectively avoiding #include just to get a
     * typedef name.
     *
     * Declaring a typedef name in its "obvious" place can result in
     * inclusion cycles, in particular for complete struct and union
     * types that need more types for their members.  It can also result
     * in headers pulling in many more headers, slowing down builds.
     *
     * You can break such cycles and unwanted dependencies by declaring
     * the typedef name here.
     *
     * For struct types used in only a few headers, judicious use of the
     * struct tag instead of the typedef name is commonly preferable.
     */

    /*
     * Incomplete struct types
     * Please keep this list in case-insensitive alphabetical order.
     */
    typedef struct AdapterInfo AdapterInfo;
    [...]

    /*
     * Pointer types
     * Such typedefs should be limited to cases where the typedef's users
     * are oblivious of its "pointer-ness".
     * Please keep this list in case-insensitive alphabetical order.
     */
    typedef struct IRQState *qemu_irq;

    /*
     * Function types
     */
    typedef void SaveStateHandler(QEMUFile *f, void *opaque);
    typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);
    typedef void (*qemu_irq_handler)(void *opaque, int n, int level);

What do you think?

[...]