[Qemu-commits] [qemu/qemu] 9d3524: qapi-visit: Honor prefix of discriminator enum

[GitHub]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 9d3524b39e1fe5f3bb7a990ad7841e469e954a3b
      
https://github.com/qemu/qemu/commit/9d3524b39e1fe5f3bb7a990ad7841e469e954a3b
  Author: Eric Blake <address@hidden>
  Date:   2016-02-19 (Fri, 19 Feb 2016)

  Changed paths:
    M scripts/qapi-visit.py
    M tests/qapi-schema/qapi-schema-test.json
    M tests/qapi-schema/qapi-schema-test.out

  Log Message:
  -----------
  qapi-visit: Honor prefix of discriminator enum

When we added support for a user-specified prefix for an enum
type (commit 351d36e), we forgot to teach the qapi-visit code
to honor that prefix in the case of using a prefixed enum as
the discriminator for a flat union.  While there is still some
on-list debate on whether we want to keep prefixes, we should
at least make it work as long as it is still part of the code
base.

Reported-by: Daniel P. Berrange <address@hidden>
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: f96493b1ab14e307495119064c32765f436b349c
      
https://github.com/qemu/qemu/commit/f96493b1ab14e307495119064c32765f436b349c
  Author: Eric Blake <address@hidden>
  Date:   2016-02-19 (Fri, 19 Feb 2016)

  Changed paths:
    M qapi/opts-visitor.c
    M qapi/qmp-input-visitor.c

  Log Message:
  -----------
  qapi: Simplify excess input reporting in input visitors

When reporting that an unvisited member remains at the end of an
input visit for a struct, we were using g_hash_table_find()
coupled with a callback function that always returns true, to
locate an arbitrary member of the hash table.  But if all we
need is an arbitrary entry, we can get that from a single-use
iterator, without needing a tautological callback function.

Technically, our cast of &(GQueue *) to (void **) is not strict
C (while void * must be able to hold all other pointers, nothing
says a void ** has to be the same width or representation as a
GQueue **).  The kosher way to write it would be the verbose:

    void *tmp;
    GQueue *any;
    if (g_hash_table_iter_next(&iter, NULL, &tmp)) {
  any = tmp;

But our code base (not to mention glib itself) already has other
cases of assuming that ALL pointers have the same width and
representation, where a compiler would have to go out of its way
to mis-compile our borderline behavior.

Suggested-by: Markus Armbruster <address@hidden>
Signed-off-by: Eric Blake <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: 02a57ae32b08e8981b59979b80e682c9a153e94d
      
https://github.com/qemu/qemu/commit/02a57ae32b08e8981b59979b80e682c9a153e94d
  Author: Eric Blake <address@hidden>
  Date:   2016-02-19 (Fri, 19 Feb 2016)

  Changed paths:
    M docs/qapi-code-gen.txt
    M scripts/qapi.py
    M tests/qapi-schema/alternate-empty.err
    M tests/qapi-schema/alternate-empty.exit
    M tests/qapi-schema/alternate-empty.json
    M tests/qapi-schema/alternate-empty.out
    M tests/qapi-schema/flat-union-empty.err
    M tests/qapi-schema/flat-union-empty.exit
    M tests/qapi-schema/flat-union-empty.json
    M tests/qapi-schema/flat-union-empty.out
    M tests/qapi-schema/union-empty.err
    M tests/qapi-schema/union-empty.exit
    M tests/qapi-schema/union-empty.json
    M tests/qapi-schema/union-empty.out

  Log Message:
  -----------
  qapi: Forbid empty unions and useless alternates

Empty unions serve no purpose, and while we compile with gcc
which permits them, strict C99 forbids them.  We happen to inject
a dummy 'void *data' member into the C unions that represent QAPI
unions and alternates, but we want to get rid of that member (it
pollutes the namespace for no good reason), which would leave us
with an empty union if the user didn't provide any branches.  While
empty structs make sense in QAPI, empty unions don't add any
expressiveness to the QMP language.  So prohibit them at parse
time.  Update the documentation and testsuite to match.

Note that the documentation already mentioned that alternates
should have "two or more JSON data types"; so this also fixes
the code to enforce that.  However, we have existing uses of a
union type with only one branch, so the 2-or-more strictness
is intentionally limited to alternates.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: 46534309e667fd860720f983c2c9aefe0354340d
      
https://github.com/qemu/qemu/commit/46534309e667fd860720f983c2c9aefe0354340d
  Author: Eric Blake <address@hidden>
  Date:   2016-02-19 (Fri, 19 Feb 2016)

  Changed paths:
    M scripts/qapi.py
    M tests/Makefile
    A tests/qapi-schema/alternate-any.err
    A tests/qapi-schema/alternate-any.exit
    A tests/qapi-schema/alternate-any.json
    A tests/qapi-schema/alternate-any.out

  Log Message:
  -----------
  qapi: Forbid 'any' inside an alternate

The whole point of an alternate is to allow some type-safety while
still accepting more than one JSON type.  Meanwhile, the 'any'
type exists to bypass type-safety altogether.  The two are
incompatible: you can't accept every type, and still tell which
branch of the alternate to use for the parse; fix this to give a
sane error instead of a Python stack trace.

Note that other types that can't be alternate members are caught
earlier, by check_type().

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: 68d078395d8233ca8455a95f05e5c23e367777c2
      
https://github.com/qemu/qemu/commit/68d078395d8233ca8455a95f05e5c23e367777c2
  Author: Eric Blake <address@hidden>
  Date:   2016-02-19 (Fri, 19 Feb 2016)

  Changed paths:
    M tests/qapi-schema/qapi-schema-test.json
    M tests/qapi-schema/qapi-schema-test.out
    M tests/test-qmp-input-visitor.c
    M tests/test-qmp-output-visitor.c

  Log Message:
  -----------
  qapi: Add tests of complex objects within alternate

Upcoming patches will adjust how we visit an object branch of an
alternate; but we were completely lacking testsuite coverage.
Rectify this, so that the future patches will be able to highlight
the changes and still prove that we avoided regressions.

In particular, the use of a flat union UserDefFlatUnion rather
than a simple struct UserDefA as the branch will give us coverage
of an object with variants.  And visiting an alternate as both
the top level and as a nested member gives confidence in correct
memory allocation handling, especially if the test is run under
valgrind.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: d7445b57f4342d50c25b5bc1746048fa3720511b
      
https://github.com/qemu/qemu/commit/d7445b57f4342d50c25b5bc1746048fa3720511b
  Author: Markus Armbruster <address@hidden>
  Date:   2016-02-19 (Fri, 19 Feb 2016)

  Changed paths:
    M scripts/qapi-visit.py

  Log Message:
  -----------
  qapi-visit: Simplify how we visit common union members

For a simple union SU, gen_visit_union() generates a visit of its
single tag member, like this:

    visit_type_SUKind(v, "type", &(*obj)->type, &err);

For a flat union FU with base B, it generates a visit of its base
fields:

    visit_type_B_fields(v, (B **)obj, &err);

Instead, we can simply visit the common members using the same fields
visit function we use for structs, generated with
gen_visit_struct_fields().  This function visits the base if any, then
the local members.

For a simple union SU, visit_type_SU_fields() contains exactly the old
tag member visit, because there is no base, and the tag member is the
only member.  For instance, the code generated for qapi-schema.json's
KeyValue changes like this:

    +static void visit_type_KeyValue_fields(Visitor *v, KeyValue **obj, Error 
**errp)
    +{
    +    Error *err = NULL;
    +
    +    visit_type_KeyValueKind(v, "type", &(*obj)->type, &err);
    +    if (err) {
    +        goto out;
    +    }
    +
    +out:
    +    error_propagate(errp, err);
    +}
    +
     void visit_type_KeyValue(Visitor *v, const char *name, KeyValue **obj, 
Error **errp)
     {
   Error *err = NULL;
    @@ -4863,7 +4911,7 @@ void visit_type_KeyValue(Visitor *v, con
   if (!*obj) {
       goto out_obj;
   }
    -    visit_type_KeyValueKind(v, "type", &(*obj)->type, &err);
    +    visit_type_KeyValue_fields(v, obj, &err);
   if (err) {
       goto out_obj;
   }

For a flat union FU, visit_type_FU_fields() contains exactly the old
base fields visit, because there is a base, but no members.  For
instance, the code generated for qapi-schema.json's CpuInfo changes
like this:

     static void visit_type_CpuInfoBase_fields(Visitor *v, CpuInfoBase **obj, 
Error **errp);

    +static void visit_type_CpuInfo_fields(Visitor *v, CpuInfo **obj, Error 
**errp)
    +{
    +    Error *err = NULL;
    +
    +    visit_type_CpuInfoBase_fields(v, (CpuInfoBase **)obj, &err);
    +    if (err) {
    +        goto out;
    +    }
    +
    +out:
    +    error_propagate(errp, err);
    +}
    +
     static void visit_type_CpuInfoX86_fields(Visitor *v, CpuInfoX86 **obj, 
Error **errp)
...
    @@ -3485,7 +3509,7 @@ void visit_type_CpuInfo(Visitor *v, cons
   if (!*obj) {
       goto out_obj;
   }
    -    visit_type_CpuInfoBase_fields(v, (CpuInfoBase **)obj, &err);
    +    visit_type_CpuInfo_fields(v, obj, &err);
   if (err) {
       goto out_obj;
   }

As you see, the generated code grows a bit, but in practice, it's lost
in the noise: qapi-schema.json's qapi-visit.c gains roughly 1%.

This simplification became possible with commit 441cbac "qapi-visit:
Convert to QAPISchemaVisitor, fixing bugs".  It's a step towards
unifying gen_struct() and gen_union().

Signed-off-by: Markus Armbruster <address@hidden>
Message-Id: <address@hidden>
[improve commit message examples]
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
[Commit message tweaked]


  Commit: 9a5cd424d5f06fb5293eb264456d89343c557558
      
https://github.com/qemu/qemu/commit/9a5cd424d5f06fb5293eb264456d89343c557558
  Author: Eric Blake <address@hidden>
  Date:   2016-02-19 (Fri, 19 Feb 2016)

  Changed paths:
    M scripts/qapi-visit.py

  Log Message:
  -----------
  qapi: Visit variants in visit_type_FOO_fields()

We initially created the static visit_type_FOO_fields() helper
function for reuse of code - we have cases where the initial
setup for a visit has different allocation (depending on whether
the fields represent a stand-alone type or are embedded as part
of a larger type), but where the actual field visits are
identical once a pointer is available.

Up until the previous patch, visit_type_FOO_fields() was only
used for structs (no variants), so it was covering every field
for each type where it was emitted.

Meanwhile, the code for visiting unions looks like:

static visit_type_U_fields() {
    visit base;
    visit local_members;
}
visit_type_U() {
    visit_start_struct();
    visit_type_U_fields();
    visit variants;
    visit_end_struct();
}

which splits the fields of the union visit across two functions.
Move the code to visit variants to live inside visit_type_U_fields(),
while making it conditional on having variants so that all other
instances of the helper function remain unchanged.  This is also
a step closer towards unifying struct and union visits, and towards
allowing one union type to be the branch of another flat union.

The resulting diff to the generated code is a bit hard to read,
but it can be verified that it touches only union types, and that
the end result is the following general structure:

static visit_type_U_fields() {
    visit base;
    visit local_members;
    visit variants;
}
visit_type_U() {
    visit_start_struct();
    visit_type_U_fields();
    visit_end_struct();
}

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
[gen_visit_struct_fields() parameter variants made mandatory]
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: 59d9e84cc94b8268f13ec184acfb997e8f352593
      
https://github.com/qemu/qemu/commit/59d9e84cc94b8268f13ec184acfb997e8f352593
  Author: Markus Armbruster <address@hidden>
  Date:   2016-02-19 (Fri, 19 Feb 2016)

  Changed paths:
    M scripts/qapi-visit.py

  Log Message:
  -----------
  qapi-visit: Unify struct and union visit

gen_visit_union() is now just like gen_visit_struct().  Rename
it to gen_visit_object(), use it for structs, and drop
gen_visit_struct().  Output is unchanged.

Signed-off-by: Markus Armbruster <address@hidden>
Message-Id: <address@hidden>
[split out variant handling, rebase to earlier changes]
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>


  Commit: 655519030b5d20967ae3afa1fe91ef5ad4406065
      
https://github.com/qemu/qemu/commit/655519030b5d20967ae3afa1fe91ef5ad4406065
  Author: Eric Blake <address@hidden>
  Date:   2016-02-19 (Fri, 19 Feb 2016)

  Changed paths:
    M scripts/qapi-visit.py

  Log Message:
  -----------
  qapi-visit: Less indirection in visit_type_Foo_fields()

We were passing 'Foo **obj' to the internal helper function, but
all uses within the helper were via reads of '*obj'.  Refactor
things to pass one less level of indirection, by having the
callers dereference before calling.

For an example of the generated code change:

|-static void visit_type_BalloonInfo_fields(Visitor *v, BalloonInfo **obj, 
Error **errp)
|+static void visit_type_BalloonInfo_fields(Visitor *v, BalloonInfo *obj, Error 
**errp)
| {
|     Error *err = NULL;
|
|-    visit_type_int(v, "actual", &(*obj)->actual, &err);
|+    visit_type_int(v, "actual", &obj->actual, &err);
|     error_propagate(errp, err);
| }
|
|@@ -261,7 +261,7 @@ void visit_type_BalloonInfo(Visitor *v,
|     if (!*obj) {
|         goto out_obj;
|     }
|-    visit_type_BalloonInfo_fields(v, obj, &err);
|+    visit_type_BalloonInfo_fields(v, *obj, &err);
| out_obj:

The refactoring will also make it easier to reuse the helpers in
a future patch when implicit structs are stored directly in the
parent struct rather than boxed through a pointer.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: e65d89bf1a4484e0db0f3dc820a8b209f2fb1e8b
      
https://github.com/qemu/qemu/commit/e65d89bf1a4484e0db0f3dc820a8b209f2fb1e8b
  Author: Eric Blake <address@hidden>
  Date:   2016-02-19 (Fri, 19 Feb 2016)

  Changed paths:
    M include/qapi/visitor-impl.h
    M include/qapi/visitor.h
    M qapi/opts-visitor.c
    M qapi/qapi-dealloc-visitor.c
    M qapi/qapi-visit-core.c
    M qapi/qmp-input-visitor.c
    M qapi/qmp-output-visitor.c
    M qapi/string-input-visitor.c
    M qapi/string-output-visitor.c
    M scripts/qapi-types.py
    M scripts/qapi-visit.py

  Log Message:
  -----------
  qapi: Adjust layout of FooList types

By sticking the next pointer first, we don't need a union with
64-bit padding for smaller types.  On 32-bit platforms, this
can reduce the size of uint8List from 16 bytes (or 12, depending
on whether 64-bit ints can tolerate 4-byte alignment) down to 8.
It has no effect on 64-bit platforms (where alignment still
dictates a 16-byte struct); but fewer anonymous unions is still
a win in my book.

It requires visit_next_list() to gain a size parameter, to know
what size element to allocate; comparable to the size parameter
of visit_start_struct().

I debated about going one step further, to allow for fewer casts,
by doing:
    typedef GenericList GenericList;
    struct GenericList {
  GenericList *next;
    };
    struct FooList {
  GenericList base;
  Foo *value;
    };
so that you convert to 'GenericList *' by '&foolist->base', and
back by 'container_of(generic, GenericList, base)' (as opposed to
the existing '(GenericList *)foolist' and '(FooList *)generic').
But doing that would require hoisting the declaration of
GenericList prior to inclusion of qapi-types.h, rather than its
current spot in visitor.h; it also makes iteration a bit more
verbose through 'foolist->base.next' instead of 'foolist->next'.

Note that for lists of objects, the 'value' payload is still
hidden behind a boxed pointer.  Someday, it would be nice to do:

struct FooList {
    FooList *next;
    Foo value;
};

for one less level of malloc for each list element.  This patch
is a step in that direction (now that 'next' is no longer at a
fixed non-zero offset within the struct, we can store more than
just a pointer's-worth of data as the value payload), but the
actual conversion would be a task for another series, as it will
touch a lot of code.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: 1de5d4ca0752138034305f3d4e8fe17ef6503569
      
https://github.com/qemu/qemu/commit/1de5d4ca0752138034305f3d4e8fe17ef6503569
  Author: Eric Blake <address@hidden>
  Date:   2016-02-19 (Fri, 19 Feb 2016)

  Changed paths:
    M scripts/qapi-types.py

  Log Message:
  -----------
  qapi: Emit structs used as variants in topological order

Right now, we emit the branches of union types as a boxed pointer,
and it suffices to have a forward declaration of the type.  However,
a future patch will swap things to directly use the branch type,
instead of hiding it behind a pointer.  For this to work, the
compiler needs the full definition of the type, not just a forward
declaration, prior to the union that is including the branch type.
This patch just adds topological sorting to hoist all types
mentioned in a branch of a union to be fully declared before the
union itself.  The sort is always possible, because we do not
allow circular union types that include themselves as a direct
branch (it is, however, still possible to include a branch type
that itself has a pointer to the union, for a type that can
indirectly recursively nest itself - that remains safe, because
that the member of the branch type will remain a pointer, and the
QMP representation of such a type adds another {} for each recurring
layer of the union type).

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: 2208d64998c5f867ccee7eeee298971685bf822d
      
https://github.com/qemu/qemu/commit/2208d64998c5f867ccee7eeee298971685bf822d
  Author: Eric Blake <address@hidden>
  Date:   2016-02-19 (Fri, 19 Feb 2016)

  Changed paths:
    M scripts/qapi-visit.py

  Log Message:
  -----------
  qapi-visit: Use common idiom in gen_visit_fields_decl()

We have several instances of methods that do an early exit if
output is not needed, then log that output is being generated,
and finally produce the output; see qapi-types.py:gen_object()
and qapi-visit.py:gen_visit_implicit_struct().  The odd man
out was gen_visit_fields_decl(); rearrange it to be more like
the others.  No semantic change or difference to generated code.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: becceedc4d9bc1435099c90a0514945a89844d3a
      
https://github.com/qemu/qemu/commit/becceedc4d9bc1435099c90a0514945a89844d3a
  Author: Eric Blake <address@hidden>
  Date:   2016-02-19 (Fri, 19 Feb 2016)

  Changed paths:
    M scripts/qapi-types.py
    M scripts/qapi-visit.py
    M scripts/qapi.py
    M tests/test-qmp-input-visitor.c
    M tests/test-qmp-output-visitor.c

  Log Message:
  -----------
  qapi: Don't box struct branch of alternate

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;
| };

The corresponding spot in qapi-visit.c calls visit_type_FOO(), which
first calls visit_start_struct() to allocate or deallocate the member
and handle a layer of {} from the JSON stream, then visits the
members.  To peel off the indirection and the memory management that
comes with it, we inline this call, then suppress allocation /
deallocation by passing NULL to visit_start_struct(), and adjust the
member visit:

|     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 <address@hidden>
Message-Id: <address@hidden>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: 544a3731591f5d53e15f22de00ce5ac758d490b3
      
https://github.com/qemu/qemu/commit/544a3731591f5d53e15f22de00ce5ac758d490b3
  Author: Eric Blake <address@hidden>
  Date:   2016-02-19 (Fri, 19 Feb 2016)

  Changed paths:
    M cpus.c
    M hmp.c
    M include/qapi/visitor-impl.h
    M include/qapi/visitor.h
    M qapi/qapi-dealloc-visitor.c
    M qapi/qapi-visit-core.c
    M scripts/qapi-types.py
    M scripts/qapi-visit.py
    M tests/test-qmp-input-visitor.c
    M tests/test-qmp-output-visitor.c

  Log Message:
  -----------
  qapi: Don't box branches of flat unions

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 <address@hidden>
Message-Id: <address@hidden>
[Dead code deletion squashed in, commit message updated accordingly]
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: dbf11922622685934bfb41e7cf2be9bd4a0405c0
      
https://github.com/qemu/qemu/commit/dbf11922622685934bfb41e7cf2be9bd4a0405c0
  Author: Eric Blake <address@hidden>
  Date:   2016-02-19 (Fri, 19 Feb 2016)

  Changed paths:
    M include/qapi/visitor-impl.h
    M include/qapi/visitor.h
    M qapi/qapi-dealloc-visitor.c
    M qapi/qapi-visit-core.c
    M qapi/qmp-input-visitor.c
    M scripts/qapi-visit.py

  Log Message:
  -----------
  qapi: Change visit_start_implicit_struct to visit_start_alternate

After recent changes, the only remaining use of
visit_start_implicit_struct() is for allocating the space needed
when visiting an alternate.  Since the term 'implicit struct' is
hard to explain, rename the function to its current usage.  While
at it, we can merge the functionality of visit_get_next_type()
into the same function, making it more like visit_start_struct().

Generated code is now slightly smaller:

| {
|     Error *err = NULL;
|
|-    visit_start_implicit_struct(v, (void**) obj, sizeof(BlockdevRef), &err);
|+    visit_start_alternate(v, name, (GenericAlternate **)obj, sizeof(**obj),
|+                          true, &err);
|     if (err) {
|         goto out;
|     }
|-    visit_get_next_type(v, name, &(*obj)->type, true, &err);
|-    if (err) {
|-        goto out_obj;
|-    }
|     switch ((*obj)->type) {
|     case QTYPE_QDICT:
|         visit_start_struct(v, name, NULL, 0, &err);
...
|     }
|-out_obj:
|-    visit_end_implicit_struct(v);
|+    visit_end_alternate(v);
| out:
|     error_propagate(errp, err);
| }

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: 5cfffc30de4a34a47d2719d2cd87ababb6c6379b
      
https://github.com/qemu/qemu/commit/5cfffc30de4a34a47d2719d2cd87ababb6c6379b
  Author: Peter Maydell <address@hidden>
  Date:   2016-02-19 (Fri, 19 Feb 2016)

  Changed paths:
    M cpus.c
    M docs/qapi-code-gen.txt
    M hmp.c
    M include/qapi/visitor-impl.h
    M include/qapi/visitor.h
    M qapi/opts-visitor.c
    M qapi/qapi-dealloc-visitor.c
    M qapi/qapi-visit-core.c
    M qapi/qmp-input-visitor.c
    M qapi/qmp-output-visitor.c
    M qapi/string-input-visitor.c
    M qapi/string-output-visitor.c
    M scripts/qapi-types.py
    M scripts/qapi-visit.py
    M scripts/qapi.py
    M tests/Makefile
    A tests/qapi-schema/alternate-any.err
    A tests/qapi-schema/alternate-any.exit
    A tests/qapi-schema/alternate-any.json
    A tests/qapi-schema/alternate-any.out
    M tests/qapi-schema/alternate-empty.err
    M tests/qapi-schema/alternate-empty.exit
    M tests/qapi-schema/alternate-empty.json
    M tests/qapi-schema/alternate-empty.out
    M tests/qapi-schema/flat-union-empty.err
    M tests/qapi-schema/flat-union-empty.exit
    M tests/qapi-schema/flat-union-empty.json
    M tests/qapi-schema/flat-union-empty.out
    M tests/qapi-schema/qapi-schema-test.json
    M tests/qapi-schema/qapi-schema-test.out
    M tests/qapi-schema/union-empty.err
    M tests/qapi-schema/union-empty.exit
    M tests/qapi-schema/union-empty.json
    M tests/qapi-schema/union-empty.out
    M tests/test-qmp-input-visitor.c
    M tests/test-qmp-output-visitor.c

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2016-02-19' into 
staging

QAPI patches for 2016-02-19

# gpg: Signature made Fri 19 Feb 2016 10:10:18 GMT using RSA key ID EB918653
# gpg: Good signature from "Markus Armbruster <address@hidden>"
# gpg:                 aka "Markus Armbruster <address@hidden>"

* remotes/armbru/tags/pull-qapi-2016-02-19:
  qapi: Change visit_start_implicit_struct to visit_start_alternate
  qapi: Don't box branches of flat unions
  qapi: Don't box struct branch of alternate
  qapi-visit: Use common idiom in gen_visit_fields_decl()
  qapi: Emit structs used as variants in topological order
  qapi: Adjust layout of FooList types
  qapi-visit: Less indirection in visit_type_Foo_fields()
  qapi-visit: Unify struct and union visit
  qapi: Visit variants in visit_type_FOO_fields()
  qapi-visit: Simplify how we visit common union members
  qapi: Add tests of complex objects within alternate
  qapi: Forbid 'any' inside an alternate
  qapi: Forbid empty unions and useless alternates
  qapi: Simplify excess input reporting in input visitors
  qapi-visit: Honor prefix of discriminator enum

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/09125c5e7692...5cfffc30de4a