Re: [Qemu-devel] Issues around TYPE_INTERFACE

[Markus Armbruster]

Paolo Bonzini <address@hidden> writes:

> On 18/03/19 13:50, Markus Armbruster wrote:
>>> I actually wonder why we even need "OBJECT_CLASS" as it exists here
>>> at all.
>>>
>>> $ git grep 'OBJECT_CLASS\b' | wc -l
>>> 38
>>> $ git grep 'OBJECT_CLASS_CHECK\b' | wc -l
>>> 196
>>>
>>> There's no obvious pattern in why OBJECT_CLASS is being favoured.
>>> The only compelling benefit of it is that it is faster since it
>>> avoids checking anything.
>
> It's for consistency with e.g. DEVICE_CLASS.  Because all classes
> descend from Object, all XxxClass structs descend from ObjectClass;
> hence, OBJECT_CLASS does not need to do the slow check.  However, we
> could certainly redefine OBJECT_CLASS to use OBJECT_CLASS_CHECK.
>
>>> IOW, I think we could/should remove OBJECT_CLASS and rename
>>> OBJECT_CLASS_CHECK to OBJECT_CLASS.
>
> Why? OBJECT_CLASS is in the same family as DEVICE_CLASS, it takes a
> class pointer and returns another class pointer.  FOO_GET_CLASS takes an
> object pointer and returns a class pointer.  But OBJECT_CLASS_CHECK
> takes 3 arguments, it's absolutely not the same thing as a *_CLASS macro.
>
>>> If we wanted to keep INTERFACE_CHECK then I agree we should make it fail
>>> when not given an interface type, but i'm not really convinced it is
>>> worth it. Code is inevitably going to get these mixed up, as this QAUTHZ
>>> example shows. So doing a stronger check will just turn currently working
>>> code into broken code at runtime.
>>>
>>> I'd vote for removing INTERFACE_CHECK and using OBJECT_CHECK universally.
>>
>> Slightly unclean, since interface types really aren't object types.
>> Tolerable.
>
> But it takes an Object * (or subclass thereof) as the first argument, so
> I'm very much in favor of this.

Elsewhere in this thread, I described how OBJECT_CHECK() and
INTERFACE_CHECK() return their @obj argument even when casting to an
interface.  If we decide that's what we want, then I'm fine with getting
rid of INTERFACE_CHECK().

>>>> Yet they're used for less than half of the interface names:
>>>>
>>>>     TYPE_FW_PATH_PROVIDER
>>>>     TYPE_HOTPLUG_HANDLER
>>>>     TYPE_INTERRUPT_STATS_PROVIDER
>>>>     TYPE_ISADMA
>>>>     TYPE_MEMORY_DEVICE
>>>>     TYPE_NMI
>>>>     TYPE_NVRAM
>>>>     TYPE_PPC_VIRTUAL_HYPERVISOR
>>>>     TYPE_STREAM_SLAVE
>>>>     TYPE_USER_CREATABLE
>>>>     TYPE_XICS_FABRIC
>>>>     TYPE_XIVE_NOTIFIER
>>>>
>>>> Can we agree on one naming convention, and enforce it?
>
> I tend to like interface names that express an ability ("foo-er",
> "foo-able").  Anything else should use a *_IF or *_IFACE suffix.

I like the _IFACE suffix.

Does eliding the _IFACE suffix for names ending with ER or ABLE improve
readability?

"foo-er" interface types:

    INTERFACE_RDMA_PROVIDER
    TYPE_FW_PATH_PROVIDER
    TYPE_HOTPLUG_HANDLER
    TYPE_INTERRUPT_STATS_PROVIDER
    TYPE_XIVE_NOTIFIER

"foo-er" non-interface types:

    TYPE_A9_GTIMER
    TYPE_ALTERA_TIMER
    TYPE_ARM_MPTIMER
    TYPE_ASPEED_TIMER
    TYPE_CMSDK_APB_DUALTIMER
    TYPE_CMSDK_APB_TIMER
    TYPE_CPU_CLUSTER
    TYPE_CRYPTODEV_BACKEND_VHOST_USER
    TYPE_DIGIC_TIMER
    TYPE_ETRAX_FS_TIMER
    TYPE_EXYNOS4210_COMBINER
    TYPE_FILTER_BUFFER
    TYPE_FILTER_REWRITER
    TYPE_FLAT_HEADER
    TYPE_GENERIC_LOADER
    TYPE_GRLIB_GPTIMER
    TYPE_LM32_TIMER
    TYPE_MACIO_ID_REGISTER
    TYPE_MEDIUM_CHANGER
    TYPE_MSS_TIMER
    TYPE_NETFILTER
    TYPE_NRF51_TIMER
    TYPE_PC_SPEAKER
    TYPE_PRINTER
    TYPE_PR_MANAGER
    TYPE_PR_MANAGER_HELPER
    TYPE_PXA2XX_TIMER
    TYPE_QIO_CHANNEL_BUFFER
    TYPE_QIO_DNS_RESOLVER
    TYPE_QIO_NET_LISTENER
    TYPE_REGISTER
    TYPE_ROCKER
    TYPE_SCANNER
    TYPE_SLAVIO_TIMER
    TYPE_STM32F2XX_TIMER
    TYPE_SUN4U_POWER
    TYPE_XILINX_TIMER
    TYPE_XIVE_ROUTER

"foo-able" interface types:

    TYPE_USER_CREATABLE

"foo-able" non-interface types:

    TYPE_SPAPR_TCE_TABLE