[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/6] qapi: Add qobject_to()
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/6] qapi: Add qobject_to() |
Date: |
Mon, 22 Jan 2018 17:53:58 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 |
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 <address@hidden>
>> ---
>> 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!
>
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v2 0/6] block: Handle null backing link, Max Reitz, 2018/01/20
- [Qemu-devel] [PATCH v2 1/6] qapi: Add qobject_to(), Max Reitz, 2018/01/20
- [Qemu-devel] [PATCH v2 3/6] qapi: Remove qobject_to_X() functions, Max Reitz, 2018/01/20
- [Qemu-devel] [PATCH v2 4/6] qapi: Make more of qobject_to(), Max Reitz, 2018/01/20
- [Qemu-devel] [PATCH v2 6/6] block: Deprecate "backing": "", Max Reitz, 2018/01/20
- [Qemu-devel] [PATCH v2 5/6] block: Handle null backing link, Max Reitz, 2018/01/20
- [Qemu-devel] [PATCH v2 2/6] qapi: Replace qobject_to_X(o) by qobject_to(o, X), Max Reitz, 2018/01/20