Re: [Xen-devel] [Qemu-devel] [PATCH for-3.2 v3 02/14] qom: make interface types abstract
On Wed, 7 Nov 2018 16:36:40 +0400
Marc-André Lureau wrote:
> Interfaces don't have instance, let's make the interface type really
> abstract to avoid confusion.
>
> Signed-off-by: Marc-André Lureau
Reviewed-by: Igor Mammedov
> ---
> include/hw/acpi/acpi_dev_interface.h | 6 +-
> include/hw/arm/linux-boot-if.h | 5 +
> include/hw/fw-path-provider.h| 4 +---
> include/hw/hotplug.h | 6 +-
> include/hw/intc/intc.h | 4 +---
> include/hw/ipmi/ipmi.h | 4 +---
> include/hw/isa/isa.h | 4
> include/hw/mem/memory-device.h | 4 +---
> include/hw/nmi.h | 4 +---
> include/hw/stream.h | 4 +---
> include/hw/timer/m48t59.h| 4 +---
> include/qom/object_interfaces.h | 6 +-
> include/sysemu/tpm.h | 4 +---
> target/arm/idau.h| 4 +---
> tests/check-qom-interface.c | 4 +---
> 15 files changed, 14 insertions(+), 53 deletions(-)
>
> diff --git a/include/hw/acpi/acpi_dev_interface.h
> b/include/hw/acpi/acpi_dev_interface.h
> index dabf4c4fc9..43ff119179 100644
> --- a/include/hw/acpi/acpi_dev_interface.h
> +++ b/include/hw/acpi/acpi_dev_interface.h
> @@ -25,11 +25,7 @@ typedef enum {
> INTERFACE_CHECK(AcpiDeviceIf, (obj), \
> TYPE_ACPI_DEVICE_IF)
>
> -
> -typedef struct AcpiDeviceIf {
> -/* */
> -Object Parent;
> -} AcpiDeviceIf;
> +typedef struct AcpiDeviceIf AcpiDeviceIf;
>
> void acpi_send_event(DeviceState *dev, AcpiEventStatusBits event);
>
> diff --git a/include/hw/arm/linux-boot-if.h b/include/hw/arm/linux-boot-if.h
> index aba4479a14..7bbdfd1cc6 100644
> --- a/include/hw/arm/linux-boot-if.h
> +++ b/include/hw/arm/linux-boot-if.h
> @@ -16,10 +16,7 @@
> #define ARM_LINUX_BOOT_IF(obj) \
> INTERFACE_CHECK(ARMLinuxBootIf, (obj), TYPE_ARM_LINUX_BOOT_IF)
>
> -typedef struct ARMLinuxBootIf {
> -/*< private >*/
> -Object parent_obj;
> -} ARMLinuxBootIf;
> +typedef struct ARMLinuxBootIf ARMLinuxBootIf;
>
> typedef struct ARMLinuxBootIfClass {
> /*< private >*/
> diff --git a/include/hw/fw-path-provider.h b/include/hw/fw-path-provider.h
> index 050cb05d92..5df893a3d8 100644
> --- a/include/hw/fw-path-provider.h
> +++ b/include/hw/fw-path-provider.h
> @@ -30,9 +30,7 @@
> #define FW_PATH_PROVIDER(obj) \
> INTERFACE_CHECK(FWPathProvider, (obj), TYPE_FW_PATH_PROVIDER)
>
> -typedef struct FWPathProvider {
> -Object parent_obj;
> -} FWPathProvider;
> +typedef struct FWPathProvider FWPathProvider;
>
> typedef struct FWPathProviderClass {
> InterfaceClass parent_class;
> diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h
> index 1a0516a479..6321e292fd 100644
> --- a/include/hw/hotplug.h
> +++ b/include/hw/hotplug.h
> @@ -23,11 +23,7 @@
> #define HOTPLUG_HANDLER(obj) \
> INTERFACE_CHECK(HotplugHandler, (obj), TYPE_HOTPLUG_HANDLER)
>
> -
> -typedef struct HotplugHandler {
> -/* */
> -Object Parent;
> -} HotplugHandler;
> +typedef struct HotplugHandler HotplugHandler;
>
> /**
> * hotplug_fn:
> diff --git a/include/hw/intc/intc.h b/include/hw/intc/intc.h
> index 27d9828943..fb3e8e621f 100644
> --- a/include/hw/intc/intc.h
> +++ b/include/hw/intc/intc.h
> @@ -15,9 +15,7 @@
> INTERFACE_CHECK(InterruptStatsProvider, (obj), \
> TYPE_INTERRUPT_STATS_PROVIDER)
>
> -typedef struct InterruptStatsProvider {
> -Object parent;
> -} InterruptStatsProvider;
> +typedef struct InterruptStatsProvider InterruptStatsProvider;
>
> typedef struct InterruptStatsProviderClass {
> InterfaceClass parent;
> diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h
> index 0affe5a4d8..99661d2bf0 100644
> --- a/include/hw/ipmi/ipmi.h
> +++ b/include/hw/ipmi/ipmi.h
> @@ -114,9 +114,7 @@ uint32_t ipmi_next_uuid(void);
> #define IPMI_INTERFACE_GET_CLASS(class) \
> OBJECT_GET_CLASS(IPMIInterfaceClass, (class), TYPE_IPMI_INTERFACE)
>
> -typedef struct IPMIInterface {
> -Object parent;
> -} IPMIInterface;
> +typedef struct IPMIInterface IPMIInterface;
>
> typedef struct IPMIInterfaceClass {
> InterfaceClass parent;
> diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
> index b9dbab24b4..e62ac91c19 100644
> --- a/include/hw/isa/isa.h
> +++ b/include/hw/isa/isa.h
> @@ -43,10 +43,6 @@ static inline uint16_t applesmc_port(void)
> #define ISADMA(obj) \
> INTERFACE_CHECK(IsaDma, (obj), TYPE_ISADMA)
>
> -struct IsaDma {
> -Object parent;
> -};
> -
> typedef enum {
> ISADMA_TRANSFER_VERIFY,
> ISADMA_TRANSFER_READ,
> diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h
> index e904e194d5..0293a96abb 100644
> --- a/include/hw/mem/memory-device.h
> +++ b/include/hw/mem/memory-device.h
> @@ -25,9 +25,7 @@
> #define MEMORY_DEVICE(obj) \
> INTERFACE_CHECK(MemoryDeviceState, (obj), TYPE_MEMORY_DEVICE)
>
> -typedef struct MemoryDeviceSta
Re: [Xen-devel] [Qemu-devel] [PATCH for-3.2 v3 02/14] qom: make interface types abstract
On Tue, 20 Nov 2018 19:54:23 +0100
Laszlo Ersek wrote:
> On 11/20/18 17:33, Igor Mammedov wrote:
> > On Wed, 7 Nov 2018 16:36:40 +0400
> > Marc-André Lureau wrote:
> >
> >> Interfaces don't have instance, let's make the interface type really
> >> abstract to avoid confusion.
> >>
> >> Signed-off-by: Marc-André Lureau
> >> ---
> >> include/hw/acpi/acpi_dev_interface.h | 6 +-
> >> include/hw/arm/linux-boot-if.h | 5 +
> >> include/hw/fw-path-provider.h| 4 +---
> >> include/hw/hotplug.h | 6 +-
> >> include/hw/intc/intc.h | 4 +---
> >> include/hw/ipmi/ipmi.h | 4 +---
> >> include/hw/isa/isa.h | 4
> >> include/hw/mem/memory-device.h | 4 +---
> >> include/hw/nmi.h | 4 +---
> >> include/hw/stream.h | 4 +---
> >> include/hw/timer/m48t59.h| 4 +---
> >> include/qom/object_interfaces.h | 6 +-
> >> include/sysemu/tpm.h | 4 +---
> >> target/arm/idau.h| 4 +---
> >> tests/check-qom-interface.c | 4 +---
> >> 15 files changed, 14 insertions(+), 53 deletions(-)
> >>
> >> diff --git a/include/hw/acpi/acpi_dev_interface.h
> >> b/include/hw/acpi/acpi_dev_interface.h
> >> index dabf4c4fc9..43ff119179 100644
> >> --- a/include/hw/acpi/acpi_dev_interface.h
> >> +++ b/include/hw/acpi/acpi_dev_interface.h
> >> @@ -25,11 +25,7 @@ typedef enum {
> >> INTERFACE_CHECK(AcpiDeviceIf, (obj), \
> >> TYPE_ACPI_DEVICE_IF)
> >>
> >> -
> >> -typedef struct AcpiDeviceIf {
> >> -/* */
> >> -Object Parent;
> >> -} AcpiDeviceIf;
> >> +typedef struct AcpiDeviceIf AcpiDeviceIf;
> >>
> >> void acpi_send_event(DeviceState *dev, AcpiEventStatusBits event);
> >>
> >> diff --git a/include/hw/arm/linux-boot-if.h
> >> b/include/hw/arm/linux-boot-if.h
> >> index aba4479a14..7bbdfd1cc6 100644
> >> --- a/include/hw/arm/linux-boot-if.h
> >> +++ b/include/hw/arm/linux-boot-if.h
> >> @@ -16,10 +16,7 @@
> >> #define ARM_LINUX_BOOT_IF(obj) \
> >> INTERFACE_CHECK(ARMLinuxBootIf, (obj), TYPE_ARM_LINUX_BOOT_IF)
> >>
> >> -typedef struct ARMLinuxBootIf {
> >> -/*< private >*/
> >> -Object parent_obj;
> >> -} ARMLinuxBootIf;
> >> +typedef struct ARMLinuxBootIf ARMLinuxBootIf;
> > I like how it makes interface truly opaque and removes the need for
> > structure declaration but:
> >
> > 1: I'm not sure if it's acceptable thing to do from language point of view
> >
>
> Yeah, it's fine. If you have just
>
> struct ARMLinuxBootIf;
>
> (and, optionally, a typedef to it,) then this type is called an
> "incomplete type" (for translation units that don't see the actual type
> definition). You can't apply the "sizeof" operator to it, you can't put
> it in other structs and arrays etc. I'm too lazy to look up the exact
> details in the C standard now. :) But, importantly,
> "pointer-to-ARMLinuxBootIf" is a complete type, and you can do all the
> usual things with that. (Define variables of that pointer type, embed
> them in other structures, use it as an array element type, pass them to
> functions, and so on.)
Thanks Laszlo, that's the answer I was looking for.
> Thanks
> Laszlo
>
> > 2: For a reader not aware of a trick, it's sort of confusing to have
> > forward declaration but without structure itself. So if #1 is acceptable we
> > probably should document interface trick in object.h
> >
> > [...]
> >
>
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH for-3.2 v3 02/14] qom: make interface types abstract
On 11/20/18 17:33, Igor Mammedov wrote:
> On Wed, 7 Nov 2018 16:36:40 +0400
> Marc-André Lureau wrote:
>
>> Interfaces don't have instance, let's make the interface type really
>> abstract to avoid confusion.
>>
>> Signed-off-by: Marc-André Lureau
>> ---
>> include/hw/acpi/acpi_dev_interface.h | 6 +-
>> include/hw/arm/linux-boot-if.h | 5 +
>> include/hw/fw-path-provider.h| 4 +---
>> include/hw/hotplug.h | 6 +-
>> include/hw/intc/intc.h | 4 +---
>> include/hw/ipmi/ipmi.h | 4 +---
>> include/hw/isa/isa.h | 4
>> include/hw/mem/memory-device.h | 4 +---
>> include/hw/nmi.h | 4 +---
>> include/hw/stream.h | 4 +---
>> include/hw/timer/m48t59.h| 4 +---
>> include/qom/object_interfaces.h | 6 +-
>> include/sysemu/tpm.h | 4 +---
>> target/arm/idau.h| 4 +---
>> tests/check-qom-interface.c | 4 +---
>> 15 files changed, 14 insertions(+), 53 deletions(-)
>>
>> diff --git a/include/hw/acpi/acpi_dev_interface.h
>> b/include/hw/acpi/acpi_dev_interface.h
>> index dabf4c4fc9..43ff119179 100644
>> --- a/include/hw/acpi/acpi_dev_interface.h
>> +++ b/include/hw/acpi/acpi_dev_interface.h
>> @@ -25,11 +25,7 @@ typedef enum {
>> INTERFACE_CHECK(AcpiDeviceIf, (obj), \
>> TYPE_ACPI_DEVICE_IF)
>>
>> -
>> -typedef struct AcpiDeviceIf {
>> -/* */
>> -Object Parent;
>> -} AcpiDeviceIf;
>> +typedef struct AcpiDeviceIf AcpiDeviceIf;
>>
>> void acpi_send_event(DeviceState *dev, AcpiEventStatusBits event);
>>
>> diff --git a/include/hw/arm/linux-boot-if.h b/include/hw/arm/linux-boot-if.h
>> index aba4479a14..7bbdfd1cc6 100644
>> --- a/include/hw/arm/linux-boot-if.h
>> +++ b/include/hw/arm/linux-boot-if.h
>> @@ -16,10 +16,7 @@
>> #define ARM_LINUX_BOOT_IF(obj) \
>> INTERFACE_CHECK(ARMLinuxBootIf, (obj), TYPE_ARM_LINUX_BOOT_IF)
>>
>> -typedef struct ARMLinuxBootIf {
>> -/*< private >*/
>> -Object parent_obj;
>> -} ARMLinuxBootIf;
>> +typedef struct ARMLinuxBootIf ARMLinuxBootIf;
> I like how it makes interface truly opaque and removes the need for
> structure declaration but:
>
> 1: I'm not sure if it's acceptable thing to do from language point of view
Yeah, it's fine. If you have just
struct ARMLinuxBootIf;
(and, optionally, a typedef to it,) then this type is called an
"incomplete type" (for translation units that don't see the actual type
definition). You can't apply the "sizeof" operator to it, you can't put
it in other structs and arrays etc. I'm too lazy to look up the exact
details in the C standard now. :) But, importantly,
"pointer-to-ARMLinuxBootIf" is a complete type, and you can do all the
usual things with that. (Define variables of that pointer type, embed
them in other structures, use it as an array element type, pass them to
functions, and so on.)
Thanks
Laszlo
> 2: For a reader not aware of a trick, it's sort of confusing to have forward
> declaration but without structure itself. So if #1 is acceptable we probably
> should document interface trick in object.h
>
> [...]
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH for-3.2 v3 02/14] qom: make interface types abstract
On Tue, Nov 20, 2018 at 05:33:24PM +0100, Igor Mammedov wrote:
> On Wed, 7 Nov 2018 16:36:40 +0400
> Marc-André Lureau wrote:
>
> > Interfaces don't have instance, let's make the interface type really
> > abstract to avoid confusion.
> >
> > Signed-off-by: Marc-André Lureau
> > ---
> > include/hw/acpi/acpi_dev_interface.h | 6 +-
> > include/hw/arm/linux-boot-if.h | 5 +
> > include/hw/fw-path-provider.h| 4 +---
> > include/hw/hotplug.h | 6 +-
> > include/hw/intc/intc.h | 4 +---
> > include/hw/ipmi/ipmi.h | 4 +---
> > include/hw/isa/isa.h | 4
> > include/hw/mem/memory-device.h | 4 +---
> > include/hw/nmi.h | 4 +---
> > include/hw/stream.h | 4 +---
> > include/hw/timer/m48t59.h| 4 +---
> > include/qom/object_interfaces.h | 6 +-
> > include/sysemu/tpm.h | 4 +---
> > target/arm/idau.h| 4 +---
> > tests/check-qom-interface.c | 4 +---
> > 15 files changed, 14 insertions(+), 53 deletions(-)
> >
> > diff --git a/include/hw/acpi/acpi_dev_interface.h
> > b/include/hw/acpi/acpi_dev_interface.h
> > index dabf4c4fc9..43ff119179 100644
> > --- a/include/hw/acpi/acpi_dev_interface.h
> > +++ b/include/hw/acpi/acpi_dev_interface.h
> > @@ -25,11 +25,7 @@ typedef enum {
> > INTERFACE_CHECK(AcpiDeviceIf, (obj), \
> > TYPE_ACPI_DEVICE_IF)
> >
> > -
> > -typedef struct AcpiDeviceIf {
> > -/* */
> > -Object Parent;
> > -} AcpiDeviceIf;
> > +typedef struct AcpiDeviceIf AcpiDeviceIf;
> >
> > void acpi_send_event(DeviceState *dev, AcpiEventStatusBits event);
> >
> > diff --git a/include/hw/arm/linux-boot-if.h b/include/hw/arm/linux-boot-if.h
> > index aba4479a14..7bbdfd1cc6 100644
> > --- a/include/hw/arm/linux-boot-if.h
> > +++ b/include/hw/arm/linux-boot-if.h
> > @@ -16,10 +16,7 @@
> > #define ARM_LINUX_BOOT_IF(obj) \
> > INTERFACE_CHECK(ARMLinuxBootIf, (obj), TYPE_ARM_LINUX_BOOT_IF)
> >
> > -typedef struct ARMLinuxBootIf {
> > -/*< private >*/
> > -Object parent_obj;
> > -} ARMLinuxBootIf;
> > +typedef struct ARMLinuxBootIf ARMLinuxBootIf;
> I like how it makes interface truly opaque and removes the need for
> structure declaration but:
>
> 1: I'm not sure if it's acceptable thing to do from language point of view
>
> 2: For a reader not aware of a trick, it's sort of confusing to have forward
> declaration but without structure itself. So if #1 is acceptable we probably
> should document interface trick in object.h
I'm not sure what's the question here. We do this all the time
on typedefs.h, don't we?
--
Eduardo
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH for-3.2 v3 02/14] qom: make interface types abstract
On Wed, 7 Nov 2018 16:36:40 +0400
Marc-André Lureau wrote:
> Interfaces don't have instance, let's make the interface type really
> abstract to avoid confusion.
>
> Signed-off-by: Marc-André Lureau
> ---
> include/hw/acpi/acpi_dev_interface.h | 6 +-
> include/hw/arm/linux-boot-if.h | 5 +
> include/hw/fw-path-provider.h| 4 +---
> include/hw/hotplug.h | 6 +-
> include/hw/intc/intc.h | 4 +---
> include/hw/ipmi/ipmi.h | 4 +---
> include/hw/isa/isa.h | 4
> include/hw/mem/memory-device.h | 4 +---
> include/hw/nmi.h | 4 +---
> include/hw/stream.h | 4 +---
> include/hw/timer/m48t59.h| 4 +---
> include/qom/object_interfaces.h | 6 +-
> include/sysemu/tpm.h | 4 +---
> target/arm/idau.h| 4 +---
> tests/check-qom-interface.c | 4 +---
> 15 files changed, 14 insertions(+), 53 deletions(-)
>
> diff --git a/include/hw/acpi/acpi_dev_interface.h
> b/include/hw/acpi/acpi_dev_interface.h
> index dabf4c4fc9..43ff119179 100644
> --- a/include/hw/acpi/acpi_dev_interface.h
> +++ b/include/hw/acpi/acpi_dev_interface.h
> @@ -25,11 +25,7 @@ typedef enum {
> INTERFACE_CHECK(AcpiDeviceIf, (obj), \
> TYPE_ACPI_DEVICE_IF)
>
> -
> -typedef struct AcpiDeviceIf {
> -/* */
> -Object Parent;
> -} AcpiDeviceIf;
> +typedef struct AcpiDeviceIf AcpiDeviceIf;
>
> void acpi_send_event(DeviceState *dev, AcpiEventStatusBits event);
>
> diff --git a/include/hw/arm/linux-boot-if.h b/include/hw/arm/linux-boot-if.h
> index aba4479a14..7bbdfd1cc6 100644
> --- a/include/hw/arm/linux-boot-if.h
> +++ b/include/hw/arm/linux-boot-if.h
> @@ -16,10 +16,7 @@
> #define ARM_LINUX_BOOT_IF(obj) \
> INTERFACE_CHECK(ARMLinuxBootIf, (obj), TYPE_ARM_LINUX_BOOT_IF)
>
> -typedef struct ARMLinuxBootIf {
> -/*< private >*/
> -Object parent_obj;
> -} ARMLinuxBootIf;
> +typedef struct ARMLinuxBootIf ARMLinuxBootIf;
I like how it makes interface truly opaque and removes the need for
structure declaration but:
1: I'm not sure if it's acceptable thing to do from language point of view
2: For a reader not aware of a trick, it's sort of confusing to have forward
declaration but without structure itself. So if #1 is acceptable we probably
should document interface trick in object.h
[...]
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
