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 --- 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 ++++++-------- 10 files changed, 20 insertions(+), 50 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 'tests') 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, ==, -- cgit v1.2.3