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.
>>> 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.
Paolo
