| Message ID | 1455884286-26272-15-git-send-email-armbru@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Feb 19, 2016 at 01:18:05PM +0100, Markus Armbruster wrote: > From: Eric Blake <eblake@redhat.com> > > There's no reason to do two malloc's for a flat union; let's just > inline the branch struct directly into the C union branch of the > flat union. > > Surprisingly, fewer clients were actually using explicit references > to the branch types in comparison to the number of flat unions > thus modified. > > This lets us reduce the hack in qapi-types:gen_variants() added in > the previous patch; we no longer need to distinguish between > alternates and flat unions. > > The change to unboxed structs means that u.data (added in commit > cee2dedb) is now coincident with random fields of each branch of > the flat union, whereas beforehand it was only coincident with > pointers (since all branches of a flat union have to be objects). > Note that this was already the case for simple unions - but there > we got lucky. Remember, visit_start_union() blindly returns true > for all visitors except for the dealloc visitor, where it returns > the value !!obj->u.data, and that this result then controls > whether to proceed with the visit to the variant. Pre-patch, > this meant that flat unions were testing whether the boxed pointer > was still NULL, and thereby skipping visit_end_implicit_struct() > and avoiding a NULL dereference if the pointer had not been > allocated. The same was true for simple unions where the current > branch had pointer type, except there we bypassed visit_type_FOO(). > But for simple unions where the current branch had scalar type, the > contents of that scalar meant that the decision to call > visit_type_FOO() was data-dependent - the reason we got lucky there > is that visit_type_FOO() for all scalar types in the dealloc visitor > is a no-op (only the pointer variants had anything to free), so it > did not matter whether the dealloc visit was skipped. But with this > patch, we would risk leaking memory if we could skip a call to > visit_type_FOO_fields() based solely on a data-dependent decision. > > But notice: in the dealloc visitor, visit_type_FOO() already handles > a NULL obj - it was only the visit_type_implicit_FOO() that was > failing to check for NULL. And now that we have refactored things to > have the branch be part of the parent struct, we no longer have a > separate pointer that can be NULL in the first place. So we can just > delete the call to visit_start_union() altogether, and blindly visit > the branch type; there is no change in behavior except to the dealloc > visitor, where we now unconditionally visit the branch, but where that > visit is now always safe (for a flat union, we can no longer > dereference NULL, and for a simple union, visit_type_FOO() was already > safely handling NULL on pointer types). > > Unfortunately, simple unions are not as easy to switch to unboxed > layout; because we are special-casing the hidden implicit type with > a single 'data' member, we really DO need to keep calling another > layer of visit_start_struct(), with a second malloc; although there > are some cleanups planned for simple unions in later patches. > > visit_start_union() and gen_visit_implicit_struct() are now unused. > Drop them. > > Note that after this patch, the only remaining use of > visit_start_implicit_struct() is for alternate types; the next patch > will do further cleanup based on that fact. > > Signed-off-by: Eric Blake <eblake@redhat.com> > Message-Id: <1455778109-6278-14-git-send-email-eblake@redhat.com> > [Dead code deletion squashed in, commit message updated accordingly] > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > cpus.c | 18 ++++++------------ > hmp.c | 12 ++++++------ > include/qapi/visitor-impl.h | 2 -- > include/qapi/visitor.h | 1 - > qapi/qapi-dealloc-visitor.c | 26 -------------------------- > qapi/qapi-visit-core.c | 8 -------- > scripts/qapi-types.py | 13 +++---------- > scripts/qapi-visit.py | 36 ++---------------------------------- > tests/test-qmp-input-visitor.c | 10 +++++----- > tests/test-qmp-output-visitor.c | 6 ++---- > 10 files changed, 24 insertions(+), 108 deletions(-) This change has broken my pending LUKS encryption patch series[1], and I'm not entirely sure of the best way to resolve it. I'll simplify the schema slighty for sake of illustration here. Consider I have the following: { 'struct': 'QCryptoBlockOptionsBase', 'data': { 'format': 'QCryptoBlockFormat' }} { 'struct': 'QCryptoBlockOptionsQCow', 'data': { '*key-secret': 'str' }} { 'struct': 'QCryptoBlockOptionsLUKS', 'data': { '*key-secret': 'str', '*cipher-alg': 'QCryptoCipherAlgorithm', '*cipher-mode': 'QCryptoCipherMode', '*ivgen-alg': 'QCryptoIVGenAlgorithm', '*ivgen-hash-alg': 'QCryptoHashAlgorithm', '*hash-alg': 'QCryptoHashAlgorithm'}} { 'union': 'QCryptoBlockOptions', 'base': 'QCryptoBlockOptionsBase', 'discriminator': 'format', 'data': { 'qcow': 'QCryptoBlockOptionsQCow', 'luks': 'QCryptoBlockOptionsLUKS' } } If I want to visit QCryptoBlockOptions, allocating a new struct instance, I can still do so QCryptoBlockOptions *opts = NULL; visit_type_QCryptoBlockOptions(v, "block", &opts, errp) Similarly, if I want to visit QCryptoBlockOptionsLUKS, allocating a new struct instance, I can still do so QCryptoBlockOptionsLUKS *opts = NULL; visit_type_QCryptoBlockOptionsLUKS(v, "luks", &opts, errp) My code needs todo something a little different though - it needs to directly visit one of the union branches. Previously, I could allocate a QCryptoBlockOptions and then visit a specific union branch like this: QCryptoBlockOptions *opts = NULL; opts = g_new0(QCryptoBlockOptions, 1); opts.format = Q_CRYPTO_BLOCK_FORMAT_LUKS visit_type_QCryptoBlockOptionsLUKS(v, "luks", &opts.u.luks, errp) I need todo this, because my visitor instance does not have any 'format' field to visit - we know the format upfront from the block layer driver choice. So we need to directly visit the appropriate inline union branch. This no longer works, because the opts.u.luks field is now inlined instead of being boxed :-( I could visit_type_QCryptoBlockOptionsLUKS_fields(v, opts.u.luks, errp) but the '_fields' methods are all declared static in qapi-visit.c preventing their use. IMHO, now that QAPI inlines the flat unions, we should be making the _fields() methods public. Regards, Daniel [1] https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg03176.html
On 02/23/2016 07:50 AM, Daniel P. Berrange wrote: > On Fri, Feb 19, 2016 at 01:18:05PM +0100, Markus Armbruster wrote: >> From: Eric Blake <eblake@redhat.com> >> >> There's no reason to do two malloc's for a flat union; let's just >> inline the branch struct directly into the C union branch of the >> flat union. >> > > My code needs todo something a little different though - it needs > to directly visit one of the union branches. Previously, I could > allocate a QCryptoBlockOptions and then visit a specific union > branch like this: > > QCryptoBlockOptions *opts = NULL; > > opts = g_new0(QCryptoBlockOptions, 1); > opts.format = Q_CRYPTO_BLOCK_FORMAT_LUKS > > visit_type_QCryptoBlockOptionsLUKS(v, "luks", &opts.u.luks, errp) which allocated a pointer and visited the fields. We no longer need the pointer, but still need the fields visited. > > > I need todo this, because my visitor instance does not have any > 'format' field to visit - we know the format upfront from the > block layer driver choice. So we need to directly visit the > appropriate inline union branch. This no longer works, because > the opts.u.luks field is now inlined instead of being boxed :-( You're using OptsVisitor, right? Is it possible to hack the QemuOpts that you feed to opts_visitor_new() to include a 'format' field? > > I could visit_type_QCryptoBlockOptionsLUKS_fields(v, opts.u.luks, errp) > but the '_fields' methods are all declared static in qapi-visit.c > preventing their use. > > IMHO, now that QAPI inlines the flat unions, we should be making > the _fields() methods public. But my suggestion would just be a hack. Yours makes more sense in the long run, so I'll go ahead and propose that patch.
On 02/23/2016 08:18 AM, Eric Blake wrote: >> >> I could visit_type_QCryptoBlockOptionsLUKS_fields(v, opts.u.luks, errp) >> but the '_fields' methods are all declared static in qapi-visit.c >> preventing their use. >> >> IMHO, now that QAPI inlines the flat unions, we should be making >> the _fields() methods public. > > But my suggestion would just be a hack. Yours makes more sense in the > long run, so I'll go ahead and propose that patch. Another alternative, but not my preference (mentioning only for completeness): We could change the contract of visit_type_FOO(&obj) to say that if it points to non-NULL, we just visit the fields in place. All callers would then be required to set *obj = NULL prior to visit_type_FOO(&obj) for existing semantics, and you would set *obj = &parent.u.luks for new semantics. But this would entail an audit of existing callers (too many are passing uninitialized pointers, which must now be initialized), and feels a bit too complex.
On Tue, Feb 23, 2016 at 08:18:55AM -0700, Eric Blake wrote: > On 02/23/2016 07:50 AM, Daniel P. Berrange wrote: > > On Fri, Feb 19, 2016 at 01:18:05PM +0100, Markus Armbruster wrote: > >> From: Eric Blake <eblake@redhat.com> > >> > >> There's no reason to do two malloc's for a flat union; let's just > >> inline the branch struct directly into the C union branch of the > >> flat union. > >> > > > > > My code needs todo something a little different though - it needs > > to directly visit one of the union branches. Previously, I could > > allocate a QCryptoBlockOptions and then visit a specific union > > branch like this: > > > > QCryptoBlockOptions *opts = NULL; > > > > opts = g_new0(QCryptoBlockOptions, 1); > > opts.format = Q_CRYPTO_BLOCK_FORMAT_LUKS > > > > visit_type_QCryptoBlockOptionsLUKS(v, "luks", &opts.u.luks, errp) > > which allocated a pointer and visited the fields. We no longer need the > pointer, but still need the fields visited. > > > > > > > I need todo this, because my visitor instance does not have any > > 'format' field to visit - we know the format upfront from the > > block layer driver choice. So we need to directly visit the > > appropriate inline union branch. This no longer works, because > > the opts.u.luks field is now inlined instead of being boxed :-( > > You're using OptsVisitor, right? Is it possible to hack the QemuOpts > that you feed to opts_visitor_new() to include a 'format' field? Yes, using QemuOpts, so I could feed in a fake 'format' i guess. The temp hack I chose was to use an intermediate variable and then memcpy & free. ie QCryptoBlockOpenOptions *ret; QCryptoBlockOptionsLUKS *tmp; ret = g_new0(QCryptoBlockOpenOptions, 1); ret->format = format; visit_type_QCryptoBlockOptionsLUKS(opts_get_visitor(ov), "luks", &tmp, &local_err); if (!local_err) { memcpy(&ret->u.luks, tmp, sizeof(*tmp)); g_free(tmp); } > > I could visit_type_QCryptoBlockOptionsLUKS_fields(v, opts.u.luks, errp) > > but the '_fields' methods are all declared static in qapi-visit.c > > preventing their use. > > > > IMHO, now that QAPI inlines the flat unions, we should be making > > the _fields() methods public. > > But my suggestion would just be a hack. Yours makes more sense in the > long run, so I'll go ahead and propose that patch. Regards, Daniel
diff --git a/cpus.c b/cpus.c index 898426c..9592163 100644 --- a/cpus.c +++ b/cpus.c @@ -1568,28 +1568,22 @@ CpuInfoList *qmp_query_cpus(Error **errp) info->value->thread_id = cpu->thread_id; #if defined(TARGET_I386) info->value->arch = CPU_INFO_ARCH_X86; - info->value->u.x86 = g_new0(CpuInfoX86, 1); - info->value->u.x86->pc = env->eip + env->segs[R_CS].base; + info->value->u.x86.pc = env->eip + env->segs[R_CS].base; #elif defined(TARGET_PPC) info->value->arch = CPU_INFO_ARCH_PPC; - info->value->u.ppc = g_new0(CpuInfoPPC, 1); - info->value->u.ppc->nip = env->nip; + info->value->u.ppc.nip = env->nip; #elif defined(TARGET_SPARC) info->value->arch = CPU_INFO_ARCH_SPARC; - info->value->u.q_sparc = g_new0(CpuInfoSPARC, 1); - info->value->u.q_sparc->pc = env->pc; - info->value->u.q_sparc->npc = env->npc; + info->value->u.q_sparc.pc = env->pc; + info->value->u.q_sparc.npc = env->npc; #elif defined(TARGET_MIPS) info->value->arch = CPU_INFO_ARCH_MIPS; - info->value->u.q_mips = g_new0(CpuInfoMIPS, 1); - info->value->u.q_mips->PC = env->active_tc.PC; + info->value->u.q_mips.PC = env->active_tc.PC; #elif defined(TARGET_TRICORE) info->value->arch = CPU_INFO_ARCH_TRICORE; - info->value->u.tricore = g_new0(CpuInfoTricore, 1); - info->value->u.tricore->PC = env->PC; + info->value->u.tricore.PC = env->PC; #else info->value->arch = CPU_INFO_ARCH_OTHER; - info->value->u.other = g_new0(CpuInfoOther, 1); #endif /* XXX: waiting for the qapi to support GSList */ diff --git a/hmp.c b/hmp.c index 996cb91..bfbd667 100644 --- a/hmp.c +++ b/hmp.c @@ -314,22 +314,22 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict) switch (cpu->value->arch) { case CPU_INFO_ARCH_X86: - monitor_printf(mon, " pc=0x%016" PRIx64, cpu->value->u.x86->pc); + monitor_printf(mon, " pc=0x%016" PRIx64, cpu->value->u.x86.pc); break; case CPU_INFO_ARCH_PPC: - monitor_printf(mon, " nip=0x%016" PRIx64, cpu->value->u.ppc->nip); + monitor_printf(mon, " nip=0x%016" PRIx64, cpu->value->u.ppc.nip); break; case CPU_INFO_ARCH_SPARC: monitor_printf(mon, " pc=0x%016" PRIx64, - cpu->value->u.q_sparc->pc); + cpu->value->u.q_sparc.pc); monitor_printf(mon, " npc=0x%016" PRIx64, - cpu->value->u.q_sparc->npc); + cpu->value->u.q_sparc.npc); break; case CPU_INFO_ARCH_MIPS: - monitor_printf(mon, " PC=0x%016" PRIx64, cpu->value->u.q_mips->PC); + monitor_printf(mon, " PC=0x%016" PRIx64, cpu->value->u.q_mips.PC); break; case CPU_INFO_ARCH_TRICORE: - monitor_printf(mon, " PC=0x%016" PRIx64, cpu->value->u.tricore->PC); + monitor_printf(mon, " PC=0x%016" PRIx64, cpu->value->u.tricore.PC); break; default: break; diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h index 7905a28..c4af3e0 100644 --- a/include/qapi/visitor-impl.h +++ b/include/qapi/visitor-impl.h @@ -58,8 +58,6 @@ struct Visitor /* May be NULL; most useful for input visitors. */ void (*optional)(Visitor *v, const char *name, bool *present); - - bool (*start_union)(Visitor *v, bool data_present, Error **errp); }; void input_type_enum(Visitor *v, const char *name, int *obj, diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index 8a2d5cc..a6678b1 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -79,6 +79,5 @@ void visit_type_str(Visitor *v, const char *name, char **obj, Error **errp); void visit_type_number(Visitor *v, const char *name, double *obj, Error **errp); void visit_type_any(Visitor *v, const char *name, QObject **obj, Error **errp); -bool visit_start_union(Visitor *v, bool data_present, Error **errp); #endif diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c index 6667e8c..4eae555 100644 --- a/qapi/qapi-dealloc-visitor.c +++ b/qapi/qapi-dealloc-visitor.c @@ -169,31 +169,6 @@ static void qapi_dealloc_type_enum(Visitor *v, const char *name, int *obj, { } -/* If there's no data present, the dealloc visitor has nothing to free. - * Thus, indicate to visitor code that the subsequent union fields can - * be skipped. This is not an error condition, since the cleanup of the - * rest of an object can continue unhindered, so leave errp unset in - * these cases. - * - * NOTE: In cases where we're attempting to deallocate an object that - * may have missing fields, the field indicating the union type may - * be missing. In such a case, it's possible we don't have enough - * information to differentiate data_present == false from a case where - * data *is* present but happens to be a scalar with a value of 0. - * This is okay, since in the case of the dealloc visitor there's no - * work that needs to done in either situation. - * - * The current inability in QAPI code to more thoroughly verify a union - * type in such cases will likely need to be addressed if we wish to - * implement this interface for other types of visitors in the future, - * however. - */ -static bool qapi_dealloc_start_union(Visitor *v, bool data_present, - Error **errp) -{ - return data_present; -} - Visitor *qapi_dealloc_get_visitor(QapiDeallocVisitor *v) { return &v->visitor; @@ -224,7 +199,6 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void) v->visitor.type_str = qapi_dealloc_type_str; v->visitor.type_number = qapi_dealloc_type_number; v->visitor.type_any = qapi_dealloc_type_anything; - v->visitor.start_union = qapi_dealloc_start_union; QTAILQ_INIT(&v->stack); diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index b4a0f21..f7b9980 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -61,14 +61,6 @@ void visit_end_list(Visitor *v) v->end_list(v); } -bool visit_start_union(Visitor *v, bool data_present, Error **errp) -{ - if (v->start_union) { - return v->start_union(v, data_present, errp); - } - return true; -} - bool visit_optional(Visitor *v, const char *name, bool *present) { if (v->optional) { diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 4dabe91..eac90d2 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -116,14 +116,6 @@ static inline %(base)s *qapi_%(c_name)s_base(const %(c_name)s *obj) def gen_variants(variants): - # HACK: Determine if this is an alternate (at least one variant - # is not an object); unions have all branches as objects. - unboxed = False - for v in variants.variants: - if not isinstance(v.type, QAPISchemaObjectType): - unboxed = True - break - # FIXME: What purpose does data serve, besides preventing a union that # has a branch named 'data'? We use it in qapi-visit.py to decide # whether to bypass the switch statement if visiting the discriminator @@ -140,11 +132,12 @@ def gen_variants(variants): for var in variants.variants: # Ugly special case for simple union TODO get rid of it - typ = var.simple_union_type() or var.type + simple_union_type = var.simple_union_type() + typ = simple_union_type or var.type ret += mcgen(''' %(c_type)s %(c_name)s; ''', - c_type=typ.c_type(is_unboxed=unboxed), + c_type=typ.c_type(is_unboxed=not simple_union_type), c_name=c_name(var.name)) ret += mcgen(''' diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index f4e38d1..3a3918f 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -15,10 +15,6 @@ 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() @@ -45,31 +41,6 @@ static void visit_type_%(c_type)s_fields(Visitor *v, %(c_type)s *obj, Error **er c_type=typ.c_name()) -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(''' - -static void visit_type_implicit_%(c_type)s(Visitor *v, %(c_type)s **obj, Error **errp) -{ - Error *err = NULL; - - visit_start_implicit_struct(v, (void **)obj, sizeof(%(c_type)s), &err); - if (!err) { - visit_type_%(c_type)s_fields(v, *obj, errp); - visit_end_implicit_struct(v); - } - error_propagate(errp, err); -} -''', - c_type=typ.c_name()) - return ret - - def gen_visit_struct_fields(name, base, members, variants): ret = '' @@ -79,7 +50,7 @@ def gen_visit_struct_fields(name, base, members, variants): for var in variants.variants: # Ugly special case for simple union TODO get rid of it if not var.simple_union_type(): - ret += gen_visit_implicit_struct(var.type) + ret += gen_visit_fields_decl(var.type) struct_fields_seen.add(name) ret += mcgen(''' @@ -102,9 +73,6 @@ static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s *obj, Error **er if variants: ret += mcgen(''' - if (!visit_start_union(v, !!obj->u.data, &err) || err) { - goto out; - } switch (obj->%(c_name)s) { ''', c_name=c_name(variants.tag_member.name)) @@ -126,7 +94,7 @@ static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s *obj, Error **er c_name=c_name(var.name)) else: ret += mcgen(''' - visit_type_implicit_%(c_type)s(v, &obj->u.%(c_name)s, &err); + visit_type_%(c_type)s_fields(v, &obj->u.%(c_name)s, &err); ''', c_type=var.type.c_name(), c_name=c_name(var.name)) diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c index 47cf6aa..b05da5b 100644 --- a/tests/test-qmp-input-visitor.c +++ b/tests/test-qmp-input-visitor.c @@ -295,7 +295,7 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data, g_assert_cmpint(tmp->enum1, ==, ENUM_ONE_VALUE1); g_assert_cmpstr(tmp->string, ==, "str"); g_assert_cmpint(tmp->integer, ==, 41); - g_assert_cmpint(tmp->u.value1->boolean, ==, true); + g_assert_cmpint(tmp->u.value1.boolean, ==, true); base = qapi_UserDefFlatUnion_base(tmp); g_assert(&base->enum1 == &tmp->enum1); @@ -330,8 +330,8 @@ static void test_visitor_in_alternate(TestInputVisitorData *data, g_assert_cmpint(tmp->u.udfu.integer, ==, 1); g_assert_cmpstr(tmp->u.udfu.string, ==, "str"); g_assert_cmpint(tmp->u.udfu.enum1, ==, ENUM_ONE_VALUE1); - g_assert_cmpint(tmp->u.udfu.u.value1->boolean, ==, true); - g_assert_cmpint(tmp->u.udfu.u.value1->has_a_b, ==, false); + g_assert_cmpint(tmp->u.udfu.u.value1.boolean, ==, true); + g_assert_cmpint(tmp->u.udfu.u.value1.has_a_b, ==, false); qapi_free_UserDefAlternate(tmp); v = visitor_input_test_init(data, "false"); @@ -358,8 +358,8 @@ static void test_visitor_in_alternate(TestInputVisitorData *data, g_assert_cmpint(wrap->alt->u.udfu.integer, ==, 1); g_assert_cmpstr(wrap->alt->u.udfu.string, ==, "str"); g_assert_cmpint(wrap->alt->u.udfu.enum1, ==, ENUM_ONE_VALUE1); - g_assert_cmpint(wrap->alt->u.udfu.u.value1->boolean, ==, true); - g_assert_cmpint(wrap->alt->u.udfu.u.value1->has_a_b, ==, false); + g_assert_cmpint(wrap->alt->u.udfu.u.value1.boolean, ==, true); + g_assert_cmpint(wrap->alt->u.udfu.u.value1.has_a_b, ==, false); qapi_free_WrapAlternate(wrap); } diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c index fe2f1a1..a7f8b45 100644 --- a/tests/test-qmp-output-visitor.c +++ b/tests/test-qmp-output-visitor.c @@ -403,9 +403,8 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data, UserDefFlatUnion *tmp = g_malloc0(sizeof(UserDefFlatUnion)); tmp->enum1 = ENUM_ONE_VALUE1; tmp->string = g_strdup("str"); - tmp->u.value1 = g_malloc0(sizeof(UserDefA)); tmp->integer = 41; - tmp->u.value1->boolean = true; + tmp->u.value1.boolean = true; visit_type_UserDefFlatUnion(data->ov, NULL, &tmp, &error_abort); arg = qmp_output_get_qobject(data->qov); @@ -460,8 +459,7 @@ static void test_visitor_out_alternate(TestOutputVisitorData *data, tmp->u.udfu.integer = 1; tmp->u.udfu.string = g_strdup("str"); tmp->u.udfu.enum1 = ENUM_ONE_VALUE1; - tmp->u.udfu.u.value1 = g_new0(UserDefA, 1); - tmp->u.udfu.u.value1->boolean = true; + tmp->u.udfu.u.value1.boolean = true; visit_type_UserDefAlternate(data->ov, NULL, &tmp, &error_abort); arg = qmp_output_get_qobject(data->qov);