Paolo Bonzini <pbonz...@redhat.com> 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
