[Qemu-devel] [PATCH v10 09/25] qapi: Prefer typesafe upcasts to qapi base classes

[Eric Blake]

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

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 <address@hidden>

---
v10: new patch
---
 scripts/qapi-types.py | 20 ++++++++++++++++++++
 ui/spice-core.c       | 14 ++++++++++----
 ui/vnc.c              |  9 ++++++---
 3 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index a97cc10..ed4a11c 100644
--- a/scripts/qapi-types.py
+++ 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)
+
+
 def gen_alternate_qtypes_decl(name):
     return mcgen('''

@@ -268,6 +285,9 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
             self.decl += gen_union(name, base, variants)
         else:
             self.decl += gen_struct(name, base, members)
+        if base:
+            # TODO Drop last param once structs have sane layout
+            self.decl += gen_upcast(name, base, not variants)
         self._gen_type_cleanup(name)

     def visit_alternate_type(self, name, info, variants):
diff --git a/ui/spice-core.c b/ui/spice-core.c
index bf4fd07..d43cfbe 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -218,9 +218,11 @@ static void channel_event(int event, SpiceChannelEventInfo 
*info)
     }

     if (info->flags & SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT) {
-        add_addr_info(client->base, (struct sockaddr *)&info->paddr_ext,
+        add_addr_info(qapi_SpiceChannel_base(client),
+                      (struct sockaddr *)&info->paddr_ext,
                       info->plen_ext);
-        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",
@@ -229,7 +231,9 @@ static void channel_event(int event, SpiceChannelEventInfo 
*info)

     switch (event) {
     case SPICE_CHANNEL_EVENT_CONNECTED:
-        qapi_event_send_spice_connected(server->base, client->base, 
&error_abort);
+        qapi_event_send_spice_connected(qapi_SpiceServerInfo_base(server),
+                                        qapi_SpiceChannel_base(client),
+                                        &error_abort);
         break;
     case SPICE_CHANNEL_EVENT_INITIALIZED:
         if (auth) {
@@ -242,7 +246,9 @@ static void channel_event(int event, SpiceChannelEventInfo 
*info)
         break;
     case SPICE_CHANNEL_EVENT_DISCONNECTED:
         channel_list_del(info);
-        qapi_event_send_spice_disconnected(server->base, client->base, 
&error_abort);
+        qapi_event_send_spice_disconnected(qapi_SpiceServerInfo_base(server),
+                                           qapi_SpiceChannel_base(client),
+                                           &error_abort);
         break;
     default:
         break;
diff --git a/ui/vnc.c b/ui/vnc.c
index 502a10a..9473b32 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -263,7 +263,8 @@ static VncServerInfo *vnc_server_info_get(VncDisplay *vd)

     info = g_malloc(sizeof(*info));
     info->base = g_malloc(sizeof(*info->base));
-    vnc_init_basic_info_from_server_addr(vd->lsock, info->base, &err);
+    vnc_init_basic_info_from_server_addr(vd->lsock,
+                                         qapi_VncServerInfo_base(info), &err);
     info->has_auth = true;
     info->auth = g_strdup(vnc_auth_name(vd));
     if (err) {
@@ -301,7 +302,8 @@ static void vnc_client_cache_addr(VncState *client)

     client->info = g_malloc0(sizeof(*client->info));
     client->info->base = g_malloc0(sizeof(*client->info->base));
-    vnc_init_basic_info_from_remote_addr(client->csock, client->info->base,
+    vnc_init_basic_info_from_remote_addr(client->csock,
+                                         qapi_VncClientInfo_base(client->info),
                                          &err);
     if (err) {
         qapi_free_VncClientInfo(client->info);
@@ -326,7 +328,8 @@ static void vnc_qmp_event(VncState *vs, QAPIEvent event)

     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);
-- 
2.4.3