aboutsummaryrefslogtreecommitdiff
path: root/scripts/qapi.py
AgeCommit message (Collapse)AuthorFilesLines
2017-01-16qapi: add qapi2texi scriptMarc-André Lureau1-2/+249
As the name suggests, the qapi2texi script converts JSON QAPI description into a texi file suitable for different target formats (info/man/txt/pdf/html...). It parses the following kind of blocks: Free-form: ## # = Section # == Subsection # # Some text foo with *emphasis* # 1. with a list # 2. like that # # And some code: # | $ echo foo # | -> do this # | <- get that # ## Symbol description: ## # @symbol: # # Symbol body ditto ergo sum. Foo bar # baz ding. # # @param1: the frob to frobnicate # @param2: #optional how hard to frobnicate # # Returns: the frobnicated frob. # If frob isn't frobnicatable, GenericError. # # Since: version # Notes: notes, comments can have # - itemized list # - like this # # Example: # # -> { "execute": "quit" } # <- { "return": {} } # ## That's roughly following the following EBNF grammar: api_comment = "##\n" comment "##\n" comment = freeform_comment | symbol_comment freeform_comment = { "# " text "\n" | "#\n" } symbol_comment = "# @" name ":\n" { member | tag_section | freeform_comment } member = "# @" name ':' [ text ] "\n" freeform_comment tag_section = "# " ( "Returns:", "Since:", "Note:", "Notes:", "Example:", "Examples:" ) [ text ] "\n" freeform_comment text = free text with markup Note that the grammar is ambiguous: a line "# @foo:\n" can be parsed both as freeform_comment and as symbol_comment. The actual parser recognizes symbol_comment. See docs/qapi-code-gen.txt for more details. Deficiencies and limitations: - the generated QMP documentation includes internal types - union type support is lacking - type information is lacking in generated documentation - doc comment error message positions are imprecise, they point to the beginning of the comment. - a few minor issues, all marked TODO/FIXME in the code Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-Id: <20170113144135.5150-16-marcandre.lureau@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> [test-qapi.py tweaked to avoid trailing empty lines in .out] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2017-01-16qapi: rework qapi ExceptionMarc-André Lureau1-178/+156
Use a base class QAPIError, and QAPIParseError for parser errors and QAPISemError for semantic errors, suggested by Markus Armbruster. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20170113144135.5150-12-marcandre.lureau@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-07-19qapi: Implement boxed types for commands/eventsEric Blake1-13/+50
Turn on the ability to pass command and event arguments in a single boxed parameter, which must name a non-empty type (although the type can be a struct with all optional members). For structs, it makes it possible to pass a single qapi type instead of a breakout of all struct members (useful if the arguments are already in a struct or if the number of members is large); for other complex types, it is now possible to use a union or alternate as the data for a command or event. The empty type may be technically feasible if needed down the road, but it's easier to forbid it now and relax things to allow it later, than it is to allow it now and have to special case how the generated 'q_empty' type is handled (see commit 7ce106a9 for reasons why nothing is generated for the empty type). An alternate type is never considered empty, but now that a boxed type can be either an object or an alternate, we have to provide a trivial QAPISchemaAlternateType.is_empty(). The new call to arg_type.is_empty() during QAPISchemaCommand.check() requires that we first check the type in question; but there is no chance of introducing a cycle since objects do not refer back to commands. We still have a split in syntax checking between ad-hoc parsing up front (merely validates that 'boxed' has a sane value) and during .check() methods (if 'boxed' is set, then 'data' must name a non-empty user-defined type). Generated code is unchanged, as long as no client uses the new feature. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1468468228-27827-10-git-send-email-eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> [Test files renamed to *-boxed-*] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-07-19qapi: Plumb in 'boxed' to qapi generator lower levelsEric Blake1-16/+27
The next patch will add support for passing a qapi union type as the 'data' of a command. But to do that, the user function for implementing the command, as called by the generated marshal command, must take the corresponding C struct as a single boxed pointer, rather than a breakdown into one parameter per member. Even without a union, being able to use a C struct rather than a list of parameters can make it much easier to handle coding with QAPI. This patch adds the internal plumbing of a 'boxed' flag associated with each command and event. In several cases, this means adding indentation, with one new dead branch and the remaining branch being the original code more deeply nested; this was done so that the new implementation in the next patch is easier to review without also being mixed with indentation changes. For this patch, no behavior or generated output changes, other than the testsuite outputting the value of the new flag (always False for now). Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1468468228-27827-9-git-send-email-eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> [Identifier box renamed to boxed in two places] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-07-19qapi: Drop useless gen_err_check()Eric Blake1-8/+0
Ever since commit 12f254f removed the last parameterization of gen_err_check(), it no longer makes sense to hide the three lines of generated C code behind a macro call. Just inline it into the remaining users. No change to generated code. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1468468228-27827-7-git-send-email-eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-07-19qapi: Add type.is_empty() helperEric Blake1-0/+4
In the near future, we want to lift our artificial restriction of no variants at the top level of an event, at which point the currently open-coded check for empty members will become insufficient. Factor it out into a new helper method is_empty() now, and future-proof it by checking variants, too, along with an assert that it is not used prior to the completion of .check(). Update places that were checking for (non-)empty .members to use the new helper. All of the current callers assert that there are no variants (either directly, or by qapi.py asserting that base types have no variants), so this is not a semantic change. No change to generated code. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1468468228-27827-6-git-send-email-eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-07-19qapi: Hide tag_name data member of variantsEric Blake1-4/+4
Clean up the only remaining external use of the tag_name field of QAPISchemaObjectTypeVariants, by explicitly listing the generated 'type' tag for all variants in the testsuite (you can still tell simple unions by the -wrapper types). Then we can mark the tag_name field as private by adding a leading underscore to prevent any further use. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1468468228-27827-5-git-send-email-eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-07-19qapi: Special case c_name() for empty typeEric Blake1-0/+1
Commit 7ce106a rendered QAPISchemaObjectType.c_name() redundant, since it now does nothing more than delegate to its superclass. However, rather than deleting it, we can restore part of the assertion that was removed in that commit, to prove that we never emit the empty type directly in generated code, but rather special-case it as a built-in that makes other aspects of code generation easier to reason about. Reported-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1468468228-27827-4-git-send-email-eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-07-19qapi: Require all branches of flat union enum to be coveredEric Blake1-0/+8
We were previously enforcing that all flat union branches were found in the corresponding enum, but not that all enum values were covered by branches. The resulting generated code would abort() if the user passes the uncovered enum value. We don't automatically treat non-present branches in a flat union as empty types, for symmetry with simple unions (there, the enum type is generated from the list of all branches, so there is no way to omit a branch but still have it be part of the union). A later patch will add shorthand so that branches that are empty in flat unions can be declared as 'branch':{} instead of 'branch':'Empty', to avoid the need for an otherwise useless explicit empty type. [Such shorthand for simple unions is a bit harder to justify, since we would still have to generate a wrapper type that parses 'data':{}, rather than truly being an empty branch with no additional siblings to the 'type' member.] Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1468468228-27827-3-git-send-email-eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-03-18qapi: Use anonymous bases in QMP flat unionsEric Blake1-1/+1
Now that the generator supports it, we might as well use an anonymous base rather than breaking out a single-use Base structure, for all three of our current QMP flat unions. Oddly enough, this change does not affect the resulting introspection output (because we already inline the members of a base type into an object, and had no independent use of the base type reachable from a command). The case_whitelist now has to list the name of an implicit type; which is not too bad (consider it a feature if it makes it harder for developers to make the whitelist grow :) Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1458254921-17042-16-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-03-18qapi: Allow anonymous base for flat unionEric Blake1-2/+10
Rather than requiring all flat unions to explicitly create a separate base struct, we can allow the qapi schema to specify the common members via an inline dictionary. This is similar to how commands can specify an inline anonymous type for its 'data'. We already have several struct types that only exist to serve as a single flat union's base; the next commit will clean them up. In particular, this patch's change to the BlockdevOptions example in qapi-code-gen.txt will actually be done in the real QAPI schema. Now that anonymous bases are legal, we need to rework the flat-union-bad-base negative test (as previously written, it forms what is now valid QAPI; tweak it to now provide coverage of a new error message path), and add a positive test in qapi-schema-test to use an anonymous base (making the integer argument optional, for even more coverage). Note that this patch only allows anonymous bases for flat unions; simple unions are already enough syntactic sugar that we do not want to burden them further. Meanwhile, while it would be easy to also allow an anonymous base for structs, that would be quite redundant, as the members can be put right into the struct instead. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1458254921-17042-15-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-03-18qapi: Don't special-case simple union wrappersEric Blake1-10/+0
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: Eric Blake <eblake@redhat.com> Message-Id: <1458254921-17042-13-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-03-18qapi: Drop unused c_null()Eric Blake1-29/+17
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: Eric Blake <eblake@redhat.com> Message-Id: <1458254921-17042-12-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-03-18qapi: Inline gen_visit_members() into lone callerEric Blake1-44/+2
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: Markus Armbruster <armbru@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1458254921-17042-11-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-03-18qapi: Emit implicit structs in generated CEric Blake1-2/+0
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: Eric Blake <eblake@redhat.com> Message-Id: <1458254921-17042-6-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-03-18qapi: Adjust names of implicit typesEric Blake1-8/+10
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: Eric Blake <eblake@redhat.com> Message-Id: <1458254921-17042-5-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-03-18qapi: Make c_type() more OO-likeEric Blake1-9/+30
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: Markus Armbruster <armbru@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1458254921-17042-4-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-03-05qapi: Rename 'fields' to 'members' in generatorEric Blake1-10/+10
C types and JSON objects don't have fields, but members. We shouldn't gratuitously invent terminology. This patch is a strict renaming of generator code internals (including testsuite comments), before later patches rename C interfaces. No change to generated code with this patch. Suggested-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1457021813-10704-2-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-03-01qapi: rename InputAxis values.Gerd Hoffmann1-1/+0
Lowercase them. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2016-03-01qapi: rename input buttonsGerd Hoffmann1-1/+0
All lowercase, use-dash instead of CamelCase. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2016-02-19qapi: Don't box struct branch of alternateEric Blake1-5/+7
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 <eblake@redhat.com> Message-Id: <1455778109-6278-13-git-send-email-eblake@redhat.com> [Commit message tweaked] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-02-19qapi: Forbid 'any' inside an alternateEric Blake1-1/+4
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 <eblake@redhat.com> Message-Id: <1455778109-6278-4-git-send-email-eblake@redhat.com> [Commit message tweaked] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-02-19qapi: Forbid empty unions and useless alternatesEric Blake1-2/+10
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 <eblake@redhat.com> Message-Id: <1455778109-6278-3-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-02-08qapi: Fix compilation failure on MIPS and SPARCEric Blake1-1/+1
Commit 86f4b687 broke compilation on MIPS and SPARC, which have a preprocessor pollution of '#define mips 1' and '#define sparc 1', respectively. Treat it the same way as we do for the pollution with 'unix', so that QMP remains backwards compatible and only the C code needs to use the alternative 'q_mips', 'q_sparc' spelling. CC: James Hogan <james.hogan@imgtec.com> CC: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Eric Blake <eblake@redhat.com> Tested-by: James Hogan <james.hogan@imgtec.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-02-08qapi: Swap visit_* arguments for consistent 'name' placementEric Blake1-2/+2
JSON uses "name":value, but many of our visitor interfaces were called with visit_type_FOO(v, &value, name, errp). This can be a bit confusing to have to mentally swap the parameter order to match JSON order. It's particularly bad for visit_start_struct(), where the 'name' parameter is smack in the middle of the otherwise-related group of 'obj, kind, size' parameters! It's time to do a global swap of the parameter ordering, so that the 'name' parameter is always immediately after the Visitor argument. Additional reason in favor of the swap: the existing include/qjson.h prefers listing 'name' first in json_prop_*(), and I have plans to unify that file with the qapi visitors; listing 'name' first in qapi will minimize churn to the (admittedly few) qjson.h clients. Later patches will then fix docs, object.h, visitor-impl.h, and those clients to match. Done by first patching scripts/qapi*.py by hand to make generated files do what I want, then by running the following Coccinelle script to affect the rest of the code base: $ spatch --sp-file script `git grep -l '\bvisit_' -- '**/*.[ch]'` I then had to apply some touchups (Coccinelle insisted on TAB indentation in visitor.h, and botched the signature of visit_type_enum() by rewriting 'const char *const strings[]' to the syntactically invalid 'const char*const[] strings'). The movement of parameters is sufficient to provoke compiler errors if any callers were missed. // Part 1: Swap declaration order @@ type TV, TErr, TObj, T1, T2; identifier OBJ, ARG1, ARG2; @@ void visit_start_struct -(TV v, TObj OBJ, T1 ARG1, const char *name, T2 ARG2, TErr errp) +(TV v, const char *name, TObj OBJ, T1 ARG1, T2 ARG2, TErr errp) { ... } @@ type bool, TV, T1; identifier ARG1; @@ bool visit_optional -(TV v, T1 ARG1, const char *name) +(TV v, const char *name, T1 ARG1) { ... } @@ type TV, TErr, TObj, T1; identifier OBJ, ARG1; @@ void visit_get_next_type -(TV v, TObj OBJ, T1 ARG1, const char *name, TErr errp) +(TV v, const char *name, TObj OBJ, T1 ARG1, TErr errp) { ... } @@ type TV, TErr, TObj, T1, T2; identifier OBJ, ARG1, ARG2; @@ void visit_type_enum -(TV v, TObj OBJ, T1 ARG1, T2 ARG2, const char *name, TErr errp) +(TV v, const char *name, TObj OBJ, T1 ARG1, T2 ARG2, TErr errp) { ... } @@ type TV, TErr, TObj; identifier OBJ; identifier VISIT_TYPE =~ "^visit_type_"; @@ void VISIT_TYPE -(TV v, TObj OBJ, const char *name, TErr errp) +(TV v, const char *name, TObj OBJ, TErr errp) { ... } // Part 2: swap caller order @@ expression V, NAME, OBJ, ARG1, ARG2, ERR; identifier VISIT_TYPE =~ "^visit_type_"; @@ ( -visit_start_struct(V, OBJ, ARG1, NAME, ARG2, ERR) +visit_start_struct(V, NAME, OBJ, ARG1, ARG2, ERR) | -visit_optional(V, ARG1, NAME) +visit_optional(V, NAME, ARG1) | -visit_get_next_type(V, OBJ, ARG1, NAME, ERR) +visit_get_next_type(V, NAME, OBJ, ARG1, ERR) | -visit_type_enum(V, OBJ, ARG1, ARG2, NAME, ERR) +visit_type_enum(V, NAME, OBJ, ARG1, ARG2, ERR) | -VISIT_TYPE(V, OBJ, NAME, ERR) +VISIT_TYPE(V, NAME, OBJ, ERR) ) Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-Id: <1454075341-13658-19-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-02-08qapi: Improve generated event use of qapi visitorEric Blake1-2/+3
All other successful clients of visit_start_struct() were paired with an unconditional visit_end_struct(); but the generated code for events was relying on qmp_output_visitor_cleanup() to work on an incomplete visit. Alter the code to guarantee that the struct is completed, which will make a future patch to split visit_end_struct() easier to reason about. While at it, drop some assertions and comments that are not present in other uses of the qmp output visitor, and pass NULL rather than "" as the 'kind' parameter (matching most other uses where obj is NULL). The changes to the generated code look like: | qmp = qmp_event_build_dict("DEVICE_TRAY_MOVED"); | | qov = qmp_output_visitor_new(); |- g_assert(qov); |- | v = qmp_output_get_visitor(qov); |- g_assert(v); | |- /* Fake visit, as if all members are under a structure */ |- visit_start_struct(v, NULL, "", "DEVICE_TRAY_MOVED", 0, &err); |+ visit_start_struct(v, NULL, NULL, "DEVICE_TRAY_MOVED", 0, &err); | if (err) { | goto out; | } | visit_type_str(v, (char **)&device, "device", &err); | if (err) { |- goto out; |+ goto out_obj; | } | visit_type_bool(v, &tray_open, "tray-open", &err); | if (err) { |- goto out; |+ goto out_obj; | } |- visit_end_struct(v, &err); |+out_obj: |+ visit_end_struct(v, err ? NULL : &err); | if (err) { | goto out; | } | | obj = qmp_output_get_qobject(qov); |- g_assert(obj != NULL); |+ g_assert(obj); | | qdict_put_obj(qmp, "data", obj); | emit(QAPI_EVENT_DEVICE_TRAY_MOVED, qmp, &err); Note that the 'goto out_obj' with no intervening code before the label, as well as the construct of 'err ? NULL : &err', are both a bit unusual but also temporary; they get fixed in a later patch that splits visit_end_struct() to drop its errp parameter by moving some checking before the label. But until that time, this was the simplest way to avoid the appearance of passing a possibly-set error to visit_end_struct(), even though actual code inspection shows that visit_end_struct() for a QMP output visitor will never set an error. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1454075341-13658-11-git-send-email-eblake@redhat.com> [Commit message's code diff tweaked] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-02-08qapi: Drop dead parameter in gen_params()Eric Blake1-2/+2
Commit 5cdc8831 reworked gen_params() to be simpler, but forgot to clean up a now-unused errp named argument. No change to generated code. Reported-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1454075341-13658-6-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-02-08qapi: Use Python 2.6 "except E as ..." syntaxMarkus Armbruster1-4/+4
PEP 8 calls for it, because it's forward compatible with Python 3. Supported since Python 2.6, which we require (commit fec2103). Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Acked-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <1450425164-24969-2-git-send-email-armbru@redhat.com>
2015-12-17qapi: Detect base class loopsEric Blake1-1/+3
It should be fairly obvious that qapi base classes need to form an acyclic graph, since QMP cannot specify the same key more than once, while base classes are included as flat members alongside other members added by the child. But the old check_member_clash() parser function was not prepared to check for this, and entered an infinite recursion (at least until Python gives up, complaining about nesting too deep). Now that check_member_clash() has been recently removed, attempts at self-inheritance trigger an assertion failure introduced by commit ac88219a. The obvious fix is to turn the assertion into a conditional. This patch includes both the tests (base-cycle-direct and base-cycle-indirect) and the fix, since the .err file output for the unfixed case is not useful (particularly when it was warning about unbounded recursion, as that limit may be platform-specific). We don't need to worry about cycles in flat unions (neither the base type nor the type of a variant can be a union) nor in alternates (alternate branches cannot themselves be an alternate). But if we later allow a union type as a variant, we will still be okay, as QAPISchemaObjectTypeVariants.check() triggers the same QAPISchemaObjectType.check() that will detect any loops. Likewise, we need not worry about the case of diamond inheritance where the same class is used for a flat union base class and one of its variants; either both uses will introduce a collision in trying to insert the same member name twice, or the shared type is empty and changes nothing. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-16-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Move duplicate collision checks to schema check()Eric Blake1-50/+1
With the recent commit 'qapi: Detect collisions in C member names', we have two different locations for detecting clashes - one at parse time, and another at QAPISchema*.check() time. Remove all of the ad hoc parser checks, and delete associated code (for example, the global check_member_clash() method is no longer needed). Testing this showed that the test union-bad-branch wasn't adding much: union-clash-branches also exposes the error message when branches collide, and we've recently fixed things to avoid an implicit collision with max. Likewise, the error for enum-clash-member changes to report our new detection of upper case in a value name, unless we modify the test to use all lower case. The wording of several error messages has changed, but the change is generally an improvement rather than a regression. No change to generated code. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-15-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Enforce (or whitelist) case conventions on qapi membersEric Blake1-0/+17
We document that members of enums and objects should be 'lower-case', although we were not enforcing it. We have to whitelist a few pre-existing entities that violate the norms. Add three new tests to expose the new error message, each of which first uses the whitelisted name 'UuidInfo' to prove the whitelist works, then triggers the failure (this is the same pattern used in the existing returns-whitelist.json test). Note that by adding this check, we have effectively forbidden an entity with a case-insensitive clash of member names, for any entity that is not on the whitelist (although there is still the possibility to clash via '-' vs. '_'). Not done here: a future patch should also add naming convention support and whitelist exceptions for command, event, and type names. The additions to QAPISchemaMember.check_clash() check whether info['name'] is in the whitelist (the top-most entity name at the point 'info' tracks), rather than self.owner (the type, possibly implicit, that directly owns the member), because it is easier to maintain the whitelist by the names actually in the user's .json file, rather than worrying about the names of implicit types. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-14-git-send-email-eblake@redhat.com> [Simplified a bit as per discussion with Eric] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Track enum values by QAPISchemaMember, not stringEric Blake1-11/+26
Rather than using just an array of strings, make enum.values be an array of the new QAPISchemaMember type, and add a helper member_names() method to get back at the original list of names. Likewise, creating an enum requires wrapping strings, via a new QAPISchema._make_enum_members() method. The benefit of wrapping enum members in a QAPISchemaMember Python object is that we now share the existing code for C name clash detection (although the code is not yet active until a later commit removes the earlier ad hoc parser checks). In a related change, the QAPISchemaMember._pretty_owner() method needs to learn about one more implicit type name: the generated enum associated with a simple union. In the interest of keeping the changes of this patch local to one file, the visitor interface still passes just a list of names rather than the full list of QAPISchemaMember instances. We may want to revisit this in the future, if the consistency with visit_object_type() is worth it. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-12-git-send-email-eblake@redhat.com> [Eric's simplifying followup squashed in] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Prepare new QAPISchemaMember base classEric Blake1-12/+17
We want to share some clash detection code between enum values and object type members. To assist with that, split off part of QAPISchemaObjectTypeMember into a new base class QAPISchemaMember that tracks name, owner, and common clash detection code; while the former keeps the additional fields for type and optional flag. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-11-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Shorter visits of optional fieldsEric Blake1-2/+1
For less code, reflect the determined boolean value of an optional visit back to the caller instead of making the caller read the boolean after the fact. The resulting generated code has the following diff: |- visit_optional(v, &has_fdset_id, "fdset-id"); |- if (has_fdset_id) { |+ if (visit_optional(v, &has_fdset_id, "fdset-id")) { | visit_type_int(v, &fdset_id, "fdset-id", &err); | if (err) { | goto out; | } | } Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-10-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Simplify visits of optional fieldsEric Blake1-6/+2
None of the visitor callbacks would set an error when testing if an optional field was present; make this part of the interface contract by eliminating the errp argument. The resulting generated code has a nice diff: |- visit_optional(v, &has_fdset_id, "fdset-id", &err); |- if (err) { |- goto out; |- } |+ visit_optional(v, &has_fdset_id, "fdset-id"); | if (has_fdset_id) { | visit_type_int(v, &fdset_id, "fdset-id", &err); | if (err) { | goto out; | } | } Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-9-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Inline _make_implicit_tag()Eric Blake1-6/+3
Now that alternates no longer use an implicit tag, we can inline _make_implicit_tag() into its one caller, _def_union_type(). No change to generated code. Suggested-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-7-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Simplify visiting of alternate typesEric Blake1-5/+13
Previously, working with alternates required two lookup arrays and some indirection: for type Foo, we created Foo_qtypes[] which maps each qtype to a value of the generated FooKind enum, then look up that value in FooKind_lookup[] like we do for other union types. This has a couple of subtle bugs. First, the generator was creating a call with a parameter '(int *) &(*obj)->type' where type is an enum type; this is unsafe if the compiler chooses to store the enum type in a different size than int, where assigning through the wrong size pointer can corrupt data or cause a SIGBUS. Related bug, not not fixed in this patch: qapi-visit.py's gen_visit_enum() generates a cast of its enum * argument to int *. Marked FIXME. Second, since the values of the FooKind enum start at zero, all entries of the Foo_qtypes[] array that were not explicitly initialized will map to the same branch of the union as the first member of the alternate, rather than triggering a desired failure in visit_get_next_type(). Fortunately, the bug seldom bites; the very next thing the input visitor does is try to parse the incoming JSON with the wrong parser, which normally fails; the output visitor is not used with a C struct in that state, and the dealloc visitor has nothing to clean up (so there is no leak). However, the second bug IS observable in one case: parsing an integer causes unusual behavior in an alternate that contains at least a 'number' member but no 'int' member, because the 'number' parser accepts QTYPE_QINT in addition to the expected QTYPE_QFLOAT (that is, since 'int' is not a member, the type QTYPE_QINT accidentally maps to FooKind 0; if this enum value is the 'number' branch the integer parses successfully, but if the 'number' branch is not first, some other branch tries to parse the integer and rejects it). A later patch will worry about fixing alternates to always parse all inputs that a non-alternate 'number' would accept, for now this is still marked FIXME in the updated test-qmp-input-visitor.c, to merely point out that new undesired behavior of 'ans' matches the existing undesired behavior of 'asn'. This patch fixes the default-initialization bug by deleting the indirection, and modifying get_next_type() to directly assign a QTypeCode parameter. This in turn fixes the type-casting bug, as we are no longer casting a pointer to enum to a questionable size. There is no longer a need to generate an implicit FooKind enum associated with the alternate type (since the QMP wire format never uses the stringized counterparts of the C union member names). Since the updated visit_get_next_type() does not know which qtypes are expected, the generated visitor is modified to generate an error statement if an unexpected type is encountered. Callers now have to know the QTYPE_* mapping when looking at the discriminator; but so far, only the testsuite was even using the C struct of an alternate types. I considered the possibility of keeping the internal enum FooKind, but initialized differently than most generated arrays, as in: typedef enum FooKind { FOO_KIND_A = QTYPE_QDICT, FOO_KIND_B = QTYPE_QINT, } FooKind; to create nicer aliases for knowing when to use foo->a or foo->b when inspecting foo->type; but it turned out to add too much complexity, especially without a client. There is a user-visible side effect to this change, but I consider it to be an improvement. Previously, the invalid QMP command: {"execute":"blockdev-add", "arguments":{"options": {"driver":"raw", "id":"a", "file":true}}} failed with: {"error": {"class": "GenericError", "desc": "Invalid parameter type for 'file', expected: QDict"}} (visit_get_next_type() succeeded, and the error comes from the visit_type_BlockdevOptions() expecting {}; there is no mention of the fact that a string would also work). Now it fails with: {"error": {"class": "GenericError", "desc": "Invalid parameter type for 'file', expected: BlockdevRef"}} (the error when the next type doesn't match any expected types for the overall alternate). Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-5-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Convert QType into QAPI built-in enum typeEric Blake1-0/+6
What's more meta than using qapi to define qapi? :) Convert QType into a full-fledged[*] builtin qapi enum type, so that a subsequent patch can then use it as the discriminator type of qapi alternate types. Fortunately, the judicious use of 'prefix' in the qapi definition avoids churn to the spelling of the enum constants. To avoid circular definitions, we have to flip the order of inclusion between "qobject.h" vs. "qapi-types.h". Back in commit 28770e0, we had the latter include the former, so that we could use 'QObject *' for our implementation of 'any'. But that usage also works with only a forward declaration, whereas the definition of QObject requires QType to be a complete type. [*] The type has to be builtin, rather than declared in qapi/common.json, because we want to use it for alternates even when common.json is not included. But since it is the first builtin enum type, we have to add special cases to qapi-types and qapi-visit to only emit definitions once, even when two qapi files are being compiled into the same binary (the way we already handled builtin list types like 'intList'). We may need to revisit how multiple qapi files share common types, but that's a project for another day. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-4-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qobject: Rename qtype_code to QTypeEric Blake1-1/+1
The name QType matches our CODING_STYLE conventions for type names in CamelCase. It also matches the fact that we are already naming all the enum members with a prefix of QTYPE, not QTYPE_CODE. And doing the rename will also make it easier for the next patch to use QAPI for providing the enum, which also wants CamelCase type names. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-3-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Change munging of CamelCase enum valuesEric Blake1-1/+1
When munging enum values, the fact that we were passing the entire prefix + value through camel_to_upper() meant that enum values spelled with CamelCase could be turned into CAMEL_CASE. However, this provides a potential collision (both OneTwo and One-Two would munge into ONE_TWO) for enum types, when the same two names are valid side-by-side as QAPI member names. By changing the generation of enum constants to always be prefix + '_' + c_name(value, False).upper(), and ensuring that there are no case collisions (in the next patches), we no longer have to worry about names that would be distinct as QAPI members but collide as variant tag names, without having to think about what munging the heuristics in camel_to_upper() will actually perform on an enum value. Making the change will affect enums that did not follow coding conventions, using 'CamelCase' rather than desired 'lower-case'. Thankfully, there are only two culprits: InputButton and ErrorClass. We already tweaked ErrorClass to make it an alias of QapiErrorClass, where only the alias needs changing rather than the whole tree. So the bulk of this change is modifying INPUT_BUTTON_WHEEL_UP to the new INPUT_BUTTON_WHEELUP (and likewise for WHEELDOWN). That part of this commit may later need reverting if we rename the enum constants from 'WheelUp' to 'wheel-up' as part of moving x-input-send-event to a stable interface; but at least we have documentation bread crumbs in place to remind us (commit 513e7cd), and it matches the fact that SDL constants are also spelled SDL_BUTTON_WHEELUP. Suggested by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1447836791-369-27-git-send-email-eblake@redhat.com> [Commit message tweaked] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Remove obsolete tests for MAX collisionEric Blake1-5/+3
Now that we no longer collide with an implicit _MAX enum member, we no longer need to reject it in the ad hoc parser, and can remove several tests that are no longer needed. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1447836791-369-24-git-send-email-eblake@redhat.com> [Commit message tweaked] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Don't let implicit enum MAX member collideEric Blake1-3/+3
Now that we guarantee the user doesn't have any enum values beginning with a single underscore, we can use that for our own purposes. Renaming ENUM_MAX to ENUM__MAX makes it obvious that the sentinel is generated. This patch was mostly generated by applying a temporary patch: |diff --git a/scripts/qapi.py b/scripts/qapi.py |index e6d014b..b862ec9 100644 |--- a/scripts/qapi.py |+++ b/scripts/qapi.py |@@ -1570,6 +1570,7 @@ const char *const %(c_name)s_lookup[] = { | max_index = c_enum_const(name, 'MAX', prefix) | ret += mcgen(''' | [%(max_index)s] = NULL, |+// %(max_index)s | }; | ''', | max_index=max_index) then running: $ cat qapi-{types,event}.c tests/test-qapi-types.c | sed -n 's,^// \(.*\)MAX,s|\1MAX|\1_MAX|g,p' > list $ git grep -l _MAX | xargs sed -i -f list The only things not generated are the changes in scripts/qapi.py. Rejecting enum members named 'MAX' is now useless, and will be dropped in the next patch. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1447836791-369-23-git-send-email-eblake@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> [Rebased to current master, commit message tweaked] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Tighten the regex on valid namesEric Blake1-5/+7
We already documented that qapi names should match specific patterns (such as starting with a letter unless it was an enum value or a downstream extension). Tighten that from a suggestion into a hard requirement, which frees up names beginning with a single underscore for qapi internal usage. The tighter regex doesn't forbid everything insane that a user could provide (for example, a user could name a type 'Foo-lookup' to collide with the generated 'Foo_lookup[]' for an enum 'Foo'), but does a good job at protecting the most obvious uses, and also happens to reserve single leading underscore for later use. The handling of enum values starting with a digit is tricky: commit 9fb081e introduced a subtle bug by using c_name() on a munged value, which would allow an enum to include the member 'q-int' in spite of our reservation. Furthermore, munging with a leading '_' would fail our tighter regex. So fix it by only munging for leading digits (which are never ticklish in c_name()) and by using a different prefix (I picked 'D', although any letter should do). Add new tests, reserved-member-underscore and reserved-enum-q, to demonstrate the tighter checking. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1447836791-369-22-git-send-email-eblake@redhat.com> Message-Id: <1447883135-18020-1-git-send-email-eblake@redhat.com> [Eric's fixup squashed in] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Fix c_name() mungingEric Blake1-1/+2
The method c_name() is supposed to do two different actions: munge '-' into '_', and add a 'q_' prefix to ticklish names. But it did these steps out of order, making it possible to submit input that is not ticklish until after munging, where the output then lacked the desired prefix. The failure is exposed easily if you have a compiler that recognizes C11 keywords, and try to name a member '_Thread-local', as it would result in trying to compile the declaration 'uint64_t _Thread_local;' which is not valid. However, this name violates our conventions (ultimately, want to enforce that no qapi names start with single underscore), so the test is slightly weaker by instead testing 'wchar-t'; the declaration 'uint64_t wchar_t;' is valid in C (where wchar_t is only a typedef) but would fail with a C++ compiler (where it is a keyword). Fix things by reversing the order of actions within c_name(). Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1447836791-369-18-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Detect collisions in C member namesEric Blake1-12/+19
Detect attempts to declare two object members that would result in the same C member name, by keying the 'seen' dictionary off of the C name rather than the qapi name. It also requires passing info through the check_clash() methods. This addresses a TODO and fixes the previously-broken args-name-clash test. The resulting error message demonstrates the utility of the .describe() method added previously. No change to generated code. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1447836791-369-17-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Track owner of each object memberEric Blake1-2/+38
Future commits will migrate semantic checking away from parsing and over to the various QAPISchema*.check() methods. But to report an error message about an incorrect semantic use of a member of an object type, it helps to know which type, command, or event owns the member. In particular, when a member is inherited from a base type, it is desirable to associate the member name with the base type (and not the type calling member.check()). Rather than packing additional information into the seen array passed to each member.check() (as in seen[m.name] = {'member':m, 'owner':type}), it is easier to have each member track the name of the owner type in the first place (keeping things simpler with the existing seen[m.name] = m). The new member.owner field is set via a new set_owner() method, called when registering the members and variants arrays with an object or variant type. Track only a name, and not the actual type object, to avoid creating a circular python reference chain. Note that Variants.set_owner() method does not set the owner for the tag_member field; this field is set earlier either as part of an object's non-variant members, or explicitly by alternates. The source information is intended for human consumption in error messages, and a new describe() method is added to access the resulting information. For example, given the qapi: { 'command': 'foo', 'data': { 'string': 'str' } } an implementation of visit_command() that calls arg_type.members[0].describe() will see "'string' (parameter of foo)". To make the human-readable name of implicit types work without duplicating efforts, the describe() method has to reverse the name of implicit types, via the helper _pretty_owner(). No change to generated code. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1447836791-369-16-git-send-email-eblake@redhat.com> [Incorrect & unused -wrapper case in _pretty_owner() dropped] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Remove outdated tests related to QMP/branch collisionsEric Blake1-9/+2
Now that branches are in a separate C namespace, we can remove the restrictions in the parser that claim a branch name would collide with QMP, and delete the negative tests that are no longer problematic. A separate patch can then add positive tests to qapi-schema-test to test that any corner cases will compile correctly. This reverts the scripts/qapi.py portion of commit 7b2a5c2, now that the assertions that it plugged are no longer possible. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1447836791-369-15-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Hoist tag collision check to Variants.check()Eric Blake1-5/+2
Checking that a given QAPISchemaObjectTypeVariant.name is a member of the corresponding QAPISchemaEnumType of the owning QAPISchemaObjectTypeVariants.tag_member ensures that there are no collisions in the generated C union for those tag values (since the enum itself should have no collisions). However, ever since its introduction in f51d8c3d, this was the only additional action of of Variant.check(), beyond calling the superclass Member.check(). This forces a difference in .check() signatures, just to pass the enum type down. Simplify things by instead doing the tag name check as part of Variants.check(), at which point we can rely on inheritance instead of overriding Variant.check(). Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1447836791-369-14-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Factor out QAPISchemaObjectType.check_clash()Eric Blake1-7/+7
Consolidate two common sequences of clash detection into a new QAPISchemaObjectType.check_clash() helper method. No change to generated code. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1447836791-369-13-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Check for QAPI collisions involving variant membersEric Blake1-0/+15
Right now, our ad hoc parser ensures that we cannot have a flat union that introduces any members that would clash with non-variant members inherited from the union's base type (see flat-union-clash-member.json). We want QAPISchemaObjectType.check() to make the same check, so we can later reduce some of the ad hoc checks. We already have a map 'seen' of all non-variant members. We still need to check for collisions between each variant type's members and the non-variant ones. To know the variant type's members, we need to call variant.type.check(). This also detects when a type contains itself in a variant, exactly like the existing base.check() detects when a type contains itself as a base. (Except that we currently forbid anything but a struct as the type of a variant, so we can't actually trigger this type of loop yet.) Slight complication: an alternate's variant can have arbitrary type, but only an object type's check() may be called outside QAPISchema.check(). We could either skip the call for variants of alternates, or skip it for non-object types. For now, do the latter, because it's easier. Then we call each variant member's check_clash() with the appropriate 'seen' map. Since members of different variants can't clash, we have to clone a fresh seen for each variant. Wrap this in a new helper method QAPISchemaObjectTypeVariants.check_clash(). Note that cloning 'seen' inside .check_clash() resembles the one we just removed from .check() in 'qapi: Drop obsolete tag value collision assertions'; the difference here is that we are now checking for clashes among the qapi members of the variant type, rather than for a single clash with the variant tag name itself. Note that, by construction, collisions can't actually happen for simple unions: each variant's type is a wrapper with a single member 'data', which will never collide with the only non-variant member 'type'. For alternates, there's nothing for a variant object type's members to clash with, and therefore no need to call the new variants.check_clash(). No change to generated code. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1447836791-369-12-git-send-email-eblake@redhat.com> [Commit message tweaked] Signed-off-by: Markus Armbruster <armbru@redhat.com>