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. QObject must have no members but QObjectBase_ base, or
else QOBJECT() breaks.

QObjectBase_ is not typedef and use a trailing underscore to make it
obvious it is not for normal use and to avoid potential abuse.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qapi/qmp/qbool.h   |  2 +-
 include/qapi/qmp/qdict.h   |  2 +-
 include/qapi/qmp/qlist.h   |  2 +-
 include/qapi/qmp/qnull.h   |  2 +-
 include/qapi/qmp/qnum.h    |  2 +-
 include/qapi/qmp/qobject.h | 30 +++++++++++++++++++-----------
 include/qapi/qmp/qstring.h |  2 +-
 qobject/qobject.c          | 12 ++++++------
 tests/check-qdict.c        |  6 +++---
 9 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/include/qapi/qmp/qbool.h b/include/qapi/qmp/qbool.h
index b9a44a1bfe..5f61e38e64 100644
--- a/include/qapi/qmp/qbool.h
+++ b/include/qapi/qmp/qbool.h
@@ -17,7 +17,7 @@
 #include "qapi/qmp/qobject.h"
 
 struct QBool {
-    QObject base;
+    struct QObjectBase_ base;
     bool value;
 };
 
diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index 2cc3e906f7..921a28d2d3 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -25,7 +25,7 @@ typedef struct QDictEntry {
 } QDictEntry;
 
 struct QDict {
-    QObject base;
+    struct QObjectBase_ base;
     size_t size;
     QLIST_HEAD(,QDictEntry) table[QDICT_BUCKET_MAX];
 };
diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
index 5c673acb06..8d2c32ca28 100644
--- a/include/qapi/qmp/qlist.h
+++ b/include/qapi/qmp/qlist.h
@@ -22,7 +22,7 @@ typedef struct QListEntry {
 } QListEntry;
 
 struct QList {
-    QObject base;
+    struct QObjectBase_ base;
     QTAILQ_HEAD(,QListEntry) head;
 };
 
diff --git a/include/qapi/qmp/qnull.h b/include/qapi/qmp/qnull.h
index c992ee2ae1..e8ea2c315a 100644
--- a/include/qapi/qmp/qnull.h
+++ b/include/qapi/qmp/qnull.h
@@ -16,7 +16,7 @@
 #include "qapi/qmp/qobject.h"
 
 struct QNull {
-    QObject base;
+    struct QObjectBase_ base;
 };
 
 extern QNull qnull_;
diff --git a/include/qapi/qmp/qnum.h b/include/qapi/qmp/qnum.h
index 3e47475b2c..45bf02a036 100644
--- a/include/qapi/qmp/qnum.h
+++ b/include/qapi/qmp/qnum.h
@@ -45,7 +45,7 @@ typedef enum {
  * convert under the hood.
  */
 struct QNum {
-    QObject base;
+    struct QObjectBase_ base;
     QNumKind kind;
     union {
         int64_t i64;
diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
index 5206ff9ee1..0a7f800d58 100644
--- a/include/qapi/qmp/qobject.h
+++ b/include/qapi/qmp/qobject.h
@@ -34,13 +34,20 @@
 
 #include "qapi/qapi-builtin-types.h"
 
-struct QObject {
+/* Not for use outside include/qapi/qmp/ */
+struct QObjectBase_ {
     QType type;
     size_t refcnt;
 };
 
-/* Get the 'base' part of an object */
-#define QOBJECT(obj) (&(obj)->base)
+struct QObject {
+    struct QObjectBase_ base;
+};
+
+#define QOBJECT(x) ({                                       \
+    typeof(x) __x = (x);                                    \
+    __x ? container_of(&(__x)->base, QObject, base) : NULL; \
+})
 
 /* High-level interface for qobject_incref() */
 #define QINCREF(obj)      \
@@ -68,8 +75,8 @@ QEMU_BUILD_BUG_MSG(QTYPE__MAX != 7,
 static inline void qobject_init(QObject *obj, QType type)
 {
     assert(QTYPE_NONE < type && type < QTYPE__MAX);
-    obj->refcnt = 1;
-    obj->type = type;
+    obj->base.refcnt = 1;
+    obj->base.type = type;
 }
 
 /**
@@ -77,8 +84,9 @@ static inline void qobject_init(QObject *obj, QType type)
  */
 static inline void qobject_incref(QObject *obj)
 {
-    if (obj)
-        obj->refcnt++;
+    if (obj) {
+        obj->base.refcnt++;
+    }
 }
 
 /**
@@ -101,8 +109,8 @@ void qobject_destroy(QObject *obj);
  */
 static inline void qobject_decref(QObject *obj)
 {
-    assert(!obj || obj->refcnt);
-    if (obj && --obj->refcnt == 0) {
+    assert(!obj || obj->base.refcnt);
+    if (obj && --obj->base.refcnt == 0) {
         qobject_destroy(obj);
     }
 }
@@ -112,8 +120,8 @@ static inline void qobject_decref(QObject *obj)
  */
 static inline QType qobject_type(const QObject *obj)
 {
-    assert(QTYPE_NONE < obj->type && obj->type < QTYPE__MAX);
-    return obj->type;
+    assert(QTYPE_NONE < obj->base.type && obj->base.type < QTYPE__MAX);
+    return obj->base.type;
 }
 
 /**
diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index 30ae260a7f..b3b3d444d2 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -16,7 +16,7 @@
 #include "qapi/qmp/qobject.h"
 
 struct QString {
-    QObject base;
+    struct QObjectBase_ base;
     char *string;
     size_t length;
     size_t capacity;
diff --git a/qobject/qobject.c b/qobject/qobject.c
index 87649c5be5..cf4b7e229e 100644
--- a/qobject/qobject.c
+++ b/qobject/qobject.c
@@ -37,9 +37,9 @@ static void (*qdestroy[QTYPE__MAX])(QObject *) = {
 
 void qobject_destroy(QObject *obj)
 {
-    assert(!obj->refcnt);
-    assert(QTYPE_QNULL < obj->type && obj->type < QTYPE__MAX);
-    qdestroy[obj->type](obj);
+    assert(!obj->base.refcnt);
+    assert(QTYPE_QNULL < obj->base.type && obj->base.type < QTYPE__MAX);
+    qdestroy[obj->base.type](obj);
 }
 
 
@@ -62,11 +62,11 @@ bool qobject_is_equal(const QObject *x, const QObject *y)
         return true;
     }
 
-    if (!x || !y || x->type != y->type) {
+    if (!x || !y || x->base.type != y->base.type) {
         return false;
     }
 
-    assert(QTYPE_NONE < x->type && x->type < QTYPE__MAX);
+    assert(QTYPE_NONE < x->base.type && x->base.type < QTYPE__MAX);
 
-    return qis_equal[x->type](x, y);
+    return qis_equal[x->base.type](x, y);
 }
diff --git a/tests/check-qdict.c b/tests/check-qdict.c
index 2e73c2f86e..029b6b15b9 100644
--- a/tests/check-qdict.c
+++ b/tests/check-qdict.c
@@ -570,11 +570,11 @@ static void qdict_join_test(void)
         }
 
         /* Check the references */
-        g_assert(qdict_get(dict1, "foo")->refcnt == 1);
-        g_assert(qdict_get(dict1, "bar")->refcnt == 1);
+        g_assert(qdict_get(dict1, "foo")->base.refcnt == 1);
+        g_assert(qdict_get(dict1, "bar")->base.refcnt == 1);
 
         if (!overwrite) {
-            g_assert(qdict_get(dict2, "foo")->refcnt == 1);
+            g_assert(qdict_get(dict2, "foo")->base.refcnt == 1);
         }
 
         /* Clean up */
-- 
2.17.0.rc1.36.gcedb63ea2f

On 04/17/2018 08:35 AM, Marc-André Lureau wrote:
> 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

either "allows us to write" or "allows writing"

> 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. QObject must have no members but QObjectBase_ base, or
> else QOBJECT() breaks.
> 
> QObjectBase_ is not typedef and use a trailing underscore to make it

s/not typedef and use/not a typedef and uses/

> obvious it is not for normal use and to avoid potential abuse.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---

> +++ b/include/qapi/qmp/qobject.h
> @@ -34,13 +34,20 @@
>  
>  #include "qapi/qapi-builtin-types.h"
>  
> -struct QObject {
> +/* Not for use outside include/qapi/qmp/ */
> +struct QObjectBase_ {
>      QType type;
>      size_t refcnt;
>  };
>  
> -/* Get the 'base' part of an object */
> -#define QOBJECT(obj) (&(obj)->base)
> +struct QObject {
> +    struct QObjectBase_ base;

I'd add a comment here that this struct must have no other members than
base.

> +};
> +
> +#define QOBJECT(x) ({                                       \
> +    typeof(x) __x = (x);                                    \
> +    __x ? container_of(&(__x)->base, QObject, base) : NULL; \
> +})
>  

With those changes,
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

In addition to Eric's comments...

Marc-André Lureau <marcandre.lureau@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. QObject must have no members but QObjectBase_ base, or
> else QOBJECT() breaks.
>
> QObjectBase_ is not typedef and use a trailing underscore to make it
> obvious it is not for normal use and to avoid potential abuse.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
[...]
> diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
> index 5206ff9ee1..0a7f800d58 100644
> --- a/include/qapi/qmp/qobject.h
> +++ b/include/qapi/qmp/qobject.h
> @@ -34,13 +34,20 @@
>  
>  #include "qapi/qapi-builtin-types.h"
>  
> -struct QObject {
> +/* Not for use outside include/qapi/qmp/ */
> +struct QObjectBase_ {
>      QType type;
>      size_t refcnt;
>  };
>  
> -/* Get the 'base' part of an object */
> -#define QOBJECT(obj) (&(obj)->base)
> +struct QObject {
> +    struct QObjectBase_ base;
> +};
> +
> +#define QOBJECT(x) ({                                       \
> +    typeof(x) __x = (x);                                    \
> +    __x ? container_of(&(__x)->base, QObject, base) : NULL; \
> +})

Identifiers that begin with double underscores are reserved for any
use.  Drop one of the two, please.

>  
>  /* High-level interface for qobject_incref() */
>  #define QINCREF(obj)      \
> @@ -68,8 +75,8 @@ QEMU_BUILD_BUG_MSG(QTYPE__MAX != 7,
>  static inline void qobject_init(QObject *obj, QType type)
>  {
>      assert(QTYPE_NONE < type && type < QTYPE__MAX);
> -    obj->refcnt = 1;
> -    obj->type = type;
> +    obj->base.refcnt = 1;
> +    obj->base.type = type;
>  }
>  
>  /**
[...]