diff mbox

[RFC,v2,03/12] qapi: Unbox base members

Message ID 1438919561-27750-4-git-send-email-eblake@redhat.com
State New
Headers show

Commit Message

Eric Blake Aug. 7, 2015, 3:52 a.m. UTC
Rather than storing a base class as a pointer to a box, just
store the fields of that base class in the same order, so that
a child struct can be safely cast to its parent.  This gives
less malloc overhead, less pointer dereferencing, and even less
generated code.

Without boxing, the corner case of one empty struct having
another empty struct as its base type now requires inserting a
dummy member (previously, the pointer to the base would have
sufficed).

Compare to the earlier commit "qapi: Generate a nicer struct for
flat unions".

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 hmp.c                              |  6 +++---
 scripts/qapi-types.py              | 29 +++++++++++++++--------------
 scripts/qapi-visit.py              |  9 ++++++---
 tests/test-qmp-commands.c          | 15 +++++----------
 tests/test-qmp-event.c             |  8 ++------
 tests/test-qmp-input-visitor.c     |  4 ++--
 tests/test-qmp-output-visitor.c    | 12 ++++--------
 tests/test-visitor-serialization.c | 14 ++++++--------
 ui/spice-core.c                    | 23 +++++++++++++----------
 ui/vnc.c                           | 20 +++++++++-----------
 10 files changed, 65 insertions(+), 75 deletions(-)
diff mbox

Patch

diff --git a/hmp.c b/hmp.c
index f8fb72f..9f202df 100644
--- a/hmp.c
+++ b/hmp.c
@@ -558,8 +558,8 @@  void hmp_info_vnc(Monitor *mon, const QDict *qdict)
         for (client = info->clients; client; client = client->next) {
             monitor_printf(mon, "Client:\n");
             monitor_printf(mon, "     address: %s:%s\n",
-                           client->value->base->host,
-                           client->value->base->service);
+                           client->value->host,
+                           client->value->service);
             monitor_printf(mon, "  x509_dname: %s\n",
                            client->value->x509_dname ?
                            client->value->x509_dname : "none");
@@ -627,7 +627,7 @@  void hmp_info_spice(Monitor *mon, const QDict *qdict)
         for (chan = info->channels; chan; chan = chan->next) {
             monitor_printf(mon, "Channel:\n");
             monitor_printf(mon, "     address: %s:%s%s\n",
-                           chan->value->base->host, chan->value->base->port,
+                           chan->value->host, chan->value->port,
                            chan->value->tls ? " [tls]" : "");
             monitor_printf(mon, "     session: %" PRId64 "\n",
                            chan->value->connection_id);
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 2d09b90..27ad0c1 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -47,8 +47,19 @@  def gen_struct_field(name, typ, optional):
                  c_type=typ.c_type(), c_name=c_name(name))
     return ret

-def gen_struct_fields(members):
+def gen_struct_fields(members, base, nested=False):
     ret = ''
+    if base:
+        if not nested:
+            ret += mcgen('''
+    /* Members inherited from %(c_name)s: */
+''',
+                         c_name=base.c_name())
+        ret += gen_struct_fields(base.local_members, base.base, True)
+        if not nested:
+            ret += mcgen('''
+    /* Own members: */
+''')

     for memb in members:
         ret += gen_struct_field(memb.name, memb.type, memb.optional)
@@ -61,15 +72,12 @@  struct %(c_name)s {
 ''',
                 c_name=c_name(name))

-    if base:
-        ret += gen_struct_field('base', base, False)
-
-    ret += gen_struct_fields(members)
+    ret += gen_struct_fields(members, base)

     # Make sure that all structs have at least one field; this avoids
     # potential issues with attempting to malloc space for zero-length structs
     # in C, and also incompatibility with C++ (where an empty struct is size 1).
-    if not base and not members:
+    if not (base and base.members) and not members:
         ret += mcgen('''
     char qapi_dummy_field_for_empty_struct;
 ''')
@@ -117,14 +125,7 @@  struct %(c_name)s {
 ''',
                 c_name=c_name(name))
     if base:
-        ret += mcgen('''
-    /* Members inherited from %(c_name)s: */
-''',
-                     c_name=c_name(base.name))
-        ret += gen_struct_fields(base.members)
-        ret += mcgen('''
-    /* Own members: */
-''')
+        ret += gen_struct_fields([], base)
     else:
         ret += mcgen('''
     %(c_type)s type;
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index eb96e36..ef96345 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -59,12 +59,15 @@  static void visit_type_implicit_%(c_type)s(Visitor *m, %(c_type)s **obj, Error *
     return ret

 def gen_visit_struct_fields(name, base, members):
+    if name in struct_fields_seen:
+        return ''
     struct_fields_seen.add(name)

     ret = ''

     if base:
-        ret += gen_visit_implicit_struct(base)
+        ret += gen_visit_struct_fields(base.name, base.base,
+                                       base.local_members)

     ret += mcgen('''

@@ -78,12 +81,12 @@  static void visit_type_%(c_name)s_fields(Visitor *m, %(c_name)s **obj, Error **e

     if base:
         ret += mcgen('''
-visit_type_implicit_%(c_type)s(m, &(*obj)->%(c_name)s, &err);
+visit_type_%(c_type)s_fields(m, (%(c_type)s **)obj, &err);
 if (err) {
     goto out;
 }
 ''',
-                     c_type=base.c_name(), c_name=c_name('base'))
+                     c_type=base.c_name())

     for memb in members:
         if memb.optional:
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 6a67e73..5181823 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -25,11 +25,9 @@  UserDefTwo *qmp_user_def_cmd2(UserDefOne *ud1a,
     UserDefOne *ud1d = g_malloc0(sizeof(UserDefOne));

     ud1c->string = strdup(ud1a->string);
-    ud1c->base = g_new0(UserDefZero, 1);
-    ud1c->base->integer = ud1a->base->integer;
+    ud1c->integer = ud1a->integer;
     ud1d->string = strdup(has_udb1 ? ud1b->string : "blah0");
-    ud1d->base = g_new0(UserDefZero, 1);
-    ud1d->base->integer = has_udb1 ? ud1b->base->integer : 0;
+    ud1d->integer = has_udb1 ? ud1b->integer : 0;

     ret = g_new0(UserDefTwo, 1);
     ret->string0 = strdup("blah1");
@@ -176,20 +174,17 @@  static void test_dealloc_types(void)
     UserDefOneList *ud1list;

     ud1test = g_malloc0(sizeof(UserDefOne));
-    ud1test->base = g_new0(UserDefZero, 1);
-    ud1test->base->integer = 42;
+    ud1test->integer = 42;
     ud1test->string = g_strdup("hi there 42");

     qapi_free_UserDefOne(ud1test);

     ud1a = g_malloc0(sizeof(UserDefOne));
-    ud1a->base = g_new0(UserDefZero, 1);
-    ud1a->base->integer = 43;
+    ud1a->integer = 43;
     ud1a->string = g_strdup("hi there 43");

     ud1b = g_malloc0(sizeof(UserDefOne));
-    ud1b->base = g_new0(UserDefZero, 1);
-    ud1b->base->integer = 44;
+    ud1b->integer = 44;
     ud1b->string = g_strdup("hi there 44");

     ud1list = g_malloc0(sizeof(UserDefOneList));
diff --git a/tests/test-qmp-event.c b/tests/test-qmp-event.c
index 28f146d..035c65c 100644
--- a/tests/test-qmp-event.c
+++ b/tests/test-qmp-event.c
@@ -179,9 +179,7 @@  static void test_event_c(TestEventData *data,
     QDict *d, *d_data, *d_b;

     UserDefOne b;
-    UserDefZero z;
-    z.integer = 2;
-    b.base = &z;
+    b.integer = 2;
     b.string = g_strdup("test1");
     b.has_enum1 = false;

@@ -209,11 +207,9 @@  static void test_event_d(TestEventData *data,
 {
     UserDefOne struct1;
     EventStructOne a;
-    UserDefZero z;
     QDict *d, *d_data, *d_a, *d_struct1;

-    z.integer = 2;
-    struct1.base = &z;
+    struct1.integer = 2;
     struct1.string = g_strdup("test1");
     struct1.has_enum1 = true;
     struct1.enum1 = ENUM_ONE_VALUE1;
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index da84993..b6deee3 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -262,7 +262,7 @@  static void test_visitor_in_struct_nested(TestInputVisitorData *data,

     check_and_free_str(udp->string0, "string0");
     check_and_free_str(udp->dict1->string1, "string1");
-    g_assert_cmpint(udp->dict1->dict2->userdef->base->integer, ==, 42);
+    g_assert_cmpint(udp->dict1->dict2->userdef->integer, ==, 42);
     check_and_free_str(udp->dict1->dict2->userdef->string, "string");
     check_and_free_str(udp->dict1->dict2->string, "string2");
     g_assert(udp->dict1->has_dict3 == false);
@@ -292,7 +292,7 @@  static void test_visitor_in_list(TestInputVisitorData *data,

         snprintf(string, sizeof(string), "string%d", i);
         g_assert_cmpstr(item->value->string, ==, string);
-        g_assert_cmpint(item->value->base->integer, ==, 42 + i);
+        g_assert_cmpint(item->value->integer, ==, 42 + i);
     }

     qapi_free_UserDefOneList(head);
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index 2162a4a..ab4bc5d 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -250,16 +250,14 @@  static void test_visitor_out_struct_nested(TestOutputVisitorData *data,
     ud2->dict1->dict2 = g_malloc0(sizeof(*ud2->dict1->dict2));
     ud2->dict1->dict2->userdef = g_new0(UserDefOne, 1);
     ud2->dict1->dict2->userdef->string = g_strdup(string);
-    ud2->dict1->dict2->userdef->base = g_new0(UserDefZero, 1);
-    ud2->dict1->dict2->userdef->base->integer = value;
+    ud2->dict1->dict2->userdef->integer = value;
     ud2->dict1->dict2->string = g_strdup(strings[2]);

     ud2->dict1->dict3 = g_malloc0(sizeof(*ud2->dict1->dict3));
     ud2->dict1->has_dict3 = true;
     ud2->dict1->dict3->userdef = g_new0(UserDefOne, 1);
     ud2->dict1->dict3->userdef->string = g_strdup(string);
-    ud2->dict1->dict3->userdef->base = g_new0(UserDefZero, 1);
-    ud2->dict1->dict3->userdef->base->integer = value;
+    ud2->dict1->dict3->userdef->integer = value;
     ud2->dict1->dict3->string = g_strdup(strings[3]);

     visit_type_UserDefTwo(data->ov, &ud2, "unused", &err);
@@ -301,8 +299,7 @@  static void test_visitor_out_struct_errors(TestOutputVisitorData *data,
                                            const void *unused)
 {
     EnumOne bad_values[] = { ENUM_ONE_MAX, -1 };
-    UserDefZero b;
-    UserDefOne u = { .base = &b }, *pu = &u;
+    UserDefOne u, *pu = &u;
     Error *err;
     int i;

@@ -416,8 +413,7 @@  static void test_visitor_out_list_qapi_free(TestOutputVisitorData *data,
         p->value->dict1->dict2 = g_new0(UserDefTwoDictDict, 1);
         p->value->dict1->dict2->userdef = g_new0(UserDefOne, 1);
         p->value->dict1->dict2->userdef->string = g_strdup(string);
-        p->value->dict1->dict2->userdef->base = g_new0(UserDefZero, 1);
-        p->value->dict1->dict2->userdef->base->integer = 42;
+        p->value->dict1->dict2->userdef->integer = 42;
         p->value->dict1->dict2->string = g_strdup(string);
         p->value->dict1->has_dict3 = false;

diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index fa86cae..634563b 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -258,15 +258,13 @@  static UserDefTwo *nested_struct_create(void)
     udnp->dict1->string1 = strdup("test_string1");
     udnp->dict1->dict2 = g_malloc0(sizeof(*udnp->dict1->dict2));
     udnp->dict1->dict2->userdef = g_new0(UserDefOne, 1);
-    udnp->dict1->dict2->userdef->base = g_new0(UserDefZero, 1);
-    udnp->dict1->dict2->userdef->base->integer = 42;
+    udnp->dict1->dict2->userdef->integer = 42;
     udnp->dict1->dict2->userdef->string = strdup("test_string");
     udnp->dict1->dict2->string = strdup("test_string2");
     udnp->dict1->dict3 = g_malloc0(sizeof(*udnp->dict1->dict3));
     udnp->dict1->has_dict3 = true;
     udnp->dict1->dict3->userdef = g_new0(UserDefOne, 1);
-    udnp->dict1->dict3->userdef->base = g_new0(UserDefZero, 1);
-    udnp->dict1->dict3->userdef->base->integer = 43;
+    udnp->dict1->dict3->userdef->integer = 43;
     udnp->dict1->dict3->userdef->string = strdup("test_string");
     udnp->dict1->dict3->string = strdup("test_string3");
     return udnp;
@@ -278,15 +276,15 @@  static void nested_struct_compare(UserDefTwo *udnp1, UserDefTwo *udnp2)
     g_assert(udnp2);
     g_assert_cmpstr(udnp1->string0, ==, udnp2->string0);
     g_assert_cmpstr(udnp1->dict1->string1, ==, udnp2->dict1->string1);
-    g_assert_cmpint(udnp1->dict1->dict2->userdef->base->integer, ==,
-                    udnp2->dict1->dict2->userdef->base->integer);
+    g_assert_cmpint(udnp1->dict1->dict2->userdef->integer, ==,
+                    udnp2->dict1->dict2->userdef->integer);
     g_assert_cmpstr(udnp1->dict1->dict2->userdef->string, ==,
                     udnp2->dict1->dict2->userdef->string);
     g_assert_cmpstr(udnp1->dict1->dict2->string, ==,
                     udnp2->dict1->dict2->string);
     g_assert(udnp1->dict1->has_dict3 == udnp2->dict1->has_dict3);
-    g_assert_cmpint(udnp1->dict1->dict3->userdef->base->integer, ==,
-                    udnp2->dict1->dict3->userdef->base->integer);
+    g_assert_cmpint(udnp1->dict1->dict3->userdef->integer, ==,
+                    udnp2->dict1->dict3->userdef->integer);
     g_assert_cmpstr(udnp1->dict1->dict3->userdef->string, ==,
                     udnp2->dict1->dict3->userdef->string);
     g_assert_cmpstr(udnp1->dict1->dict3->string, ==,
diff --git a/ui/spice-core.c b/ui/spice-core.c
index bf4fd07..86f4441 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -200,8 +200,6 @@  static void channel_event(int event, SpiceChannelEventInfo *info)
 {
     SpiceServerInfo *server = g_malloc0(sizeof(*server));
     SpiceChannel *client = g_malloc0(sizeof(*client));
-    server->base = g_malloc0(sizeof(*server->base));
-    client->base = g_malloc0(sizeof(*client->base));

     /*
      * Spice server might have called us from spice worker thread
@@ -218,9 +216,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((SpiceBasicInfo *)client,
+                      (struct sockaddr *)&info->paddr_ext,
                       info->plen_ext);
-        add_addr_info(server->base, (struct sockaddr *)&info->laddr_ext,
+        add_addr_info((SpiceBasicInfo *)server,
+                      (struct sockaddr *)&info->laddr_ext,
                       info->llen_ext);
     } else {
         error_report("spice: %s, extended address is expected",
@@ -229,7 +229,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((SpiceBasicInfo *)server,
+                                        (SpiceBasicInfo *)client,
+                                        &error_abort);
         break;
     case SPICE_CHANNEL_EVENT_INITIALIZED:
         if (auth) {
@@ -242,7 +244,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((SpiceBasicInfo *)server,
+                                           (SpiceBasicInfo *)client,
+                                           &error_abort);
         break;
     default:
         break;
@@ -378,16 +382,15 @@  static SpiceChannelList *qmp_query_spice_channels(void)

         chan = g_malloc0(sizeof(*chan));
         chan->value = g_malloc0(sizeof(*chan->value));
-        chan->value->base = g_malloc0(sizeof(*chan->value->base));

         paddr = (struct sockaddr *)&item->info->paddr_ext;
         plen = item->info->plen_ext;
         getnameinfo(paddr, plen,
                     host, sizeof(host), port, sizeof(port),
                     NI_NUMERICHOST | NI_NUMERICSERV);
-        chan->value->base->host = g_strdup(host);
-        chan->value->base->port = g_strdup(port);
-        chan->value->base->family = inet_netfamily(paddr->sa_family);
+        chan->value->host = g_strdup(host);
+        chan->value->port = g_strdup(port);
+        chan->value->family = inet_netfamily(paddr->sa_family);

         chan->value->connection_id = item->info->connection_id;
         chan->value->channel_type = item->info->type;
diff --git a/ui/vnc.c b/ui/vnc.c
index 273161c..ab76039 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -262,8 +262,7 @@  static VncServerInfo *vnc_server_info_get(VncDisplay *vd)
     VncServerInfo *info;

     info = g_malloc(sizeof(*info));
-    info->base = g_malloc(sizeof(*info->base));
-    vnc_basic_info_get_from_server_addr(vd->lsock, info->base, NULL);
+    vnc_basic_info_get_from_server_addr(vd->lsock, (VncBasicInfo *)info, NULL);
     info->has_auth = true;
     info->auth = g_strdup(vnc_auth_name(vd));
     return info;
@@ -295,8 +294,8 @@  static void vnc_client_cache_addr(VncState *client)
 {
     Error *err = NULL;
     client->info = g_malloc0(sizeof(*client->info));
-    client->info->base = g_malloc0(sizeof(*client->info->base));
-    vnc_basic_info_get_from_remote_addr(client->csock, client->info->base,
+    vnc_basic_info_get_from_remote_addr(client->csock,
+                                        (VncBasicInfo *)client->info,
                                         &err);
     if (err) {
         qapi_free_VncClientInfo(client->info);
@@ -312,7 +311,6 @@  static void vnc_qmp_event(VncState *vs, QAPIEvent event)
     if (!vs->info) {
         return;
     }
-    g_assert(vs->info->base);

     si = vnc_server_info_get(vs->vd);
     if (!si) {
@@ -321,7 +319,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, (VncBasicInfo *)vs->info,
+                                      &error_abort);
         break;
     case QAPI_EVENT_VNC_INITIALIZED:
         qapi_event_send_vnc_initialized(si, vs->info, &error_abort);
@@ -356,11 +355,10 @@  static VncClientInfo *qmp_query_vnc_client(const VncState *client)
     }

     info = g_malloc0(sizeof(*info));
-    info->base = g_malloc0(sizeof(*info->base));
-    info->base->host = g_strdup(host);
-    info->base->service = g_strdup(serv);
-    info->base->family = inet_netfamily(sa.ss_family);
-    info->base->websocket = client->websocket;
+    info->host = g_strdup(host);
+    info->service = g_strdup(serv);
+    info->family = inet_netfamily(sa.ss_family);
+    info->websocket = client->websocket;

 #ifdef CONFIG_VNC_TLS
     if (client->tls.session && client->tls.dname) {