Re: [Qemu-arm] [Qemu-devel] [PATCH for-3.2 v3 02/14] qom: make interface types abstract

[Eduardo Habkost]

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 <address@hidden> wrote:
> 
> > Interfaces don't have instance, let's make the interface type really
> > abstract to avoid confusion.
> > 
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> >  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 {
> > -    /* <private> */
> > -    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