| Message ID | 20180120154412.9990-2-mreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/20/2018 09:44 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). > > The macro uses lower-case letters because: > 1. There does not seem to be a hard rule on whether qemu macros have to > be upper-cased, > 2. The current situation in qapi/qmp is inconsistent (compare e.g. > QINCREF() vs. qdict_put()), > 3. qobject_to() will evaluate its @obj parameter only once, thus it is > generally not important to the caller whether it is a macro or not, > 4. I prefer it aesthetically. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > You're more than welcome to convince me to call it QOBJECT_TO()! I can live with the lower-case version; especially with your good list of reasons for it ;) > --- > include/qapi/qmp/qobject.h | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h > index 38ac68845c..1211989ca0 100644 > --- a/include/qapi/qmp/qobject.h > +++ b/include/qapi/qmp/qobject.h > @@ -50,6 +50,24 @@ struct QObject { > #define QDECREF(obj) \ > qobject_decref(obj ? QOBJECT(obj) : NULL) > > +/* Required for qobject_to() */ > +#define QTYPE_CAST_TO_QNull QTYPE_QNULL > +#define QTYPE_CAST_TO_QNum QTYPE_QNUM > +#define QTYPE_CAST_TO_QString QTYPE_QSTRING > +#define QTYPE_CAST_TO_QDict QTYPE_QDICT > +#define QTYPE_CAST_TO_QList QTYPE_QLIST > +#define QTYPE_CAST_TO_QBool QTYPE_QBOOL > + > +#ifdef CONFIG_STATIC_ASSERT > +_Static_assert(QTYPE__MAX == 7, > + "The QTYPE_CAST_TO_* list needs to be extended"); > +#endif Why not use QEMU_BUILD_BUG_ON() instead of _Static_assert? That way, this check will break the build even on older compilers that lack _Static_assert but where we still know how to trigger a build failure. > + > +#define qobject_to(obj, type) \ > + container_of(qobject_check_type(obj, glue(QTYPE_CAST_TO_, type)) ?: \ > + QOBJECT((type *)NULL), \ > + type, base) Slick!
On 2018-01-22 16:33, Eric Blake wrote: > On 01/20/2018 09:44 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). >> >> The macro uses lower-case letters because: >> 1. There does not seem to be a hard rule on whether qemu macros have to >> be upper-cased, >> 2. The current situation in qapi/qmp is inconsistent (compare e.g. >> QINCREF() vs. qdict_put()), >> 3. qobject_to() will evaluate its @obj parameter only once, thus it is >> generally not important to the caller whether it is a macro or not, >> 4. I prefer it aesthetically. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> You're more than welcome to convince me to call it QOBJECT_TO()! > > I can live with the lower-case version; especially with your good list > of reasons for it ;) > >> --- >> include/qapi/qmp/qobject.h | 32 ++++++++++++++++++++++++++++++++ >> 1 file changed, 32 insertions(+) >> >> diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h >> index 38ac68845c..1211989ca0 100644 >> --- a/include/qapi/qmp/qobject.h >> +++ b/include/qapi/qmp/qobject.h >> @@ -50,6 +50,24 @@ struct QObject { >> #define QDECREF(obj) \ >> qobject_decref(obj ? QOBJECT(obj) : NULL) >> >> +/* Required for qobject_to() */ >> +#define QTYPE_CAST_TO_QNull QTYPE_QNULL >> +#define QTYPE_CAST_TO_QNum QTYPE_QNUM >> +#define QTYPE_CAST_TO_QString QTYPE_QSTRING >> +#define QTYPE_CAST_TO_QDict QTYPE_QDICT >> +#define QTYPE_CAST_TO_QList QTYPE_QLIST >> +#define QTYPE_CAST_TO_QBool QTYPE_QBOOL >> + >> +#ifdef CONFIG_STATIC_ASSERT >> +_Static_assert(QTYPE__MAX == 7, >> + "The QTYPE_CAST_TO_* list needs to be extended"); >> +#endif > > Why not use QEMU_BUILD_BUG_ON() instead of _Static_assert? That way, > this check will break the build even on older compilers that lack > _Static_assert but where we still know how to trigger a build failure. One reason would be because _Static_assert() allows me to give a nice warning message -- I could add a comment above the QEMU_BUILD_BUG_ON() line and hope for the best, but still, I like that better. The other is that I assumed most people would be using C11 compilers now, so the omission would be spotted sufficiently early. "Sufficiently" because it's actually not that bad for this list to be incomplete. The worst is that you can't use qobject_to() for your desired target type, but that's hardly catastrophic. I could add an #else branch with QEMU_BUILD_BUG_ON(). Or I could add a QEMU_BUILD_BUG_MSG() macro that takes a message (which is omitted if you don't have _Static_assert() but would still make for a nice comment). I guess the latter would be a nice idea in general...? Max >> + >> +#define qobject_to(obj, type) \ >> + container_of(qobject_check_type(obj, glue(QTYPE_CAST_TO_, type)) ?: \ >> + QOBJECT((type *)NULL), \ >> + type, base) > > Slick! >
On Sat 20 Jan 2018 04:44:07 PM CET, 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). > > The macro uses lower-case letters because: > 1. There does not seem to be a hard rule on whether qemu macros have to > be upper-cased, > 2. The current situation in qapi/qmp is inconsistent (compare e.g. > QINCREF() vs. qdict_put()), > 3. qobject_to() will evaluate its @obj parameter only once, thus it is > generally not important to the caller whether it is a macro or not, > 4. I prefer it aesthetically. > > Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Berto
diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h index 38ac68845c..1211989ca0 100644 --- a/include/qapi/qmp/qobject.h +++ b/include/qapi/qmp/qobject.h @@ -50,6 +50,24 @@ struct QObject { #define QDECREF(obj) \ qobject_decref(obj ? QOBJECT(obj) : NULL) +/* Required for qobject_to() */ +#define QTYPE_CAST_TO_QNull QTYPE_QNULL +#define QTYPE_CAST_TO_QNum QTYPE_QNUM +#define QTYPE_CAST_TO_QString QTYPE_QSTRING +#define QTYPE_CAST_TO_QDict QTYPE_QDICT +#define QTYPE_CAST_TO_QList QTYPE_QLIST +#define QTYPE_CAST_TO_QBool QTYPE_QBOOL + +#ifdef CONFIG_STATIC_ASSERT +_Static_assert(QTYPE__MAX == 7, + "The QTYPE_CAST_TO_* list needs to be extended"); +#endif + +#define qobject_to(obj, type) \ + container_of(qobject_check_type(obj, glue(QTYPE_CAST_TO_, type)) ?: \ + QOBJECT((type *)NULL), \ + type, base) + /* Initialize an object to default values */ static inline void qobject_init(QObject *obj, QType type) { @@ -102,4 +120,18 @@ static inline QType qobject_type(const QObject *obj) return obj->type; } +/** + * qobject_check_type(): Helper function for the qobject_to() macro. + * Return @obj, but only if @obj is not NULL and @type is equal to + * @obj's type. Return NULL otherwise. + */ +static inline QObject *qobject_check_type(const QObject *obj, QType type) +{ + if (obj && qobject_type(obj) == type) { + return (QObject *)obj; + } else { + return NULL; + } +} + #endif /* QOBJECT_H */
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). The macro uses lower-case letters because: 1. There does not seem to be a hard rule on whether qemu macros have to be upper-cased, 2. The current situation in qapi/qmp is inconsistent (compare e.g. QINCREF() vs. qdict_put()), 3. qobject_to() will evaluate its @obj parameter only once, thus it is generally not important to the caller whether it is a macro or not, 4. I prefer it aesthetically. Signed-off-by: Max Reitz <mreitz@redhat.com> --- You're more than welcome to convince me to call it QOBJECT_TO()! --- include/qapi/qmp/qobject.h | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)