| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| ... | |
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
As of commit 8c3f8e77, we test compilation of forward references
for a struct base type (UserDefOne), flat union base type
(UserDefUnionBase), and flat union branch type
(UserDefFlatUnion2). The only remaining forward reference being
tested for parsing in flat-union-reverse-define was a forward
enum declaration. Once we make sure that always compiles,
the smaller parse-only test is redundant and can be deleted.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-7-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
| |
qapi-schema-test was already testing that we could have a
command returning int, but burned a command name in the whitelist.
Merge the redundant positive test returns-int, and pick a name
that reduces the whitelist size.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-6-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
|
| |
Rather than just asserting that we can parse an empty enum,
let's also make sure we can compile it, by including it in
qapi-schema-test.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-5-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
| |
The alternate-good.json test was already covered by
qapi-schema-test.json. As future commits will be tweaking
how alternates are laid out, removing the duplicate test now
reduces churn.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-4-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Rather than open-code the check for a valid base type, we
should reuse the common functionality. This allows for
consistent error messages, and also makes it easier for a
later patch to turn on support for inline anonymous base
structures.
Test flat-union-inline is updated to test only one feature
(anonymous branch dictionaries), which can be implemented
independently (test flat-union-bad-base already covers the
idea of an anonymous base dictionary).
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-10-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Add some testsuite exposure for use of a 'number' as part of
an alternate. The current state of the tree has a few bugs
exposed by this: our input parser depends on the ordering of
how the qapi schema declared the alternate, and the parser
does not accept integers for a 'number' in an alternate even
though it does for numbers outside of an alternate.
Mixing 'int' and 'number' in the same alternate is unusual,
since both are supplied by json-numbers, but there does not
seem to be a technical reason to forbid it given that our
json lexer distinguishes between json-numbers that can be
represented as an int vs. those that cannot.
Improve the existing test_visitor_in_alternate() to match the
style of the new test_visitor_in_alternate_number(), and to
ensure full coverage of all possible qtype parsing.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-9-git-send-email-eblake@redhat.com>
[Eric's follow-up fixes squashed in]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The documentation claims that alternates are useful for
allowing two or more types, although nothing enforces this.
Meanwhile, it is silent on whether empty unions are allowed.
In practice, the generated code will compile, in part because
we have a 'void *data' branch; but attempting to visit such a
type will cause an abort(). While there's no technical reason
that a degenerate union could not be made to work, it's harder
to justify the time spent in chasing known (the current
abort() during visit) and unknown corner cases, than it would
be to just outlaw them. A future patch will probably take the
approach of forbidding them; in the meantime, we can at least
add testsuite coverage to make it obvious where things stand.
In addition to adding tests to expose the problems, we also
need to adjust existing tests that are meant to test something
else, but which could fail for the wrong reason if we reject
degenerate alternates/unions.
Note that empty structs are explicitly supported (for example,
right now they are the only way to specify that one branch of a
flat union adds no additional members), and empty enums are
covered by the testsuite as working (even if they do not seem
to have much use).
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-8-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The previous commit added two tests that triggered an assertion
failure. It's fairly straightforward to avoid the failure by
just outright forbidding the collision between a union's tag
values and its discriminator name (including the implicit name
'kind' supplied for simple unions [*]). Ultimately, we'd like
to move the collision detection into QAPISchema*.check(), but
for now it is easier just to enhance the existing checks.
[*] Of course, down the road, we have plans to rename the simple
union tag name to 'type' to match the QMP wire name, but the
idea of the collision will still be present even then.
Technically, we could avoid the collision by naming the C union
members representing each enum value as '_case_value' rather
than 'value'; but until we have an actual qapi client (and not
just our testsuite) that has a legitimate reason to match a
case label to the name of a QMP key and needs the name munging
to satisfy the compiler, it's easier to just reject the qapi
as invalid.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-7-git-send-email-eblake@redhat.com>
[Polished a few comments]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Expose some weaknesses in the generator: we don't always forbid
the generation of structs that contain multiple members that map
to the same C or QMP name. This has already been marked FIXME in
qapi.py in commit d90675f, but having more tests will make sure
future patches produce desired behavior; and updating existing
patches to better document things doesn't hurt, either. Some of
these collisions are already caught in the old-style parser
checks, but ultimately we want all collisions to be caught in the
new-style QAPISchema*.check() methods.
This patch focuses on C struct members, and does not consider
collisions between commands and events (affecting C function
names), or even collisions between generated C type names with
user type names (for things like automatic FOOList struct
representing array types or FOOKind for an implicit enum).
There are two types of struct collisions we want to catch:
1) Collision between two keys in a JSON object. qapi.py prevents
that within a single struct (see test duplicate-key), but it is
possible to have collisions between a type's members and its
base type's members (existing tests struct-base-clash,
struct-base-clash-deep), and its flat union variant members
(renamed test flat-union-clash-member).
2) Collision between two members of the C struct that is generated
for a given QAPI type:
a) Multiple QAPI names map to the same C name (new test
args-name-clash)
b) A QAPI name maps to a C name that is used for another purpose
(new tests flat-union-clash-branch, struct-base-clash-base,
union-clash-data). We already fixed some such cases in commit
0f61af3e and 1e6c1616, but more remain.
c) Two C names generated for other purposes clash
(updated test alternate-clash, new test union-clash-branches,
union-clash-type, flat-union-clash-type)
Ultimately, if we need to have a flat union where a tag value
clashes with a base member name, we could change the generator to
name the union (using 'foo.u.value' rather than 'foo.value') or
otherwise munge the C name corresponding to tag values. But
unless such a need arises, it will probably be easier to just
forbid these collisions.
Some of these negative tests will be deleted later, and positive
tests added to qapi-schema-test.json in their place, when the
generator code is reworked to avoid particular code generation
collisions in class 2).
[Note that viewing this patch with git rename detection enabled
may see some confusion due to renaming some tests while adding
others, but where the content is similar enough that git picks
the wrong pre- and post-patch files to associate]
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-6-git-send-email-eblake@redhat.com>
[Improve commit message and comments a bit, drop an unrelated test]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
Use of '"...%s" % include' to print non-strings can lead to
ugly messages, such as this (if the .json change is applied
without the qapi.py change):
Expected a file name (string), got: OrderedDict()
Better is to just omit the actual non-string value in the
message.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-3-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
qapi/introspect.json defines the introspection schema. It's designed
for QMP introspection, but should do for similar uses, such as QGA.
The introspection schema does not reflect all the rules and
restrictions that apply to QAPI schemata. A valid QAPI schema has an
introspection value conforming to the introspection schema, but the
converse is not true.
Introspection lowers away a number of schema details, and makes
implicit things explicit:
* The built-in types are declared with their JSON type.
All integer types are mapped to 'int', because how many bits we use
internally is an implementation detail. It could be pressed into
external interface service as very approximate range information,
but that's a bad idea. If we need range information, we better do
it properly.
* Implicit type definitions are made explicit, and given
auto-generated names:
- Array types, named by appending "List" to the name of their
element type, like in generated C.
- The enumeration types implicitly defined by simple union types,
named by appending "Kind" to the name of their simple union type,
like in generated C.
- Types that don't occur in generated C. Their names start with ':'
so they don't clash with the user's names.
* All type references are by name.
* The struct and union types are generalized into an object type.
* Base types are flattened.
* Commands take a single argument and return a single result.
Dictionary argument or list result is an implicit type definition.
The empty object type is used when a command takes no arguments or
produces no results.
The argument is always of object type, but the introspection schema
doesn't reflect that.
The 'gen': false directive is omitted as implementation detail.
The 'success-response' directive is omitted as well for now, even
though it's not an implementation detail, because it's not used by
QMP.
* Events carry a single data value.
Implicit type definition and empty object type use, just like for
commands.
The value is of object type, but the introspection schema doesn't
reflect that.
* Types not used by commands or events are omitted.
Indirect use counts as use.
* Optional members have a default, which can only be null right now
Instead of a mandatory "optional" flag, we have an optional default.
No default means mandatory, default null means optional without
default value. Non-null is available for optional with default
(possible future extension).
* Clients should *not* look up types by name, because type names are
not ABI. Look up the command or event you're interested in, then
follow the references.
TODO Should we hide the type names to eliminate the temptation?
New generator scripts/qapi-introspect.py computes an introspection
value for its input, and generates a C variable holding it.
It can generate awfully long lines. Marked TODO.
A new test-qmp-input-visitor test case feeds its result for both
tests/qapi-schema/qapi-schema-test.json and qapi-schema.json to a
QmpInputVisitor to verify it actually conforms to the schema.
New QMP command query-qmp-schema takes its return value from that
variable. Its reply is some 85KiBytes for me right now.
If this turns out to be too much, we have a couple of options:
* We can use shorter names in the JSON. Not the QMP style.
* Optionally return the sub-schema for commands and events given as
arguments.
Right now qmp_query_schema() sends the string literal computed by
qmp-introspect.py. To compute sub-schema at run time, we'd have to
duplicate parts of qapi-introspect.py in C. Unattractive.
* Let clients cache the output of query-qmp-schema.
It changes only on QEMU upgrades, i.e. rarely. Provide a command
query-qmp-schema-hash. Clients can have a cache indexed by hash,
and re-query the schema only when they don't have it cached. Even
simpler: put the hash in the QMP greeting.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
|
| |
|
|
|
|
|
|
|
|
| |
'gen': false needs to stay for now, because netdev_add is still using
it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1442401589-24189-25-git-send-email-armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
| |
It's first class, because unlike '**', it actually works, i.e. doesn't
require 'gen': false.
'**' will go away next.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Fixes events whose data is struct with base to include the struct's
base members. Test case is qapi-schema-test.json's event
__org.qemu_x-command:
{ 'event': '__ORG.QEMU_X-EVENT', 'data': '__org.qemu_x-Struct' }
{ 'struct': '__org.qemu_x-Struct', 'base': '__org.qemu_x-Base',
'data': { '__org.qemu_x-member2': 'str' } }
{ 'struct': '__org.qemu_x-Base',
'data': { '__org.qemu_x-member1': '__org.qemu_x-Enum' } }
Patch's effect on generated qapi_event_send___org_qemu_x_event():
-void qapi_event_send___org_qemu_x_event(const char *__org_qemu_x_member2,
+void qapi_event_send___org_qemu_x_event(__org_qemu_x_Enum __org_qemu_x_member1,
+ const char *__org_qemu_x_member2,
Error **errp)
{
QDict *qmp;
@@ -224,6 +225,10 @@ void qapi_event_send___org_qemu_x_event(
goto clean;
}
+ visit_type___org_qemu_x_Enum(v, &__org_qemu_x_member1, "__org.qemu_x-member1", &local_err);
+ if (local_err) {
+ goto clean;
+ }
visit_type_str(v, (char **)&__org_qemu_x_member2, "__org.qemu_x-member2", &local_err);
if (local_err) {
goto clean;
Code is generated in a different order now, but that doesn't matter.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Fixes flat unions to visit the base's base members (the previous
commit merely added them to the struct). Same test case.
Patch's effect on visit_type_UserDefFlatUnion():
static void visit_type_UserDefFlatUnion_fields(Visitor *m, UserDefFlatUnion **obj, Error **errp)
{
Error *err = NULL;
+ visit_type_int(m, &(*obj)->integer, "integer", &err);
+ if (err) {
+ goto out;
+ }
visit_type_str(m, &(*obj)->string, "string", &err);
if (err) {
goto out;
Test cases updated for the bug fix.
Fixes alternates to generate a visitor for their implicit enumeration
type. None of them are currently used, obviously. Example:
block-core.json's BlockdevRef now generates
visit_type_BlockdevRefKind().
Code is generated in a different order now, and therefore has got a
few new forward declarations. Doesn't matter.
The guard QAPI_VISIT_BUILTIN_VISITOR_DECL is renamed to
QAPI_VISIT_BUILTIN.
The previous commit's two ugly special cases exist here, too. Mark
both TODO.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Fixes flat unions to get the base's base members. Test case is from
commit 2fc0043, in qapi-schema-test.json:
{ 'union': 'UserDefFlatUnion',
'base': 'UserDefUnionBase',
'discriminator': 'enum1',
'data': { 'value1' : 'UserDefA',
'value2' : 'UserDefB',
'value3' : 'UserDefB' } }
{ 'struct': 'UserDefUnionBase',
'base': 'UserDefZero',
'data': { 'string': 'str', 'enum1': 'EnumOne' } }
{ 'struct': 'UserDefZero',
'data': { 'integer': 'int' } }
Patch's effect on UserDefFlatUnion:
struct UserDefFlatUnion {
/* Members inherited from UserDefUnionBase: */
+ int64_t integer;
char *string;
EnumOne enum1;
/* Own members: */
union { /* union tag is @enum1 */
void *data;
UserDefA *value1;
UserDefB *value2;
UserDefB *value3;
};
};
Flat union visitors remain broken. They'll be fixed next.
Code is generated in a different order now, but that doesn't matter.
The two guards QAPI_TYPES_BUILTIN_STRUCT_DECL and
QAPI_TYPES_BUILTIN_CLEANUP_DECL are replaced by just
QAPI_TYPES_BUILTIN.
Two ugly special cases for simple unions now stand out like sore
thumbs:
1. The type tag is named 'type' everywhere, except in generated C,
where it's 'kind'.
2. QAPISchema lowers simple unions to semantically equivalent flat
unions. However, the C generated for a simple unions differs from
the C generated for its equivalent flat union, and we therefore
need special code to preserve that pointless difference for now.
Mark both TODO.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
The old code prints the result of parsing (list of expression
dictionaries), and partial results of semantic analysis (list of enum
dictionaries, list of struct dictionaries).
The new code prints a trace of a schema visit, i.e. what the back-ends
are going to use. Built-in and array types are omitted, because
they're boring.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The QAPI code generators work with a syntax tree (nested dictionaries)
plus a few symbol tables (also dictionaries) on the side.
They have clearly outgrown these simple data structures. There's lots
of rummaging around in dictionaries, and information is recomputed on
the fly. For the work I'm going to do, I want more clearly defined
and more convenient interfaces.
Going forward, I also want less coupling between the back-ends and the
syntax tree, to make messing with the syntax easier.
Create a bunch of classes to represent QAPI schemata.
Have the QAPISchema initializer call the parser, then walk the syntax
tree to create the new internal representation, and finally perform
semantic analysis.
Shortcut: the semantic analysis still relies on existing check_exprs()
to do the actual semantic checking. All this code needs to move into
the classes. Mark as TODO.
Simple unions are lowered to flat unions. Flat unions and structs are
represented as a more general object type.
Catching name collisions in generated code would be nice. Mark as
TODO.
We generate array types eagerly, even though most of them aren't used.
Mark as TODO.
Nothing uses the new intermediate representation just yet, thus no
change to generated files.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The camel_to_upper() method applies some heuristics to turn
a mixed case type name into an all-uppercase name. This is
used for example, to generate enum constant name prefixes.
The heuristics don't also generate a satisfactory name
though. eg
{ 'enum': 'QCryptoTLSCredsEndpoint',
'data': ['client', 'server']}
Results in Q_CRYPTOTLS_CREDS_ENDPOINT_CLIENT. This has
an undesirable _ after the initial Q and is missing an
_ between the CRYPTO & TLS strings.
Rather than try to add more and more heuristics to try
to cope with this, simply allow the QAPI schema to
specify the desired enum constant prefix explicitly.
eg
{ 'enum': 'QCryptoTLSCredsEndpoint',
'prefix': 'QCRYPTO_TLS_CREDS_ENDPOINT',
'data': ['client', 'server']}
Now gives the QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT name.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
check_type() first checks and peels off the array type, then checks
the element type. For two out of four error messages, it takes pains
to report errors for "array of T" instead of just T. Odd. Let's
examine the errors.
* Unknown element type, e.g.
tests/qapi-schema/args-array-unknown.json:
Member 'array' of 'data' for command 'oops' uses unknown type
'array of NoSuchType'
To make sense of this, you need to know that 'array of NoSuchType'
refers to '[NoSuchType]'. Easy enough. However, simply reporting
Member 'array' of 'data' for command 'oops' uses unknown type
'NoSuchType'
is at least as easy to understand.
* Element type's meta-type is inadmissible, e.g.
tests/qapi-schema/returns-whitelist.json:
'returns' for command 'no-way-this-will-get-whitelisted' cannot
use built-in type 'array of int'
'array of int' is technically not a built-in type, but that's
pedantry. However, simply reporting
'returns' for command 'no-way-this-will-get-whitelisted' cannot
use built-in type 'int'
avoids the issue, and is at least as easy to understand.
* The remaining two errors are unreachable, because the array checking
ensures that value is a string.
Thus, reporting some errors for "array of T" instead of just T works,
but doesn't really improve things. Drop it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
|
| |
|
|
|
|
|
| |
Fixes the errors demonstrated by the previous commit.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
|
| |
|
|
|
|
|
|
|
| |
We always report "should be a dictionary" then. This is misleading:
when allow_dict, it can be a dictionary or a type name string, else it
can only be a type name.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
|
| |
|
|
|
|
|
| |
Syntax error coverage should now be complete.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
|
| |
|
|
|
|
|
|
|
| |
The first check ensures the second one can't trigger. Drop the first
one, because the second one is in a more logical place, and emits a
nicer error message.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Reproducer: with
{ 'command': 'user_def_cmd4', 'returns': { 'a': 'int' } }
added to qapi-schema-test.json, qapi-commands.py dies when it tries to
generate the command handler function
Traceback (most recent call last):
File "/work/armbru/qemu/scripts/qapi-commands.py", line 359, in <module>
ret = generate_command_decl(cmd['command'], arglist, ret_type) + "\n"
File "/work/armbru/qemu/scripts/qapi-commands.py", line 29, in generate_command_decl
ret_type=c_type(ret_type), name=c_name(name),
File "/work/armbru/qemu/scripts/qapi.py", line 927, in c_type
assert isinstance(value, str) and value != ""
AssertionError
because the return type doesn't exist.
Simply outlaw this usage, and drop or dumb down test cases accordingly.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
A command's or event's 'data' must be a struct type, given either as a
dictionary, or as struct type name.
Commit dd883c6 tightened the checking there, but not enough: we still
accept 'union'. Fix to reject it.
We may want to support union types there, but we'll have to extend
qapi-commands.py and qapi-events.py for it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
A command's 'data' must be a struct type, given either as a
dictionary, or as struct type name.
Existing test case data-int.json covers simple type 'int'. Add test
cases for type names referring to union and alternate types.
The latter is caught (good), but the former is not (bug).
Events have the same problem, but since they get checked by the same
code, we don't bother to duplicate the tests.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
|
| |
|
|
|
|
|
|
|
| |
Since every schema entity has 'data', the data- prefix conveys no
information. These tests actually exercise commands. Only commands
have arguments, so change the prefix to to args-.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
|
| |
|
|
|
|
|
| |
Test case added in commit 2fc0043, and messed up in commit 5223070.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
| |
When event FOO's 'data' is a struct with a base, we consider only the
struct's direct members, and ignore its base. The generated
qapi_event_send_foo() doesn't take arguments for base members.
No such events currently exist in the QMP schema.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
| |
We generate a declaration, but no definition.
The QMP schema has two: Qcow2OverlapChecks and BlockdevRef. Neither
visit_type_Qcow2OverlapChecksKind() nor visit_type_BlockdevRefKind()
is actually used.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The visit_type_implicit_FOO() are generated on demand, right before
their first use. Used by visit_type_STRUCT_fields() when STRUCT has
base FOO, and by visit_type_UNION() when flat UNION has member a FOO.
If the schema defines FOO after its first use as struct base or flat
union member, visit_type_implicit_FOO() calls
visit_type_implicit_FOO() before its definition, which doesn't
compile.
Rearrange qapi-schema-test.json to demonstrate the bug.
Fix by generating the necessary forward declaration.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
|
| |
|
|
|
| |
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
|
| |
|
|
|
|
|
| |
The new test demonstrates another generator crash.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
|
| |
|
|
|
|
|
|
|
| |
We print the name as it appears in the include expression. Tools
processing error messages want it relative to the working directory.
Make it so.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
| |
Enhance the testsuite to cover downstream events and commands.
Events worked without more tweaks, but commands needed a few final
updates in the generator to mangle names in the appropriate places.
In making those tweaks, it was easier to drop type_visitor() and
inline its actions instead.
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
| |
Enhance the testsuite to cover downstream alternates, including
whether the branch name or type is downstream. Update the
generator to mangle alternate names in the appropriate places.
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
|
| |
Enhance the testsuite to cover downstream flat unions, including
the base type, discriminator name and type, and branch name and
type. Update the generator to mangle the union names in the
appropriate places.
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
| |
Enhance the testsuite to cover downstream simple unions, including
when a union branch is a downstream name. Update the generator to
mangle the union names in the appropriate places.
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
| |
Enhance the testsuite to cover downstream structs, including struct
members and base structs. Update the generator to mangle the
struct names in the appropriate places.
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
| |
Enhance the testsuite to cover a downstream enum type and enum
string. Update the generator to mangle the enum name in the
appropriate places.
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Our type inheritance for both 'struct' and for flat 'union' merges
key/value pairs from the base class with those from the type in
question. Although the C code currently boxes things so that there
is a distinction between which member is referred to, the QMP wire
format does not allow passing a key more than once in a single
object. Besides, if we ever change the generated C code to not be
quite so boxy, we'd want to avoid duplicate member names there,
too.
Fix a testsuite entry added in an earlier patch, as well as adding
a couple more tests to ensure we have appropriate coverage. Ensure
that collisions are detected, regardless of whether there is a
difference in opinion on whether the member name is optional.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The handling of \ inside QAPI strings was less than ideal, and
really only worked JSON's \/, \\, \", and our extension of \'
(an obvious extension, when you realize we use '' instead of ""
for strings). For other things, like '\n', it resulted in a
literal 'n' instead of a newline.
Of course, at the moment, we really have no use for escaped
characters, as QAPI has to map to C identifiers, and we currently
support ASCII only for that. But down the road, we may add
support for default values for string parameters to a command
or struct; if that happens, it would be nice to correctly support
all JSON escape sequences, such as \n or \uXXXX. This gets us
closer, by supporting Unicode escapes in the ASCII range.
Since JSON does not require \OCTAL or \xXX escapes, and our QMP
implementation does not understand them either, I intentionally
reject it here, but it would be an easy addition if we desired it.
Likewise, intentionally refusing the NUL byte means we don't have
to worry about C strings being shorter than the qapi input.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
A future patch will be using a 'name':{dictionary} entry in the
QAPI schema to specify a default value for an optional argument
(see previous commit messages for more details why); but existing
use of inline nested structs conflicts with that goal. Now that
all commands have been changed to avoid inline nested structs,
nuke support for them, and turn it into a hard error. Update the
testsuite to reflect tighter parsing rules.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
A future patch will be using a 'name':{dictionary} entry in the
QAPI schema to specify a default value for an optional argument;
but existing use of inline nested structs conflicts with that goal.
More precisely, a definition in the QAPI schema associates a name
with a set of properties:
Example 1: { 'struct': 'Foo', 'data': { MEMBERS... } }
associates the global name 'Foo' with properties (meta-type struct)
and MEMBERS...
Example 2: 'mumble': TYPE
within MEMBERS... above associates 'mumble' with properties (type
TYPE) and (optional false) within type Foo
The syntax of example 1 is extensible; if we need another property,
we add another name/value pair to the dictionary (such as
'base':TYPE). The syntax of example 2 is not extensible, because
the right hand side can only be a type.
We have used name encoding to add a property: "'*mumble': 'int'"
associates 'mumble' with (type int) and (optional true). Nice,
but doesn't scale. So the solution is to change our existing uses
to be syntactic sugar to an extensible form:
NAME: TYPE --> NAME: { 'type': TYPE, 'optional': false }
*ONAME: TYPE --> ONAME: { 'type': TYPE, 'optional': true }
This patch fixes the testsuite to avoid inline nested types, by
breaking the nesting into explicit types; it means that the type
is now boxed instead of unboxed in C code, but makes no difference
on the wire (and if desired, a later patch could change the
generator to not do so much boxing in C). When touching code to
add new allocations, also convert existing allocations to
consistently prefer typesafe g_new0 over g_malloc0 when a type
name is involved.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In the testsuite, UserDefTwo and UserDefNested were identical
structs other than the member names. Reduce code duplication by
having just one type, and choose names that also favor reuse.
This will also make it easier for a later patch to get rid of
inline nested types in QAPI. When touching code related to
allocations, convert g_malloc0(sizeof(Type)) to the more typesafe
g_new0(Type, 1).
Ensure that 'make check-qapi-schema check-unit' still passes.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Referring to "type" as both a meta-type (built-in, enum, union,
alternate, or struct) and a specific type (the name that the
schema uses for declaring structs) is confusing. Do the bulk of
the conversion to "struct" in qapi schema, with a fairly
mechanical:
for f in `find -name '*.json'; do sed -i "s/'type'/'struct'/"; done
followed by manually filtering out the places where we have a
'type' embedded in 'data'. Then tweak a couple of tests whose
output changes slightly due to longer lines.
I also verified that the generated files for QMP and QGA (such
as qmp-commands.h) are the same before and after, as assurance
that I didn't leave in any accidental member name changes.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Referring to "type" as both a meta-type (built-in, enum, union,
alternate, or struct) and a specific type (the name that the
schema uses for declaring structs) is confusing. The confusion
is only made worse by the fact that the generator mostly already
refers to struct even when dealing with expr['type']. This
commit changes the generator to consistently refer to it as
struct everywhere, plus a single back-compat tweak that allows
accepting the existing .json files as-is, so that the meat of
this change is separate from the mindless churn of that change.
Fix the testsuite fallout for error messages that change, and
in some cases, become more legible. Improve comments to better
match our intentions where a struct (rather than any complex
type) is required. Note that in some cases, an error message
now refers to 'struct' while the schema still refers to 'type';
that will be cleaned up in the later commit to the schema.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Now that we have a way to validate every type, we can also be
stricter about enforcing that callers that want to bypass
type safety in generated code. Prior to this patch, it didn't
matter what value was associated with the key 'gen', but it
looked odd that 'gen':'yes' could result in bypassing the
generated code. These changes also enforce the changes made
earlier in the series for documentation and consolidation of
using '**' as the wildcard type, as well as 'gen':false as the
canonical spelling for requesting type bypass.
Note that 'gen':false is a one-way switch away from the default;
we do not support 'gen':true (similar for 'success-response').
In practice, this doesn't matter.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
...or an array of dictionaries. Although we have to cater to
existing commands, returning a non-dictionary means the command
is not extensible (no new name/value pairs can be added if more
information must be returned in parallel). By making the
whitelist explicit, any new command that falls foul of this
practice will have to be self-documenting, which will encourage
developers to either justify the action or rework the design to
use a dictionary after all.
It's a little bit sloppy that we share a single whitelist among
three clients (it's too permissive for each). If this is a
problem, a future patch could tighten things by having the
generator take the whitelist as an argument (as in
scripts/qapi-commands.py --legacy-returns=...), or by having
the generator output C code that requires explicit use of the
whitelist (as in:
#ifndef FROBNICATE_LEGACY_RETURN_OK
# error Command 'frobnicate' should return a dictionary
#endif
then having the callers define appropriate macros). But until
we need such fine-grained separation (if ever), this patch does
the job just fine.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
