qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: armbru@redhat.com, Michael Roth <mdroth@linux.vnet.ibm.com>
Subject: [Qemu-devel] [PATCH v11 09/24] qapi: Prefer typesafe upcasts to qapi base classes
Date: Mon, 26 Oct 2015 16:34:48 -0600	[thread overview]
Message-ID: <1445898903-12082-10-git-send-email-eblake@redhat.com> (raw)
In-Reply-To: <1445898903-12082-1-git-send-email-eblake@redhat.com>

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.

This patch just adds upcasts.  None of our code needed to
downcast from a base qapi class to a child.  Also, in the
case of grandchildren (such as BlockdevOptionsQcow2), the
caller will need to call two functions to get to the inner
base (although it wouldn't be too hard to generate a
qapi_FOO_base_base() if desired).  If a user changes qapi
to alter the base class hierarchy, such as going from
'A -> C' to 'A -> B -> C', it will change the type of
'qapi_C_base()', and the compiler will point out the places
that are affected by the new base.

One alternative was proposed, but was deemed too ugly to use
in practice: the generators could output redundant
information using anonymous types:
| struct Child {
|     union {
|         struct {
|             Type1 parent_member1;
|             Type2 parent_member2;
|         };
|         Parent base;
|     };
| };
With that ugly proposal, for a given qapi type, obj->member
and obj->base.member would refer to the same storage; allowing
convenience in working with members without needing 'base.'
allowing typesafe upcast without needing a C cast by accessing
'&obj->base', and allowing downcasts from the parent back to
the child possible through container_of(obj, Child, base).

Signed-off-by: Eric Blake <eblake@redhat.com>

---
v11: only support unions for this patch, add testsuite coverage
v10: new patch
---
 scripts/qapi-types.py          | 16 ++++++++++++++++
 tests/test-qmp-input-visitor.c |  5 +++++
 2 files changed, 21 insertions(+)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 40e9f79..f9fcf15 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -98,6 +98,19 @@ struct %(c_name)s {
     return ret


+def gen_upcast(name, base):
+    # C makes const-correctness ugly.  We have to cast away const to let
+    # this function work for both const and non-const obj.
+    return mcgen('''
+
+static inline %(base)s *qapi_%(c_name)s_base(const %(c_name)s *obj)
+{
+    return (%(base)s *)obj;
+}
+''',
+                 c_name=c_name(name), base=base.c_name())
+
+
 def gen_alternate_qtypes_decl(name):
     return mcgen('''

@@ -267,6 +280,9 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
         if variants:
             assert not members      # not implemented
             self.decl += gen_union(name, base, variants)
+            # TODO Use gen_upcast on structs too, once they have sane layout
+            if base:
+                self.decl += gen_upcast(name, base)
         else:
             self.decl += gen_struct(name, base, members)
         self._gen_type_cleanup(name)
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 8941963..da21709 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -347,6 +347,7 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data,
     Visitor *v;
     Error *err = NULL;
     UserDefFlatUnion *tmp;
+    UserDefUnionBase *base;

     v = visitor_input_test_init(data,
                                 "{ 'enum1': 'value1', "
@@ -360,6 +361,10 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data,
     g_assert_cmpstr(tmp->string, ==, "str");
     g_assert_cmpint(tmp->integer, ==, 41);
     g_assert_cmpint(tmp->value1->boolean, ==, true);
+
+    base = qapi_UserDefFlatUnion_base(tmp);
+    g_assert(&base->enum1 == &tmp->enum1);
+
     qapi_free_UserDefFlatUnion(tmp);
 }

-- 
2.4.3

  parent reply	other threads:[~2015-10-26 22:35 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 ` Eric Blake [this message]
2015-10-27  7:46   ` [Qemu-devel] [PATCH v11 09/24] qapi: Prefer typesafe upcasts to qapi base classes Markus Armbruster
2015-10-27 14:17     ` Eric Blake
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=1445898903-12082-10-git-send-email-eblake@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).