1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v4 00/29] Tame a few "touch this, recompile the world" headers
  4. View patch

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

Markus Armbruster posted 29 patches 6 years, 6 months ago
Maintainers: Gonglei <arei.gonglei@huawei.com>, "Cédric Le Goater" <clg@kaod.org>, Guan Xuetao <gxt@mprc.pku.edu.cn>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Wen Congyang <wencongyang2@huawei.com>, Andrey Smirnov <andrew.smirnov@gmail.com>, Peter Lieven <pl@kamp.de>, Kevin Wolf <kwolf@redhat.com>, Michael Walle <michael@walle.cc>, Riku Voipio <riku.voipio@iki.fi>, Leif Lindholm <leif.lindholm@linaro.org>, Jiri Slaby <jslaby@suse.cz>, Jan Kiszka <jan.kiszka@web.de>, Hannes Reinecke <hare@suse.com>, Richard Henderson <rth@twiddle.net>, Markus Armbruster <armbru@redhat.com>, Chris Wulff <crwulff@gmail.com>, Christian Borntraeger <borntraeger@de.ibm.com>, Stefano Stabellini <sstabellini@kernel.org>, Stafford Horne <shorne@gmail.com>, Corey Minyard <cminyard@mvista.com>, Max Reitz <mreitz@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Artyom Tarasenko <atar4qemu@gmail.com>, Dmitry Fleytman <dmitry.fleytman@gmail.com>, Palmer Dabbelt <palmer@sifive.com>, David Gibson <david@gibson.dropbear.id.au>, Radoslaw Biernacki <radoslaw.biernacki@linaro.org>, Michael Roth <mdroth@linux.vnet.ibm.com>, Jiri Pirko <jiri@resnulli.us>, Eduardo Habkost <ehabkost@redhat.com>, Anthony Perard <anthony.perard@citrix.com>, "Alex Bennée" <alex.bennee@linaro.org>, Jean-Christophe Dubois <jcd@tribudubois.net>, Paul Burton <pburton@wavecomp.com>, Andrew Jeffery <andrew@aj.id.au>, Aleksandar Markovic <amarkovic@wavecomp.com>, Fam Zheng <fam@euphon.net>, Amit Shah <amit@kernel.org>, Sagar Karandikar <sagark@eecs.berkeley.edu>, Aleksandar Rikalo <arikalo@wavecomp.com>, Laurent Vivier <lvivier@redhat.com>, Alistair Francis <alistair@alistair23.me>, Ben Warren <ben@skyportsystems.com>, Joel Stanley <joel@jms.id.au>, Paul Durrant <paul.durrant@citrix.com>, zhanghailiang <zhang.zhanghailiang@huawei.com>, Juan Quintela <quintela@redhat.com>, Corey Minyard <minyard@acm.org>, Marek Vasut <marex@denx.de>, Igor Mammedov <imammedo@redhat.com>, Jia Liu <proljc@gmail.com>, Eric Farman <farman@linux.ibm.com>, Marcelo Tosatti <mtosatti@redhat.com>, Fabien Chouteau <chouteau@adacore.com>, Greg Kurz <groug@kaod.org>, Laurent Vivier <laurent@vivier.eu>, Anthony Green <green@moxielogic.com>, Xie Changlong <xiechanglong.d@gmail.com>, Liu Yuan <namei.unix@gmail.com>, Pierre Morel <pmorel@linux.ibm.com>, John Snow <jsnow@redhat.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Halil Pasic <pasic@linux.ibm.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Shannon Zhao <shannon.zhaosl@gmail.com>, Andrzej Zaborowski <balrogg@gmail.com>, David Hildenbrand <david@redhat.com>, Yuval Shaia <yuval.shaia@oracle.com>, Tony Krowiak <akrowiak@linux.ibm.com>, Stefan Hajnoczi <stefanha@redhat.com>, Subbaraya Sundeep <sundeep.lkml@gmail.com>, Antony Pavlov <antonynpavlov@gmail.com>, Collin Walling <walling@linux.ibm.com>, Eric Auger <eric.auger@redhat.com>, Beniamino Galvani <b.galvani@gmail.com>, Helge Deller <deller@gmx.de>, KONRAD Frederic <frederic.konrad@adacore.com>, Stefan Berger <stefanb@linux.ibm.com>, Stefan Weil <sw@weilnetz.de>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Eric Blake <eblake@redhat.com>, Vincenzo Maffione <v.maffione@gmail.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Cornelia Huck <cohuck@redhat.com>, Thomas Huth <thuth@redhat.com>, Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Jason Wang <jasowang@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Alberto Garcia <berto@igalia.com>, Gerd Hoffmann <kraxel@redhat.com>, Igor Mitsyanko <i.mitsyanko@gmail.com>, Andrew Baumann <Andrew.Baumann@microsoft.com>, Laszlo Ersek <lersek@redhat.com>, James Hogan <jhogan@kernel.org>, Rob Herring <robh@kernel.org>, Alistair Francis <Alistair.Francis@wdc.com>, Paolo Bonzini <pbonzini@redhat.com>, Keith Busch <keith.busch@intel.com>, Luigi Rizzo <rizzo@iet.unipi.it>, Magnus Damm <magnus.damm@gmail.com>, Giuseppe Lettieri <g.lettieri@iet.unipi.it>, "Michael S. Tsirkin" <mst@redhat.com>, Peter Chubb <peter.chubb@nicta.com.au>, Alex Williamson <alex.williamson@redhat.com>, Thomas Huth <huth@tuxfamily.org>, Aurelien Jarno <aurelien@aurel32.net>, "Hervé Poussineau" <hpoussin@reactos.org>, Xiao Guangrong <xiaoguangrong.eric@gmail.com>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Max Filippov <jcmvbkbc@gmail.com>, BALATON Zoltan <balaton@eik.bme.hu>, Samuel Thibault <samuel.thibault@ens-lyon.org>
[Qemu-devel] [PATCH v4 11/29] typedefs: Separate incomplete types and function types
Posted by Markus Armbruster 6 years, 6 months ago 
While there, drop the obsolete file comment.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 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
+ * Please keep this list in case-insensitive alphabetical order.
+ */
 typedef struct AdapterInfo AdapterInfo;
 typedef struct AddressSpace AddressSpace;
 typedef struct AioContext AioContext;
@@ -101,6 +101,10 @@ typedef struct SHPCDevice SHPCDevice;
 typedef struct SSIBus SSIBus;
 typedef struct VirtIODevice VirtIODevice;
 typedef struct Visitor Visitor;
+
+/*
+ * Function types
+ */
 typedef void SaveStateHandler(QEMUFile *f, void *opaque);
 typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);
 
-- 
2.21.0
Re: [Qemu-devel] [PATCH v4 11/29] typedefs: Separate incomplete types and function types
Posted by Alex Bennée 6 years, 6 months ago 
Markus Armbruster <armbru@redhat.com> writes:

> While there, drop the obsolete file comment.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  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."?

Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


> + * Please keep this list in case-insensitive alphabetical order.
> + */
>  typedef struct AdapterInfo AdapterInfo;
>  typedef struct AddressSpace AddressSpace;
>  typedef struct AioContext AioContext;
> @@ -101,6 +101,10 @@ typedef struct SHPCDevice SHPCDevice;
>  typedef struct SSIBus SSIBus;
>  typedef struct VirtIODevice VirtIODevice;
>  typedef struct Visitor Visitor;
> +
> +/*
> + * Function types
> + */
>  typedef void SaveStateHandler(QEMUFile *f, void *opaque);
>  typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);


--
Alex Bennée
Re: [Qemu-devel] [PATCH v4 11/29] typedefs: Separate incomplete types and function types
Posted by Markus Armbruster 6 years, 6 months ago 
Alex Bennée <alex.bennee@linaro.org> writes:

> Markus Armbruster <armbru@redhat.com> writes:
>
>> While there, drop the obsolete file comment.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  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.

> Otherwise:
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

Thanks!

>> + * Please keep this list in case-insensitive alphabetical order.
>> + */
>>  typedef struct AdapterInfo AdapterInfo;
>>  typedef struct AddressSpace AddressSpace;
>>  typedef struct AioContext AioContext;
>> @@ -101,6 +101,10 @@ typedef struct SHPCDevice SHPCDevice;
>>  typedef struct SSIBus SSIBus;
>>  typedef struct VirtIODevice VirtIODevice;
>>  typedef struct Visitor Visitor;
>> +
>> +/*
>> + * Function types
>> + */
>>  typedef void SaveStateHandler(QEMUFile *f, void *opaque);
>>  typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);
>
>
> --
> Alex Bennée
Re: [Qemu-devel] [PATCH v4 11/29] typedefs: Separate incomplete types and function types
Posted by Markus Armbruster 6 years, 6 months ago 
Markus Armbruster <armbru@redhat.com> writes:

> Alex Bennée <alex.bennee@linaro.org> writes:
>
>> Markus Armbruster <armbru@redhat.com> writes:
>>
>>> While there, drop the obsolete file comment.
>>>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>>  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?

[...]
Re: [Qemu-devel] [PATCH v4 11/29] typedefs: Separate incomplete types and function types
Posted by Alex Bennée 6 years, 6 months ago 
Markus Armbruster <armbru@redhat.com> writes:

> Markus Armbruster <armbru@redhat.com> writes:
>
>> Alex Bennée <alex.bennee@linaro.org> writes:
>>
>>> Markus Armbruster <armbru@redhat.com> writes:
>>>
>>>> While there, drop the obsolete file comment.
>>>>
>>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> ---
>>>>  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?

A definite improvement on what is currently there ;-)

>
> [...]


--
Alex Bennée
Re: [Qemu-devel] [PATCH v4 11/29] typedefs: Separate incomplete types and function types
Posted by Markus Armbruster 6 years, 6 months ago 
Alex Bennée <alex.bennee@linaro.org> writes:

> Markus Armbruster <armbru@redhat.com> writes:
>
>> Markus Armbruster <armbru@redhat.com> writes:
>>
>>> Alex Bennée <alex.bennee@linaro.org> writes:
>>>
>>>> Markus Armbruster <armbru@redhat.com> writes:
>>>>
>>>>> While there, drop the obsolete file comment.
>>>>>
>>>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>> ---
>>>>>  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?
>
> A definite improvement on what is currently there ;-)

Amending my commit.  Thanks!

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.