On Wed, Aug 7, 2013 at 3:53 PM, Alexey Kardashevskiy <a...@ozlabs.ru> wrote: > On 08/07/2013 03:45 PM, Andreas Färber wrote: >> Am 07.08.2013 07:38, schrieb Alexey Kardashevskiy: >>> On 08/07/2013 02:20 PM, Peter Crosthwaite wrote: >>>> Hi, >>>> >>>> On Wed, Aug 7, 2013 at 1:36 PM, Alexey Kardashevskiy <a...@ozlabs.ru> >>>> wrote: >>>>> On 08/06/2013 06:33 PM, Andreas Färber wrote: >>>>>> Am 06.08.2013 07:54, schrieb Alexey Kardashevskiy: >>>>>>> On 08/01/2013 12:17 PM, Andreas Färber wrote: >>>>>>>> The object argument is currently unused and may be used to optimize the >>>>>>>> class lookup when needed. >>>>>>>> >>>>>>>> Inspired-by: Peter Crosthwaite <peter.crosthwa...@xilinx.com> >>>>>>>> Signed-off-by: Andreas Färber <afaer...@suse.de> >>>>>>>> --- >>>>>>>> include/qom/object.h | 10 ++++++++++ >>>>>>>> 1 file changed, 10 insertions(+) >>>>>>>> >>>>>>>> diff --git a/include/qom/object.h b/include/qom/object.h >>>>>>>> index 23fc048..a8e71dc 100644 >>>>>>>> --- a/include/qom/object.h >>>>>>>> +++ b/include/qom/object.h >>>>>>>> @@ -511,6 +511,16 @@ struct TypeInfo >>>>>>>> OBJECT_CLASS_CHECK(class, object_get_class(OBJECT(obj)), name) >>>>>>>> >>>>>>>> /** >>>>>>>> + * OBJECT_GET_PARENT_CLASS: >>>>>>>> + * @obj: The object to obtain the parent class for. >>>>>>>> + * @name: The QOM typename of @obj. >>>>>>>> + * >>>>>>>> + * Returns the parent class for a given object of a specific class. >>>>>>>> + */ >>>>>>>> +#define OBJECT_GET_PARENT_CLASS(obj, name) \ >>>>>>>> + object_class_get_parent(object_class_by_name(name)) >>>>>>>> + >>>>>>>> +/** >>>>>>>> * InterfaceInfo: >>>>>>>> * @type: The name of the interface. >>>>>>>> * >>>>>>>> >>>>>>> >>>>>>> Has anyone ever tried to use this macro? >>>>>> >>>>>> Since you're asking me, obviously later in this virtio series it's used >>>>>> and in the IndustryPack series as well. >>>>>> >>>>>> I'm not aware of anyone else having used it yet - I'm still waiting for >>>>>> review feedback from Peter Cr. and/or Anthony (or you!) before I put it >>>>>> on qom-next. >>>>> >>>>> >>>>> Still do not understand what "obj" is doing here. >>>> >>>> The object is currently where cast cache optimizations are >>>> implemented. So having a handle to it is useful if ever these >>>> get-parent operations end up in fast paths and we need to do one of >>>> Anthony's caching tricks. >>>> >>>>> This what I would suggest: >>>>> >>>>> #define OBJECT_GET_PARENT_CLASS(obj, name) \ >>>>> object_class_get_parent(OBJECT_GET_CLASS(ObjectClass, (obj), (name))) >>>>> >>>> >>>> You have changed the semantic from what Andreas has implemented. This >>>> will always return the parent of the concrete class, whereas to solve >>>> the save-override-call problem you need to get the parent of abstract >>>> level that is overriding the function (not always the concrete class). >>>> >>>>> @name here is just to make sure we are at the right level of the class >>>>> hierarchy. >>>>> >>>> >>>> Its @name that is actually important. Its more than just assertive, >>>> variation of @name for the same object will and should return >>>> different results (aside from just checking). The demonstrative case >>>> for this requirement is TYPE_ARM_CPU, which has a realize fn that >>>> needs to call the parent version as implemented by TYPE_CPU. ARM CPUs >>>> however have a whole bunch of concrete classes inheriting from >>>> TYPE_ARM_CPU. So using your macro, TYPE_ARM_CPU's realize fn will only >>>> be able to get a handle to the parent of the concrete class (i.e. >>>> TYPE_ARM_CPU) >>> >>> This is what I would really (really) expect in this situation. >>> >>>> and not the parent of TYPE_ARM_CPU (i.e. TYPE_CPU) as >>>> intended. >>> >>> Oh. Then it is not get_parent() at all, this is get_grand_parent. >> >> No. It is parent of TYPE_ARM_CPU, as specified by the name argument. > > Here I lost you. You really want finalize() of TYPE_CPU, no matter how many > classes are between TYPE_CPU and your final ARM CPU class.
No, we want the immediate parent of the TYPE_ARM_CPU class which is not set in stone. Directly refing TYPE_CPU makes it difficult for anyone trying to re-organise the QOM heirachy. The idea behind this approach was that TYPE_ARM_CPU::realize does not make assumptions about the classes above it (except that someone in the ancestry is TYPE_DEVICE for the definition of realize) nor the classes below it (as already discussed). For example, if someone decides to implement TYPE_CPU_FOO (abstract child of TYPE_CPU) and reparents TYPE_ARM_CPU to this, then without need-for-change TYPE_ARM_CPU will now call TYPE_CPU_FOO:realize rather than inadvertently skipping over levels to a grandparent implementation. It can be done your proposed way, but re-organising the heirachy will potentially require change patterns that are avoided with this approach. So call it > directly, why do you need this workaround with get_parent if it is not > really a parent of the current obj and you do not trust to what you get in This is probably terminology confusion. Its a parent of a class not an object. The documentation and naming maybe needs work? Regards, Peter
