From f9e6102b48f21e464a847a858a456c521e7a83e5 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Mon, 26 Oct 2015 16:34:42 -0600 Subject: qapi: More robust conditions for when labels are needed We were using regular expressions to see if ret included any earlier text that emitted a 'goto out;' line, to decide whether we needed to output an 'out:' label. But this is fragile, if the ret text can possibly combine more than one generated function body, where the first function used a goto but the second does not. Change the code to just check for the known conditions which cause an error check to be needed. Besides, it's slightly more efficient to use plain checks than regular expression searching. No change to generated code. Signed-off-by: Eric Blake Message-Id: <1445898903-12082-4-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- scripts/qapi-visit.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'scripts/qapi-visit.py') diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index d0759d739a..2dc3aed220 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -87,7 +87,8 @@ static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error **e ret += gen_visit_fields(members, prefix='(*obj)->') - if re.search('^ *goto out;', ret, re.MULTILINE): + # 'goto out' produced for base, and by gen_visit_fields() for each member + if base or members: ret += mcgen(''' out: -- cgit v1.2.3-55-g7522 From d02cf37766ba3cf918d7085aa7848c9dc05fd11a Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Mon, 26 Oct 2015 16:34:46 -0600 Subject: qapi-visit: Split off visit_type_FOO_fields forward decl We generate a static visit_type_FOO_fields() for every type FOO. However, sometimes we need a forward declaration. Split the code to generate the forward declaration out of gen_visit_implicit_struct() into a new gen_visit_fields_decl(), and also prepare for a forward declaration to be emitted during gen_visit_struct(), so that a future patch can switch from using visit_type_FOO_implicit() to the simpler visit_type_FOO_fields() as part of unboxing the base class of a struct. No change to generated code. Signed-off-by: Eric Blake Message-Id: <1445898903-12082-8-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- scripts/qapi-visit.py | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) (limited to 'scripts/qapi-visit.py') diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 2dc3aed220..d4408f25b1 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -15,7 +15,12 @@ from qapi import * import re +# visit_type_FOO_implicit() is emitted as needed; track if it has already +# been output. implicit_structs_seen = set() + +# visit_type_FOO_fields() is always emitted; track if a forward declaration +# or implementation has already been output. struct_fields_seen = set() @@ -29,19 +34,24 @@ void visit_type_%(c_name)s(Visitor *v, %(c_type)sobj, const char *name, Error ** c_name=c_name(name), c_type=c_type) -def gen_visit_implicit_struct(typ): - if typ in implicit_structs_seen: - return '' - implicit_structs_seen.add(typ) - +def gen_visit_fields_decl(typ): ret = '' if typ.name not in struct_fields_seen: - # Need a forward declaration ret += mcgen(''' static void visit_type_%(c_type)s_fields(Visitor *v, %(c_type)s **obj, Error **errp); ''', c_type=typ.c_name()) + struct_fields_seen.add(typ.name) + return ret + + +def gen_visit_implicit_struct(typ): + if typ in implicit_structs_seen: + return '' + implicit_structs_seen.add(typ) + + ret = gen_visit_fields_decl(typ) ret += mcgen(''' -- cgit v1.2.3-55-g7522 From ddf21908961073199f3d186204da4810f2ea150b Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Mon, 26 Oct 2015 16:34:49 -0600 Subject: qapi: Unbox base members 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 directly cast to its parent. This gives less malloc overhead, less pointer dereferencing, and even less generated code. Compare to the earlier commit 1e6c1616a "qapi: Generate a nicer struct for flat unions" (although that patch had fewer places to change, as less of qemu was directly using qapi structs for flat unions). It also allows us to turn on automatic type-safe wrappers for upcasting to the base class of a struct. Changes to the generated code look like this in qapi-types.h: | struct SpiceChannel { |- SpiceBasicInfo *base; |+ /* Members inherited from SpiceBasicInfo: */ |+ char *host; |+ char *port; |+ NetworkAddressFamily family; |+ /* Own members: */ | int64_t connection_id; as well as additional upcast functions like qapi_SpiceChannel_base(). Meanwhile, changes to qapi-visit.c look like: | static void visit_type_SpiceChannel_fields(Visitor *v, SpiceChannel **obj, Error **errp) | { | Error *err = NULL; | |- visit_type_implicit_SpiceBasicInfo(v, &(*obj)->base, &err); |+ visit_type_SpiceBasicInfo_fields(v, (SpiceBasicInfo **)obj, &err); | if (err) { (the cast is necessary, since our upcast wrappers only deal with a single pointer, not pointer-to-pointer); plus the wholesale elimination of some now-unused visit_type_implicit_FOO() functions. 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 'Base *base' member sufficed). And now that we no longer consume a 'base' member in the generated C struct, we can delete the former negative struct-base-clash-base test. Signed-off-by: Eric Blake Message-Id: <1445898903-12082-11-git-send-email-eblake@redhat.com> [Commit message tweaked slightly] Signed-off-by: Markus Armbruster --- hmp.c | 6 +++--- scripts/qapi-types.py | 12 ++++-------- scripts/qapi-visit.py | 9 ++++----- tests/Makefile | 1 - tests/qapi-schema/struct-base-clash-base.err | 0 tests/qapi-schema/struct-base-clash-base.exit | 1 - tests/qapi-schema/struct-base-clash-base.json | 9 --------- tests/qapi-schema/struct-base-clash-base.out | 5 ----- 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 | 13 +++++-------- tests/test-visitor-serialization.c | 14 ++++++-------- ui/spice-core.c | 23 +++++++++++++---------- ui/vnc.c | 21 ++++++++++----------- 15 files changed, 54 insertions(+), 87 deletions(-) delete mode 100644 tests/qapi-schema/struct-base-clash-base.err delete mode 100644 tests/qapi-schema/struct-base-clash-base.exit delete mode 100644 tests/qapi-schema/struct-base-clash-base.json delete mode 100644 tests/qapi-schema/struct-base-clash-base.out (limited to 'scripts/qapi-visit.py') diff --git a/hmp.c b/hmp.c index 5048eeeb2d..88fd804a9e 100644 --- a/hmp.c +++ b/hmp.c @@ -569,8 +569,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"); @@ -638,7 +638,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 f9fcf150a4..faf7022e2c 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -77,16 +77,13 @@ 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; ''') @@ -280,11 +277,10 @@ 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) + if base: + self.decl += gen_upcast(name, base) self._gen_type_cleanup(name) def visit_alternate_type(self, name, info, variants): diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index d4408f25b1..f711a720f3 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -72,13 +72,12 @@ static void visit_type_implicit_%(c_type)s(Visitor *v, %(c_type)s **obj, Error * def gen_visit_struct_fields(name, base, members): - struct_fields_seen.add(name) - ret = '' if base: - ret += gen_visit_implicit_struct(base) + ret += gen_visit_fields_decl(base) + struct_fields_seen.add(name) ret += mcgen(''' static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error **errp) @@ -90,9 +89,9 @@ static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error **e if base: ret += mcgen(''' - visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err); + visit_type_%(c_type)s_fields(v, (%(c_type)s **)obj, &err); ''', - c_type=base.c_name(), c_name=c_name('base')) + c_type=base.c_name()) ret += gen_err_check() ret += gen_visit_fields(members, prefix='(*obj)->') diff --git a/tests/Makefile b/tests/Makefile index 652294cccd..fd4ec03e09 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -325,7 +325,6 @@ qapi-schema += returns-array-bad.json qapi-schema += returns-dict.json qapi-schema += returns-unknown.json qapi-schema += returns-whitelist.json -qapi-schema += struct-base-clash-base.json qapi-schema += struct-base-clash-deep.json qapi-schema += struct-base-clash.json qapi-schema += struct-data-invalid.json diff --git a/tests/qapi-schema/struct-base-clash-base.err b/tests/qapi-schema/struct-base-clash-base.err deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/tests/qapi-schema/struct-base-clash-base.exit b/tests/qapi-schema/struct-base-clash-base.exit deleted file mode 100644 index 573541ac97..0000000000 --- a/tests/qapi-schema/struct-base-clash-base.exit +++ /dev/null @@ -1 +0,0 @@ -0 diff --git a/tests/qapi-schema/struct-base-clash-base.json b/tests/qapi-schema/struct-base-clash-base.json deleted file mode 100644 index 0c840258c9..0000000000 --- a/tests/qapi-schema/struct-base-clash-base.json +++ /dev/null @@ -1,9 +0,0 @@ -# Struct member 'base' -# FIXME: this parses, but then fails to compile due to a duplicate 'base' -# (one explicit in QMP, the other used to box the base class members). -# We should either reject the collision at parse time, or change the -# generated struct to allow this to compile. -{ 'struct': 'Base', 'data': {} } -{ 'struct': 'Sub', - 'base': 'Base', - 'data': { 'base': 'str' } } diff --git a/tests/qapi-schema/struct-base-clash-base.out b/tests/qapi-schema/struct-base-clash-base.out deleted file mode 100644 index e69a416560..0000000000 --- a/tests/qapi-schema/struct-base-clash-base.out +++ /dev/null @@ -1,5 +0,0 @@ -object :empty -object Base -object Sub - base Base - member base: str optional=False diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c index bc59835835..ea700d890d 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 28f146d4b7..035c65cfdf 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 da21709714..2d95db94e6 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 c84002e2f2..cfb06bbbc6 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,8 @@ 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 = {0}; + UserDefOne *pu = &u; Error *err; int i; @@ -416,8 +414,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 fa86cae88a..634563bae4 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 bf4fd07499..6a62d712fe 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(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 +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(qapi_SpiceServerInfo_base(server), + qapi_SpiceChannel_base(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(qapi_SpiceServerInfo_base(server), + qapi_SpiceChannel_base(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 502a10a07b..cec2cee993 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -262,8 +262,8 @@ static VncServerInfo *vnc_server_info_get(VncDisplay *vd) Error *err = NULL; 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) { @@ -300,8 +300,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_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); @@ -317,7 +317,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) { @@ -326,7 +325,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); @@ -361,11 +361,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; if (client->tls) { info->x509_dname = qcrypto_tls_session_get_peer_name(client->tls); -- cgit v1.2.3-55-g7522 From 5c5e51a05b567fd48fb155d94ca6f7679dd0d478 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Mon, 26 Oct 2015 16:34:50 -0600 Subject: qapi-visit: Remove redundant functions for flat union base The code for visiting the base class of a child struct created visit_type_Base_fields() which covers all fields of Base; while the code for visiting the base class of a flat union created visit_type_Union_fields() covering all fields of the base except the discriminator. But since the base class includes the discriminator of a flat union, we can just visit the entire base, without needing a separate visit of the discriminator. Not only is consistently visiting all fields easier to understand, it lets us share code. The generated code in qapi-visit.c loses several now-unused visit_type_UNION_fields(), along with changes like: |@@ -1654,11 +1557,7 @@ void visit_type_BlockdevOptions(Visitor | if (!*obj) { | goto out_obj; | } |- visit_type_BlockdevOptions_fields(v, obj, &err); |- if (err) { |- goto out_obj; |- } |- visit_type_BlockdevDriver(v, &(*obj)->driver, "driver", &err); |+ visit_type_BlockdevOptionsBase_fields(v, (BlockdevOptionsBase **)obj, &err); | if (err) { | goto out_obj; | } and forward declarations where needed. Note that the cast of obj to BASE ** is necessary to call visit_type_BASE_fields() (and we can't use our upcast wrappers, because those work on pointers while we have a pointer-to-pointer). Signed-off-by: Eric Blake Message-Id: <1445898903-12082-12-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- scripts/qapi-visit.py | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) (limited to 'scripts/qapi-visit.py') diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index f711a720f3..33c013a7fb 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -228,8 +228,7 @@ def gen_visit_union(name, base, variants): ret = '' if base: - members = [m for m in base.members if m != variants.tag_member] - ret += gen_visit_struct_fields(name, None, members) + ret += gen_visit_fields_decl(base) for var in variants.variants: # Ugly special case for simple union TODO get rid of it @@ -254,31 +253,30 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error if base: ret += mcgen(''' - visit_type_%(c_name)s_fields(v, obj, &err); + visit_type_%(c_name)s_fields(v, (%(c_name)s **)obj, &err); ''', - c_name=c_name(name)) - ret += gen_err_check(label='out_obj') - - tag_key = variants.tag_member.name - if not variants.tag_name: - # we pointlessly use a different key for simple unions - tag_key = 'type' - ret += mcgen(''' + c_name=base.c_name()) + else: + ret += mcgen(''' visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err); - if (err) { - goto out_obj; - } +''', + c_type=variants.tag_member.type.c_name(), + # TODO ugly special case for simple union + # Use same tag name in C as on the wire to get rid of + # it, then: c_name=c_name(variants.tag_member.name) + c_name='kind', + name=variants.tag_member.name) + ret += gen_err_check(label='out_obj') + ret += mcgen(''' if (!visit_start_union(v, !!(*obj)->data, &err) || err) { goto out_obj; } switch ((*obj)->%(c_name)s) { ''', - c_type=variants.tag_member.type.c_name(), # TODO ugly special case for simple union # Use same tag name in C as on the wire to get rid of # it, then: c_name=c_name(variants.tag_member.name) - c_name=c_name(variants.tag_name or 'kind'), - name=tag_key) + c_name=c_name(variants.tag_name or 'kind')) for var in variants.variants: # TODO ugly special case for simple union -- cgit v1.2.3-55-g7522 From 150d0564a4c626642897c748f7906260a13c14e1 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Mon, 26 Oct 2015 16:34:52 -0600 Subject: qapi-visit: Convert to new qapi union layout We have two issues with our qapi union layout: 1) Even though the QMP wire format spells the tag 'type', the C code spells it 'kind', requiring some hacks in the generator. 2) The C struct uses an anonymous union, which places all tag values in the same namespace as all non-variant members. This leads to spurious collisions if a tag value matches a non-variant member's name. Make the conversion to the new layout for qapi-visit.py. Generated code changes look like: |@@ -4912,16 +4912,16 @@ void visit_type_MemoryDeviceInfo(Visitor | if (!*obj) { | goto out_obj; | } |- visit_type_MemoryDeviceInfoKind(v, &(*obj)->kind, "type", &err); |+ visit_type_MemoryDeviceInfoKind(v, &(*obj)->type, "type", &err); | if (err) { | goto out_obj; | } |- if (!visit_start_union(v, !!(*obj)->data, &err) || err) { |+ if (!visit_start_union(v, !!(*obj)->u.data, &err) || err) { | goto out_obj; | } |- switch ((*obj)->kind) { |+ switch ((*obj)->type) { | case MEMORY_DEVICE_INFO_KIND_DIMM: |- visit_type_PCDIMMDeviceInfo(v, &(*obj)->dimm, "data", &err); |+ visit_type_PCDIMMDeviceInfo(v, &(*obj)->u.dimm, "data", &err); | break; | default: | abort(); |@@ -4930,7 +4930,7 @@ out_obj: | error_propagate(errp, err); | err = NULL; | if (*obj) { |- visit_end_union(v, !!(*obj)->data, &err); |+ visit_end_union(v, !!(*obj)->u.data, &err); | } | error_propagate(errp, err); | err = NULL; Signed-off-by: Eric Blake Message-Id: <1445898903-12082-14-git-send-email-eblake@redhat.com> [Commit message tweaked slightly] Signed-off-by: Markus Armbruster --- scripts/qapi-visit.py | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) (limited to 'scripts/qapi-visit.py') diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 33c013a7fb..f40c3c792f 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -189,18 +189,18 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error if (err) { goto out; } - visit_get_next_type(v, (int*) &(*obj)->kind, %(c_name)s_qtypes, name, &err); + visit_get_next_type(v, (int*) &(*obj)->type, %(c_name)s_qtypes, name, &err); if (err) { goto out_obj; } - switch ((*obj)->kind) { + switch ((*obj)->type) { ''', c_name=c_name(name)) for var in variants.variants: ret += mcgen(''' case %(case)s: - visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, name, &err); + visit_type_%(c_type)s(v, &(*obj)->u.%(c_name)s, name, &err); break; ''', case=c_enum_const(variants.tag_member.type.name, @@ -261,22 +261,16 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err); ''', c_type=variants.tag_member.type.c_name(), - # TODO ugly special case for simple union - # Use same tag name in C as on the wire to get rid of - # it, then: c_name=c_name(variants.tag_member.name) - c_name='kind', + c_name=c_name(variants.tag_member.name), name=variants.tag_member.name) ret += gen_err_check(label='out_obj') ret += mcgen(''' - if (!visit_start_union(v, !!(*obj)->data, &err) || err) { + if (!visit_start_union(v, !!(*obj)->u.data, &err) || err) { goto out_obj; } switch ((*obj)->%(c_name)s) { ''', - # TODO ugly special case for simple union - # Use same tag name in C as on the wire to get rid of - # it, then: c_name=c_name(variants.tag_member.name) - c_name=c_name(variants.tag_name or 'kind')) + c_name=c_name(variants.tag_member.name)) for var in variants.variants: # TODO ugly special case for simple union @@ -288,13 +282,13 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error var.name)) if simple_union_type: ret += mcgen(''' - visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "data", &err); + visit_type_%(c_type)s(v, &(*obj)->u.%(c_name)s, "data", &err); ''', c_type=simple_union_type.c_name(), c_name=c_name(var.name)) else: ret += mcgen(''' - visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err); + visit_type_implicit_%(c_type)s(v, &(*obj)->u.%(c_name)s, &err); ''', c_type=var.type.c_name(), c_name=c_name(var.name)) @@ -310,7 +304,7 @@ out_obj: error_propagate(errp, err); err = NULL; if (*obj) { - visit_end_union(v, !!(*obj)->data, &err); + visit_end_union(v, !!(*obj)->u.data, &err); } error_propagate(errp, err); err = NULL; -- cgit v1.2.3-55-g7522