1. 18 3月, 2016 12 次提交
    • E
      qapi: Don't special-case simple union wrappers · 32bafa8f
      Eric Blake 提交于
      Simple unions were carrying a special case that hid their 'data'
      QMP member from the resulting C struct, via the hack method
      QAPISchemaObjectTypeVariant.simple_union_type().  But by using
      the work we started by unboxing flat union and alternate
      branches, coupled with the ability to visit the members of an
      implicit type, we can now expose the simple union's implicit
      type in qapi-types.h:
      
      | struct q_obj_ImageInfoSpecificQCow2_wrapper {
      |     ImageInfoSpecificQCow2 *data;
      | };
      |
      | struct q_obj_ImageInfoSpecificVmdk_wrapper {
      |     ImageInfoSpecificVmdk *data;
      | };
      ...
      | struct ImageInfoSpecific {
      |     ImageInfoSpecificKind type;
      |     union { /* union tag is @type */
      |         void *data;
      |-        ImageInfoSpecificQCow2 *qcow2;
      |-        ImageInfoSpecificVmdk *vmdk;
      |+        q_obj_ImageInfoSpecificQCow2_wrapper qcow2;
      |+        q_obj_ImageInfoSpecificVmdk_wrapper vmdk;
      |     } u;
      | };
      
      Doing this removes asymmetry between QAPI's QMP side and its
      C side (both sides now expose 'data'), and means that the
      treatment of a simple union as sugar for a flat union is now
      equivalent in both languages (previously the two approaches used
      a different layer of dereferencing, where the simple union could
      be converted to a flat union with equivalent C layout but
      different {} on the wire, or to an equivalent QMP wire form
      but with different C representation).  Using the implicit type
      also lets us get rid of the simple_union_type() hack.
      
      Of course, now all clients of simple unions have to adjust from
      using su->u.member to using su->u.member.data; while this touches
      a number of files in the tree, some earlier cleanup patches
      helped minimize the change to the initialization of a temporary
      variable rather than every single member access.  The generated
      qapi-visit.c code is also affected by the layout change:
      
      |@@ -7393,10 +7393,10 @@ void visit_type_ImageInfoSpecific_member
      |     }
      |     switch (obj->type) {
      |     case IMAGE_INFO_SPECIFIC_KIND_QCOW2:
      |-        visit_type_ImageInfoSpecificQCow2(v, "data", &obj->u.qcow2, &err);
      |+        visit_type_q_obj_ImageInfoSpecificQCow2_wrapper_members(v, &obj->u.qcow2, &err);
      |         break;
      |     case IMAGE_INFO_SPECIFIC_KIND_VMDK:
      |-        visit_type_ImageInfoSpecificVmdk(v, "data", &obj->u.vmdk, &err);
      |+        visit_type_q_obj_ImageInfoSpecificVmdk_wrapper_members(v, &obj->u.vmdk, &err);
      |         break;
      |     default:
      |         abort();
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1458254921-17042-13-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      32bafa8f
    • E
      qapi: Drop unused c_null() · 861877a0
      Eric Blake 提交于
      Now that we are always bulk-initializing a QAPI C struct to 0
      (whether by g_malloc0() or by 'Type arg = {0};'), we no longer
      have any clients of c_null() in the generator for per-element
      initialization.  This patch is easy enough to revert if we find
      a use in the future, but in the present, get rid of the dead code.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1458254921-17042-12-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      861877a0
    • E
      qapi: Inline gen_visit_members() into lone caller · 12f254fd
      Eric Blake 提交于
      Commit 82ca8e46 noticed that we had multiple implementations of
      visiting every member of a struct, and consolidated it into
      gen_visit_fields() (now gen_visit_members()) with enough
      parameters to cater to slight differences between the clients.
      But recent exposure of implicit types has meant that we are now
      down to a single use of that method, so we can clean up the
      unused conditionals and just inline it into the remaining
      caller: gen_visit_object_members().
      
      Likewise, gen_err_check() no longer needs optional parameters,
      as the lone use of non-defaults was via gen_visit_members().
      
      No change to generated code.
      Suggested-by: NMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1458254921-17042-11-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      12f254fd
    • E
      qapi-commands: Inline single-use helpers of gen_marshal() · c1ff0e6c
      Eric Blake 提交于
      Originally, gen_marshal_input_visit() (or gen_visitor_input_block()
      before commit f1538019) was factored out to make it easy to do two
      passes of a visit to each member of a (possibly-implicit) object,
      without duplicating lots of code.  But after recent changes, those
      visits now occupy a single line of emitted code, and the helper
      method has become a series of conditionals both before and after
      the one important line, making it rather awkward to see at a glance
      what gets emitted on the first (parsing) or second (deallocation)
      pass.  It's a lot easier to read the generator code if we just
      inline both uses directly into gen_marshal(), without all the
      conditionals.
      
      Once we've done that, it's easy to notice that gen_marshal_vars()
      is used only once, and inlining it too lets us consolidate some
      mcgen() calls that used to be split across helpers.
      
      gen_call() remains a single-use helper function, but it has
      enough indentation and complexity that inlining it would hamper
      legibility.
      
      No change to generated output.  The fact that the diffstat shows
      a net reduction in lines is an argument in favor of this cleanup.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1458254921-17042-10-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      c1ff0e6c
    • E
      qapi-commands: Utilize implicit struct visits · 386230a2
      Eric Blake 提交于
      Rather than generate inline per-member visits, take advantage
      of the 'visit_type_FOO_members()' function for command
      marshalling.  This is possible now that implicit structs can be
      visited like any other.  Generate call arguments from a stack-
      allocated struct, rather than a list of local variables:
      
      |@@ -57,26 +57,15 @@ void qmp_marshal_add_fd(QDict *args, QOb
      |     QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
      |     QapiDeallocVisitor *qdv;
      |     Visitor *v;
      |-    bool has_fdset_id = false;
      |-    int64_t fdset_id = 0;
      |-    bool has_opaque = false;
      |-    char *opaque = NULL;
      |+    q_obj_add_fd_arg arg = {0};
      |
      |     v = qmp_input_get_visitor(qiv);
      |-    if (visit_optional(v, "fdset-id", &has_fdset_id)) {
      |-        visit_type_int(v, "fdset-id", &fdset_id, &err);
      |-        if (err) {
      |-            goto out;
      |-        }
      |-    }
      |-    if (visit_optional(v, "opaque", &has_opaque)) {
      |-        visit_type_str(v, "opaque", &opaque, &err);
      |-        if (err) {
      |-            goto out;
      |-        }
      |+    visit_type_q_obj_add_fd_arg_members(v, &arg, &err);
      |+    if (err) {
      |+        goto out;
      |     }
      |
      |-    retval = qmp_add_fd(has_fdset_id, fdset_id, has_opaque, opaque, &err);
      |+    retval = qmp_add_fd(arg.has_fdset_id, arg.fdset_id, arg.has_opaque, arg.opaque, &err);
      |     if (err) {
      |         goto out;
      |     }
      |@@ -88,12 +77,7 @@ out:
      |     qmp_input_visitor_cleanup(qiv);
      |     qdv = qapi_dealloc_visitor_new();
      |     v = qapi_dealloc_get_visitor(qdv);
      |-    if (visit_optional(v, "fdset-id", &has_fdset_id)) {
      |-        visit_type_int(v, "fdset-id", &fdset_id, NULL);
      |-    }
      |-    if (visit_optional(v, "opaque", &has_opaque)) {
      |-        visit_type_str(v, "opaque", &opaque, NULL);
      |-    }
      |+    visit_type_q_obj_add_fd_arg_members(v, &arg, NULL);
      |     qapi_dealloc_visitor_cleanup(qdv);
      | }
      
      This also has the nice side effect of eliminating a chance of
      collision between argument QMP names and local variables.
      
      This patch also paves the way for some followup simplifications
      in the generator, in subsequent patches.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1458254921-17042-9-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      386230a2
    • E
      qapi-event: Utilize implicit struct visits · 0949e95b
      Eric Blake 提交于
      Rather than generate inline per-member visits, take advantage
      of the 'visit_type_FOO_members()' function for emitting events.
      This is possible now that implicit structs can be visited like
      any other.  Generated code shrinks accordingly; by initializing
      a struct based on parameters, through a new gen_param_var()
      helper, like:
      
      |@@ -338,6 +250,9 @@ void qapi_event_send_block_job_error(con
      |     QMPEventFuncEmit emit = qmp_event_get_func_emit();
      |     QmpOutputVisitor *qov;
      |     Visitor *v;
      |+    q_obj_BLOCK_JOB_ERROR_arg param = {
      |+        (char *)device, operation, action
      |+    };
      |
      |     if (!emit) {
      |         return;
      @@ -351,19 +266,7 @@ void qapi_event_send_block_job_error(con
      |     if (err) {
      |         goto out;
      |     }
      |-    visit_type_str(v, "device", (char **)&device, &err);
      |-    if (err) {
      |-        goto out_obj;
      |-    }
      |-    visit_type_IoOperationType(v, "operation", &operation, &err);
      |-    if (err) {
      |-        goto out_obj;
      |-    }
      |-    visit_type_BlockErrorAction(v, "action", &action, &err);
      |-    if (err) {
      |-        goto out_obj;
      |-    }
      |-out_obj:
      |+    visit_type_q_obj_BLOCK_JOB_ERROR_arg_members(v, &param, &err);
      |     visit_end_struct(v, err ? NULL : &err);
      
      Notice that the initialization of 'param' has to cast away const
      (just as the old gen_visit_members() had to do): we can't change
      the signature of the user function (which uses 'const char *'), but
      have to assign it to a non-const QAPI object (which requires
      'char *').
      
      While touching this, document with a FIXME comment that there is
      still a potential collision between QMP members and our choice of
      local variable names within qapi_event_send_FOO().
      
      This patch also paves the way for some followup simplifications
      in the generator, in subsequent patches.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1458254921-17042-8-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      0949e95b
    • E
      qapi-event: Drop qmp_output_get_qobject() null check · 8df59565
      Eric Blake 提交于
      qmp_output_get_qobject() was changed never to return null some time
      ago (in commit 6c2f9a15), but the qapi_event_send_FOO() functions
      still check.  Clean that up:
      
      |@@ -28,7 +28,6 @@ void qapi_event_send_acpi_device_ost(ACP
      |     QMPEventFuncEmit emit;
      |     QmpOutputVisitor *qov;
      |     Visitor *v;
      |-    QObject *obj;
      |
      |     emit = qmp_event_get_func_emit();
      |     if (!emit) {
      |@@ -54,10 +53,7 @@ out_obj:
      |         goto out;
      |     }
      |
      |-    obj = qmp_output_get_qobject(qov);
      |-    g_assert(obj);
      |-
      |-    qdict_put_obj(qmp, "data", obj);
      |+    qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov));
      |     emit(QAPI_EVENT_ACPI_DEVICE_OST, qmp, &err);
      |
      | out:
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1458254921-17042-7-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      8df59565
    • E
      qapi: Emit implicit structs in generated C · 7ce106a9
      Eric Blake 提交于
      We already have several places that want to visit all the members
      of an implicit object within a larger context (simple union variant,
      event with anonymous data, command with anonymous arguments struct);
      and will be adding another one soon (the ability to declare an
      anonymous base for a flat union).  Having a C struct declared for
      these implicit types, along with a visit_type_FOO_members() helper
      function, will make for fewer special cases in our generator.
      
      We do not, however, need qapi_free_FOO() or visit_type_FOO()
      functions for implicit types, because they should not be used
      directly outside of the generated code.  This is done by adding a
      conditional in visit_object_type() for both qapi-types.py and
      qapi-visit.py based on the object name.  The comparison of
      "name.startswith('q_')" is a bit hacky (it's basically duplicating
      what .is_implicit() already uses), but beats changing the signature
      of the visit_object_type() callback to pass a new 'implicit' flag.
      The hack should be temporary: we are considering adding a future
      patch that consolidates the narrow visit_object_type(..., base,
      local_members, variants) and visit_object_type_flat(...,
      all_members, variants) [where different sets of information are
      already broken out, and the QAPISchemaObjectType is no longer
      available] into a broader visit_object_type(obj_type) [where the
      visitor can query the needed fields from obj_type directly].
      
      Also, now that we WANT to output C code for implicits, we no longer
      need the visit_needed() filter, leaving 'q_empty' as the only object
      still needing a special case.  Remember, 'q_empty' is the only
      built-in generated object, which means that without a special case
      it would be emitted in multiple files (the main qapi-types.h and in
      qga-qapi-types.h) causing compilation failure due to redefinition.
      But since it has no members, it's easier to just avoid an attempt to
      visit that particular type; since gen_object() is called recursively,
      we also prime the objects_seen set to cover any recursion into the
      empty type.
      
      The patch relies on the changed naming of implicit types in the
      previous patch.  It is a bit unfortunate that the generated struct
      names and visit_type_FOO_members() don't match normal naming
      conventions, but it's not too bad, since they will only be used in
      generated code.
      
      The generated code grows substantially in size: the implicit
      '-wrapper' types must be emitted in qapi-types.h before any union
      can include an unboxed member of that type.  Arguably, the '-args'
      types could be emitted in a private header for just qapi-visit.c
      and qmp-marshal.c, rather than polluting qapi-types.h; but adding
      complexity to the generator to split the output location according
      to role doesn't seem worth the maintenance costs.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1458254921-17042-6-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      7ce106a9
    • E
      qapi: Adjust names of implicit types · 7599697c
      Eric Blake 提交于
      The original choice of ':obj-' as the prefix for implicit types
      made it obvious that we weren't going to clash with any user-defined
      names, which cannot contain ':'.  But now we want to create structs
      for implicit types, to get rid of special cases in the generators,
      and our use of ':' in implicit names needs a tweak to produce valid
      C code.
      
      We could transliterate ':' to '_', except that C99 mandates that
      "identifiers that begin with an underscore are always reserved for
      use as identifiers with file scope in both the ordinary and tag name
      spaces".  So it's time to change our naming convention: we can
      instead use the 'q_' prefix that we reserved for ourselves back in
      commit 9fb081e0.  Technically, since we aren't planning on exposing
      the empty type in generated code, we could keep the name ':empty',
      but renaming it to 'q_empty' makes the check for startswith('q_')
      cover all implicit types, whether or not code is generated for them.
      
      As long as we don't declare 'empty' or 'obj' ticklish, it shouldn't
      clash with c_name() prepending 'q_' to the user's ticklish names.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1458254921-17042-5-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      7599697c
    • E
      qapi: Make c_type() more OO-like · 4040d995
      Eric Blake 提交于
      QAPISchemaType.c_type() is a bit awkward: it takes two optional
      boolean flags is_param and is_unboxed, and they should never both
      be True.
      
      Add a new method for each of the flags, and drop the flags from
      c_type().
      
      Most callers pass no flags; they remain unchanged.
      
      One caller passes is_param=True; call the new .c_param_type()
      instead.
      
      One caller passes is_unboxed=True, except for simple union types.
      This is actually an ugly special case that will go away soon, so
      until then, we now have to call either .c_type() or the new
      .c_unboxed_type().  Tolerable in the interim.
      
      It requires slightly more Python, but is arguably easier to read.
      Suggested-by: NMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1458254921-17042-4-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      4040d995
    • E
      qapi: Fix command with named empty argument type · 972a1101
      Eric Blake 提交于
      The generator special-cased
      
       { 'command':'foo', 'data': {} }
      
      to avoid emitting a visitor variable, but failed to see that
      
       { 'struct':'NamedEmptyType, 'data': {} }
       { 'command':'foo', 'data':'NamedEmptyType' }
      
      needs the same treatment.  There, the generator happily generates a
      visitor to get no arguments, and a visitor to destroy no arguments;
      and the compiler isn't happy with that, as demonstrated by the updated
      qapi-schema-test.json:
      
        tests/test-qmp-marshal.c: In function ‘qmp_marshal_user_def_cmd0’:
        tests/test-qmp-marshal.c:264:14: error: variable ‘v’ set but not used [-Werror=unused-but-set-variable]
             Visitor *v;
                      ^
      
      No change to generated code except for the testsuite addition.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1458254921-17042-3-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      972a1101
    • E
      qapi: Assert in places where variants are not handled · 29f6bd15
      Eric Blake 提交于
      We are getting closer to the point where we could use one union
      as the base or variant type within another union type (as long
      as there are no collisions between any possible combination of
      member names allowed across all discriminator choices).  But
      until we get to that point, it is worth asserting that variants
      are not present in places where we are not prepared to handle
      them: when exploding a type into a parameter list, we do not
      expect variants.  The qapi.py code is already checking this,
      via the older check_type() method; but someday we hope to get
      rid of that and move checking into QAPISchema*.check().  The
      two asserts added here make sure any refactoring still catches
      problems, and makes it locally obvious why we can iterate over
      only type.members without worrying about type.variants.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1458254921-17042-2-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      29f6bd15
  2. 17 3月, 2016 28 次提交