qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v3 2/7] qapi: Add qobject_to()


From: Eric Blake
Subject: Re: [Qemu-block] [PATCH v3 2/7] qapi: Add qobject_to()
Date: Mon, 19 Mar 2018 14:36:55 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0

On 02/26/2018 05:58 AM, Max Reitz wrote:
On 2018-02-24 21:57, Eric Blake wrote:
On 02/24/2018 09:40 AM, Max Reitz wrote:
This is a dynamic casting macro that, given a QObject type, returns an
object as that type or NULL if the object is of a different type (or
NULL itself).


+#define qobject_to(obj, type) \
+    container_of(qobject_check_type(obj, glue(QTYPE_CAST_TO_, type))
?: \
+                     QOBJECT((type *)NULL), \

I guess the third (second?) branch of the ternary is written this way,
rather than the simpler 'NULL', to ensure that 'type' is still something
that can have the QOBJECT() macro applied to it?  Should be okay.

It's written this way because of the container_of() around it.  We want
the whole expression to return NULL then, and without the QOBJECT()
around it, it would only return NULL if offsetof(type, base) == 0 (which
it is not necessarily).

OTOH, container_of(&((type *)NULL)->base, type, base) is by definition NULL.

(QOBJECT(x) is &(x)->base)

Well, clang's ubsan griped:
https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg05143.html

Practically, all of our qtypes have 'base' at offset 0, which means (QObject*)addr and (QString*)addr are the same address, even when addr is NULL. But neither QOBJECT() nor container_of() are currently fit to run on a NULL pointer, since the 'base' member need not be at offset 0, at which point, we'd be converting away from the NULL pointer on the &(x)->base conversion, and then back to the NULL pointer on the container_of() conversion. So at the end of the day, we get the right results, but we relied on undefined behavior in the interim.

So here's what I'm squashing in, if you like it (and remembering that I already swapped argument order to be qobject_to(type, obj) in my pending pull requests):

diff --git i/include/qapi/qmp/qobject.h w/include/qapi/qmp/qobject.h
index ea9702270e7..e6ce9347ab8 100644
--- i/include/qapi/qmp/qobject.h
+++ w/include/qapi/qmp/qobject.h
@@ -62,9 +62,8 @@ QEMU_BUILD_BUG_MSG(QTYPE__MAX != 7,
                    "The QTYPE_CAST_TO_* list needs to be extended");

 #define qobject_to(type, obj) \
-    container_of(qobject_check_type(obj, glue(QTYPE_CAST_TO_, type)) ?: \
-                     QOBJECT((type *)NULL), \
-                 type, base)
+ ({ QObject *_tmp = qobject_check_type(obj, glue(QTYPE_CAST_TO_, type)); \
+        _tmp ? container_of(_tmp, type, base) : (type *)NULL; })

 /* Initialize an object to default values */
 static inline void qobject_init(QObject *obj, QType type)


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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]