Eric Blake <ebl...@redhat.com> writes:
> There's no reason to do two malloc's for an alternate type visiting
> a QAPI struct; let's just inline the struct directly as the C union
> branch of the struct.
>
> Surprisingly, no clients were actually using the struct member prior
> to this patch outside of the testsuite; an earlier patch in the series
> added some testsuite coverage to make the effect of this patch more
> obvious.
>
> In qapi.py, c_type() gains a new is_unboxed flag to control when we
> are emitting a C struct unboxed within the context of an outer
> struct (different from our other two modes of usage with no flags
> for normal local variable declarations, and with is_param for adding
> 'const' in a parameter list). I don't know if there is any more
> pythonic way of collapsing the two flags into a single parameter,
> as we never have a caller setting both flags at once.
>
> Ultimately, we want to also unbox branches for QAPI unions, but as
> that touches a lot more client code, it is better as separate
> patches. But since unions and alternates share gen_variants(), I
> had to hack in a way to test if we are visiting an alternate type
> for setting the is_unboxed flag: look for a non-object branch.
> This works because alternates have at least two branches, with at
> most one object branch, while unions have only object branches.
> The hack will go away in a later patch.
>
> The generated code difference to qapi-types.h is relatively small:
>
> | struct BlockdevRef {
> | QType type;
> | union { /* union tag is @type */
> | void *data;
> |- BlockdevOptions *definition;
> |+ BlockdevOptions definition;
> | char *reference;
> | } u;
> | };
>
> meanwhile, in qapi-visit.h, we trade the previous visit_type_FOO(obj)
Meanwhile
> (which allocates a pointer, handles a layer of {} from the JSON stream,
> and visits the fields) with an inline call to visit_type_FOO(NULL) (to
> consume the {} without allocation) and a direct visit of the fields:
I don't see a call to visit_type_FOO(NULL).
Suggest not to abbreviate argument lists, it's mildly confusing.
>
> | switch ((*obj)->type) {
> | case QTYPE_QDICT:
> |- visit_type_BlockdevOptions(v, name, &(*obj)->u.definition, &err);
> |+ visit_start_struct(v, name, NULL, 0, &err);
> |+ if (err) {
> |+ break;
> |+ }
> |+ visit_type_BlockdevOptions_fields(v, &(*obj)->u.definition, &err);
> |+ error_propagate(errp, err);
> |+ err = NULL;
> |+ visit_end_struct(v, &err);
> | break;
> | case QTYPE_QSTRING:
> | visit_type_str(v, name, &(*obj)->u.reference, &err);
>
> The visit of non-object fields is unchanged.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
Patch looks good.
