• E
    qapi: Don't box branches of flat unions · 544a3731
    Eric Blake 提交于
    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: NEric 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: NMarkus Armbruster <armbru@redhat.com>
    544a3731
hmp.c 70.6 KB