[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: |
Fri, 23 Oct 2015 14:44:54 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
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.
>>
>> Some of the ugliness of this patch will disappear once
>> structs are laid out in the same manner as unions; mark
>> it with TODO for now.
>>
>> Note that C makes const-correct upcasts annoying because
>> it lacks overloads; these functions cast away const so that
>> they can accept user pointers whether const or not, and the
>> result in turn can be assigned to normal or const pointers.
>> Alternatively, this could have been done with macros, but
>> those tend to not have quite as much type safety.
>
> Well, if you torture the preprocessor hard enough, it surrenders and
> lets you do type checking. Torture isn't pretty, though. See for
> instance
>
> http://git.ozlabs.org/?p=ccan;a=blob;f=ccan/check_type/check_type.h;h=77501a95597c3e73396c270d54a8ed53a9defbc4;hb=HEAD
Oddly enough, our container_of() macro looks verrrrry similar to the
comments in that sample code (that is, we DO use the preprocessor for a
type check). :)
>> +++ b/scripts/qapi-types.py
>> @@ -97,6 +97,23 @@ struct %(c_name)s {
>> return ret
>>
>>
>> +def gen_upcast(name, base, struct):
>> + # C makes const-correctness ugly. We have to cast away const to let
>> + # this function work for both const and non-const obj.
>> + # TODO Ugly difference between struct and flat union bases
>> + member = ''
>> + if struct:
>> + member = '->base'
>> + return mcgen('''
>> +
>> +static inline %(base)s *qapi_%(c_name)s_base(const %(c_name)s *obj)
>> +{
>> + return (%(base)s *)obj%(member)s;
>
>> +}
>> +''',
>> + c_name=c_name(name), base=base.c_name(), member=member)
>
> The ugliness doesn't bother me much, because it'll go away. But perhaps
> we should simply limit gen_upcast() to unions now, and extend it to
> structs when we unbox their base.
>
Okay, I could do that. Then I definitely need to post a v11 respin,
this is starting to be too much for simple fixups to v10.
>> - add_addr_info(server->base, (struct sockaddr *)&info->laddr_ext,
>> + add_addr_info(qapi_SpiceServerInfo_base(server),
>> + (struct sockaddr *)&info->laddr_ext,
>> info->llen_ext);
>> } else {
>> error_report("spice: %s, extended address is expected",
>
> This change will start to make sense when we unbox base. Right now, it
> doesn't :)
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.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH v10 10/25] qapi: Unbox base members, (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