[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v10 09/25] qapi: Prefer typesafe upcasts to qapi
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v10 09/25] qapi: Prefer typesafe upcasts to qapi base classes |
Date: |
Mon, 26 Oct 2015 10:24:56 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 10/26/2015 01:33 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>
>> On 10/23/2015 09:30 AM, Markus Armbruster wrote:
>>> Eric Blake <address@hidden> writes:
>>>
>>>> A previous patch (commit 1e6c1616) made it possible to
>>>> directly cast from a qapi type to its base type. A future
>>>> patch will do likewise for structs. However, it requires
>>>> the client code to use a C cast, which turns off compiler
>>>> type-safety checks if the client gets it wrong. So this
>>>
>>> Who's the client? Suggest to simply drop "if the client gets it wrong".
>>>
>>>> patch adds inline type-safe wrappers named qapi_FOO_base()
>>>> for any type FOO that has a base, which can be used to
>>>> upcast a qapi type to its base, then uses the new generated
>>>> functions in places where we were already casting.
>>>>
>>
>> Indeed, if I don't port gen_upcast() to types until patch 10/25, then
>> this hunk also has to move to patch 10. That means no clients of the
>> upcast macros in patch 9/25, _unless_ I add testsuite coverage. Which I
>> probably ought to do.
>>
>>>> switch (event) {
>>>> case QAPI_EVENT_VNC_CONNECTED:
>>>> - qapi_event_send_vnc_connected(si, vs->info->base, &error_abort);
>>>> + qapi_event_send_vnc_connected(si,
>>>> qapi_VncClientInfo_base(vs->info),
>>>> + &error_abort);
>>>> break;
>>>> case QAPI_EVENT_VNC_INITIALIZED:
>>>> qapi_event_send_vnc_initialized(si, vs->info, &error_abort);
>>>
>>> Not a single cast to union base?
>>
>> Not that I could find. So I'll have to create one in the testsuite.
>
> I guess I'd introduce gen_upcast() only with its first real user,
> i.e. when unboxing struct base. At that time, its obvious
> implementation should work for both struct and union, even though we
> actually use it only for struct. If you want to add a test case for
> unions, go ahead. I'm not sure I'd bother, because once we unify code
> generation for the two, testing them separately won't add much value
> anymore.
It sounds like I have two options for v11:
1. Keep 9/25 introducing gen_upcast(), just for union types, and
including testsuite coverage. In 10/25, make use of the upcast functions
to struct as part of making structs sane.
2. Swap the patch order: do 10/25 to alter struct layout first, using
ugly casts; then implement 9/25 that adds gen_upcast() and fixes the
ugly casts to instead use the new upcast functions.
I can go either way, so do you have any preference?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v10 21/25] tpm: Convert to new qapi union layout, (continued)
- [Qemu-devel] [PATCH v10 21/25] tpm: Convert to new qapi union layout, Eric Blake, 2015/10/23
- [Qemu-devel] [PATCH v10 17/25] net: Convert to new qapi union layout, Eric Blake, 2015/10/23
- [Qemu-devel] [PATCH v10 25/25] qapi: Simplify gen_struct_field(), Eric Blake, 2015/10/23
- [Qemu-devel] [PATCH v10 12/25] qapi: Start converting to new qapi union layout, Eric Blake, 2015/10/23
- [Qemu-devel] [PATCH v10 15/25] block: Convert to new qapi union layout, Eric Blake, 2015/10/23
- [Qemu-devel] [PATCH v10 14/25] tests: Convert to new qapi union layout, Eric Blake, 2015/10/23
- [Qemu-devel] [PATCH v10 09/25] qapi: Prefer typesafe upcasts to qapi base classes, Eric Blake, 2015/10/23
[Qemu-devel] [PATCH v10 20/25] memory: Convert to new qapi union layout, Eric Blake, 2015/10/23
[Qemu-devel] [PATCH v10 22/25] qapi: Finish converting to new qapi union layout, Eric Blake, 2015/10/23
[Qemu-devel] [PATCH v10 18/25] char: Convert to new qapi union layout, Eric Blake, 2015/10/23
[Qemu-devel] [PATCH v10 23/25] qapi: Reserve 'u' member name, Eric Blake, 2015/10/23
[Qemu-devel] [PATCH v10 11/25] qapi-visit: Remove redundant functions for flat union base, Eric Blake, 2015/10/23