Marc-André Lureau <marcandre.lur...@redhat.com> writes: > By moving the base fields to a QObjectBase_, QObject can be a type > which also has a 'base' field. This allows to write a generic > QOBJECT() macro that will work with any QObject type, including > QObject itself. The container_of() macro ensures that the object to > cast has a QObjectBase_ base field, giving some type safety > guarantees. However, for it to work properly, all QObject types must > have 'base' at offset 0 (which is ensured by static checking from > the previous patch)
I'm afraid this condition is neither sufficient nor necessary. QOBJECT() maps a pointer to some subtype to the base type QObject. For this to work, the subtype must contain a QObject. Before the patch, this is trivially the case: the subtypes have a member QObject base, and QOBJECT() returns its address. Afterwards, the subtypes have a member QObjectBase_ base, and QOBJECT() returns the address of a notional QObject wrapped around this member. Works because QObject has no other members. The condition 'base is at offset 0' does not ensure this, and is therefore not sufficient. It's not necessary, either: putting base elsewhere would work just fine as long as we put *all* of QObject there. Please document the real condition "QObject must have no members but QObjectBase_ base, or else QOBJECT() breaks". Feel free to check their sizes are the same (I wouldn't bother). If requiring base to be at offset zero for all subtypes materially simplifies code, then go ahead and do that in a separate patch. My gut feeling is it doesn't, but I could be wrong. > QObjectBase_ is not typedef and use a trailing underscore to make it > obvious it is not for normal use and to avoid potential abuse. Trailing underscore I like, lack of typedef I don't mind (but I'm firmly in the "eschew typedef for structs" camp). It does violate the common QEMU coding style, though. A comment /* Not for use outside include/qapi/qmp/ */ next to QObjectBase_ wouldn't hurt. > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
