scripts: qapi-event.py: support vendor extension
diff mbox

Message ID 20140708141728.412a3f1c@redhat.com
State New
Headers show

Commit Message

Luiz Capitulino July 8, 2014, 6:17 p.m. UTC
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.

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 <lcapitulino@redhat.com>
---
 scripts/qapi-event.py | 8 ++++----
 scripts/qapi.py       | 4 ++++
 2 files changed, 8 insertions(+), 4 deletions(-)

Comments

Markus Armbruster July 10, 2014, 2:31 p.m. UTC | #1
Luiz Capitulino <lcapitulino@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 <lcapitulino@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?
Luiz Capitulino July 10, 2014, 2:36 p.m. UTC | #2
On Thu, 10 Jul 2014 16:31:38 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@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 <lcapitulino@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.
Eric Blake July 10, 2014, 4:13 p.m. UTC | #3
On 07/10/2014 08:31 AM, Markus Armbruster wrote:
> Luiz Capitulino <lcapitulino@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 <lcapitulino@redhat.com>
>> ---

>> +# 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?

If the generator spits out any comments, those comments should refer to
the QMP wire name, including the '.'. But right now, generated code
doesn't seem to do that.
Wenchao Xia July 23, 2014, 2:19 p.m. UTC | #4
Reviewed-by: Wenchao Xia <wenchaoqemu@gmail.com>

I didn't expect dot in schema before.

Patch
diff mbox

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