Luiz Capitulino <lcapitul...@redhat.com> writes:
> On Thu, 10 Jul 2014 16:31:38 +0200
> Markus Armbruster <arm...@redhat.com> wrote:
>
>> Luiz Capitulino <lcapitul...@redhat.com> writes:
>>
>> > The event code generator barfs when it sees a dot in an event
>> > argument, this makes it impossible to support vendor extensions
>> > in event arguments as they always contain dots. Fix this by
>> > replacing dots by hyphens in the generated code.
>>
>> Code replaces by underbar, not hyphen.
>>
>> > PS: Event names and QMP command arguments may suffer from the
>> > same issue, but I'm not checking/fixing them today.
>> >
>> > Signed-off-by: Luiz Capitulino <lcapitul...@redhat.com>
>> > ---
>> > scripts/qapi-event.py | 8 ++++----
>> > scripts/qapi.py | 4 ++++
>> > 2 files changed, 8 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
>> > index 601e307..485694b 100644
>> > --- a/scripts/qapi-event.py
>> > +++ b/scripts/qapi-event.py
>> > @@ -23,11 +23,11 @@ def _generate_event_api_name(event_name, params):
>> > if params:
>> > for argname, argentry, optional, structured in parse_args(params):
>> > if optional:
>> > - api_name += "bool has_%s,\n" % c_var(argname)
>> > + api_name += "bool has_%s,\n" % c_arg(argname)
>> > api_name += "".ljust(l)
>> >
>> > api_name += "%s %s,\n" % (c_type(argentry, is_param=True),
>> > - c_var(argname))
>> > + c_arg(argname))
>> > api_name += "".ljust(l)
>> >
>> > api_name += "Error **errp)"
>> > @@ -98,7 +98,7 @@ def generate_event_implement(api_name, event_name,
>> > params):
>> > ret += mcgen("""
>> > if (has_%(var)s) {
>> > """,
>> > - var = c_var(argname))
>> > + var = c_arg(argname))
>> > push_indent()
>> >
>> > if argentry == "str":
>> > @@ -113,7 +113,7 @@ def generate_event_implement(api_name, event_name,
>> > params):
>> > }
>> > """,
>> > var_type = var_type,
>> > - var = c_var(argname),
>> > + var = c_arg(argname),
>> > type = type_name(argentry),
>> > name = argname)
>> >
>> > diff --git a/scripts/qapi.py b/scripts/qapi.py
>> > index f2c6d1f..ddab14d 100644
>> > --- a/scripts/qapi.py
>> > +++ b/scripts/qapi.py
>> > @@ -434,6 +434,10 @@ def c_var(name, protect=True):
>> > def c_fun(name, protect=True):
>> > return c_var(name, protect).replace('.', '_')
>> >
>> > +# Should be used where vendor extensions are supported
>> > +def c_arg(name):
>> > + return c_var(name).replace('.', '_')
>> > +
>> > def c_list_type(name):
>> > return '%sList' % name
>>
>> 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;
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'} }
##
