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

[Laszlo Ersek]

On 11/20/18 17:33, 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

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
> 
> [...]
>