From: Eric Blake <eblake@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org, Michael Roth <mdroth@linux.vnet.ibm.com>
Subject: Re: [Qemu-devel] [PATCH v11 09/24] qapi: Prefer typesafe upcasts to qapi base classes
Date: Tue, 27 Oct 2015 08:17:35 -0600 [thread overview]
Message-ID: <562F877F.2080604@redhat.com> (raw)
In-Reply-To: <87wpu8iwiv.fsf@blackfin.pond.sub.org>
[-- Attachment #1: Type: text/plain, Size: 2658 bytes --]
On 10/27/2015 01:46 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
>
>> A previous patch (commit 1e6c1616) made it possible to
>> directly cast from a qapi flat union type to its base type.
>> However, it requires the use of a C cast, which turns off
>> compiler type-safety checks. Add inline type-safe wrappers
>> named qapi_FOO_base() for any union type FOO that has a base,
>> which can be used for a safer upcast, and enhance the
>> testsuite to cover the new functionality. A future patch
>> will extend the upcast support to structs.
>>
>> 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, the macros can be made just as type-safe, but the result is either
> somewhat ugly and using gcc-isms, or very ugly and unhygienic.
>
> I'd write something like "type-safe macros are hairy, and not worthwhile
> here."
Sure, that works for me.
>
>> This patch just adds upcasts. None of our code needed to
>> downcast from a base qapi class to a child.
>
> Actually, none of our code needs to upcast unions, either. Only the new
> tests do. Code that updasts structs exist, but it doesn't use this
> patch's upcasts until later.
>
> Suggest to amend the first paragraph:
>
> A previous patch (commit 1e6c1616) made it possible to directly cast
> from a qapi flat union type to its base type. However, it requires
> the use of a C cast, which turns off compiler type-safety checks.
> Fortunately, no such casts exist just, yet.
s/ just,/, just/
>
> Regardless, add inline type-safe wrappers named qapi_FOO_base() for
> any union type FOO that has a base, which can be used for a safer
> upcast, and enhance the testsuite to cover the new functionality.
>
> A future patch will extend the upcast support to structs, where such
> casts do exist already.
>
Maybe s/casts/conversions/ - because as of this patch, it is still a
conversion via foo->base rather than (Base *)foo (it's the next patch
that gets rid of base, and therefore needs either the cast or the wrapper).
>
> Patch looks good. I can touch up the commit message in my tree.
Sure, your proposed wording + touchups is fine.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
next prev parent reply other threads:[~2015-10-27 14:17 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-26 22:34 [Qemu-devel] [PATCH v11 00/24] qapi collision reduction (post-introspection subset B') Eric Blake
2015-10-26 22:34 ` [Qemu-devel] [PATCH v11 01/24] tests/qapi-schema: Test for reserved names, empty struct Eric Blake
2015-10-26 22:34 ` [Qemu-devel] [PATCH v11 02/24] qapi: More idiomatic string operations Eric Blake
2015-10-26 22:34 ` [Qemu-devel] [PATCH v11 03/24] qapi: More robust conditions for when labels are needed Eric Blake
2015-10-26 22:34 ` [Qemu-devel] [PATCH v11 04/24] qapi: Reserve '*List' type names for list types Eric Blake
2015-10-26 22:34 ` [Qemu-devel] [PATCH v11 05/24] qapi: Reserve 'q_*' and 'has_*' member names Eric Blake
2015-10-26 22:34 ` [Qemu-devel] [PATCH v11 06/24] vnc: Hoist allocation of VncBasicInfo to callers Eric Blake
2015-10-26 22:34 ` [Qemu-devel] [PATCH v11 07/24] qapi-visit: Split off visit_type_FOO_fields forward decl Eric Blake
2015-10-26 22:34 ` [Qemu-devel] [PATCH v11 08/24] qapi-types: Refactor base fields output Eric Blake
2015-10-26 22:34 ` [Qemu-devel] [PATCH v11 09/24] qapi: Prefer typesafe upcasts to qapi base classes Eric Blake
2015-10-27 7:46 ` Markus Armbruster
2015-10-27 14:17 ` Eric Blake [this message]
2015-10-27 15:06 ` Markus Armbruster
2015-10-26 22:34 ` [Qemu-devel] [PATCH v11 10/24] qapi: Unbox base members Eric Blake
2015-10-27 7:52 ` Markus Armbruster
2015-10-27 14:20 ` Eric Blake
2015-10-26 22:34 ` [Qemu-devel] [PATCH v11 11/24] qapi-visit: Remove redundant functions for flat union base Eric Blake
2015-10-26 22:34 ` [Qemu-devel] [PATCH v11 12/24] qapi: Start converting to new qapi union layout Eric Blake
2015-10-26 22:34 ` [Qemu-devel] [PATCH v11 13/24] qapi-visit: Convert " Eric Blake
2015-10-26 22:34 ` [Qemu-devel] [PATCH v11 14/24] tests: " Eric Blake
2015-10-26 22:34 ` [Qemu-devel] [PATCH v11 15/24] block: " Eric Blake
2015-10-26 22:34 ` [Qemu-devel] [PATCH v11 16/24] sockets: " Eric Blake
2015-10-26 22:34 ` [Qemu-devel] [PATCH v11 17/24] net: " Eric Blake
2015-10-26 22:34 ` [Qemu-devel] [PATCH v11 18/24] char: " Eric Blake
2015-10-26 22:34 ` [Qemu-devel] [PATCH v11 19/24] input: " Eric Blake
2015-10-26 22:34 ` [Qemu-devel] [PATCH v11 20/24] memory: " Eric Blake
2015-10-26 22:35 ` [Qemu-devel] [PATCH v11 21/24] tpm: " Eric Blake
2015-10-26 22:35 ` [Qemu-devel] [PATCH v11 22/24] qapi: Finish converting " Eric Blake
2015-10-27 14:33 ` Eric Blake
2015-10-27 16:01 ` Markus Armbruster
2015-10-27 16:47 ` Eric Blake
2015-10-27 16:58 ` Markus Armbruster
2015-10-26 22:35 ` [Qemu-devel] [PATCH v11 23/24] qapi: Reserve 'u' member name Eric Blake
2015-10-27 8:21 ` Markus Armbruster
2015-10-27 14:23 ` Eric Blake
2015-10-27 15:14 ` Markus Armbruster
2015-10-26 22:35 ` [Qemu-devel] [PATCH v11 24/24] qapi: Simplify gen_struct_field() Eric Blake
2015-10-27 8:39 ` [Qemu-devel] [PATCH v11 00/24] qapi collision reduction (post-introspection subset B') Markus Armbruster
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=562F877F.2080604@redhat.com \
--to=eblake@redhat.com \
--cc=armbru@redhat.com \
--cc=mdroth@linux.vnet.ibm.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).