Markus Armbruster <arm...@redhat.com> writes: [...] > I had a second look. I think the generator accepting '**' in exactly > the right places relies on: > > (1) check_name() accepts only proper names, not '**'. > > (2) All names get checked with check_name(). > > (3) Except check_type() accepts special type name '**' when allow_star. > > (4) allow_star is set for exactly the right places. > > With check_name()'s superfluous / incorrect '**' check gone, (1) > obviously holds. (2) isn't obvious, thus food for code review. (3) is > trivial. (4) is trivial except for "exactly the right places". > qapi-code-gen.txt: > > In rare cases, QAPI cannot express a type-safe representation of a > corresponding Client JSON Protocol command. In these cases, if the > command expression includes the key 'gen' with boolean value false, > then the 'data' or 'returns' member that intends to bypass generated > type-safety and do its own manual validation should use '**' rather > than a valid type name. > > Unfortunately, "the 'data' or 'returns' member that intends to bypass > [...] should use '**' rather than a valid type name" can be read in (at > least) two ways: > > 1. It applies to the 'data', 'returns' members of the command object. > > 2. It applies to members of 'data', 'returns' members of the command > object. > > The schema uses both readings: qom-get has 'returns': '**', and qom-set, > netdev_add, object_add have 'data' members of the form KEY: '**'. > > Note that neither code nor tests have 'data': '**' or KEY: '**' in > 'returns'.
Type '**' generally means "any JSON value". However, a command's 'data' must be a JSON object (see docs/qmp/qmp-spec.txt). Ways to deal with this: A. Ignore. Conforming to the schema is necessary but not sufficient for a command being accepted anyway. B. The meaning of type '**' depends on context: it's "any JSON object" for a command's 'data', else "any JSON value". C. Outlaw 'data': '**' for now. I prefer C, I can accept A, I dislike B. > qapi.py appears to implement a third way: '**' may appear as type name > anywhere within 'data' or 'returns'. May well make sense, and may well > work, but we have no test coverage for it. > > We can extend tests to cover what the generator accepts (separate series > recommended), or restrict the generator to what we actually use and > test. I'm leaning towards the latter. > > Further note that '**' can only be used right in a command declaration. > Factoring out the right hand side of 'data' or 'returns' into a separate > struct type declaration isn't possible when it contains '**'. We could treat '**' as top type rather than as hack for a few commands. Conceptually clean, but hard to justify without users. [...]
