qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
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 --]

  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).