Re: [Qemu-devel] [PATCH] scripts: qapi-event.py: support vendor extension

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2114 bytes --]

On 07/11/2014 08:42 AM, Markus Armbruster wrote:

>>> Can anybody think of a use of c_var() that needs '.' preserved?
>>
>> Doing the replace in c_var() breaks some struct accesses in the generated
>> code. I didn't look deeper to determine the users though.
> 
> Feels like a misuse of c_var() to me.
> 
> Dig, dig...  aha.  generate_visit_struct_fields() joins QAPI names
> separated by '.', and passes the result to c_var().  I expect such code
> to break when one of the names contains '.'.
> 
> It does indeed; try the appended patch to see it yourself.  It generates
> 
> struct VersionInfo
> {
>     struct 
>     {
>         int64_t major;
>         int64_t minor;
>         int64_t micro;
>     } qemu;

Wait a minute.  Isn't this one of the three cases of nested structs,
where we were already arguing that nested structs are evil if we are
going to introduce a fuller syntax for optional argument defaults?

>     struct 
>     {
>         int64_t major;
>         int64_t minor;
>         int64_t micro;
>     } __com.redhat.crap;
>     char *package;
> };
> 
> Conclusion: this is simply a bug that needs fixing.
> 
> 
> diff --git a/qapi/common.json b/qapi/common.json
> index 4e9a21f..74ccde3 100644
> --- a/qapi/common.json
> +++ b/qapi/common.json
> @@ -52,6 +52,7 @@
>  ##
>  { 'type': 'VersionInfo',
>    'data': {'qemu': {'major': 'int', 'minor': 'int', 'micro': 'int'},
> +           '__com.redhat.crap': {'major': 'int', 'minor': 'int', 'micro': 'int'},
>             'package': 'str'} }

And the fix may be as simple as ditching support for nested structs in
the first place, and rewriting this as:

{ 'type': 'VersionDetails',
  'data': { major': 'int', 'minor': 'int', 'micro': 'int'} }
{ 'type': 'VersionInfo',
  'data': {'qemu': 'VersionDetails',
           '__com.redhat.crap': 'VersionDetails',
           'package': 'str' } }

But the fact that we are still discussing makes it obvious - this is 2.2
material.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]