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
signature.asc
Description: OpenPGP digital signature
