[Qemu-devel] [PATCH v5 0/3] qapi: fix coding style in generated code

[Qemu-devel] [PATCH v5 0/3] qapi: fix coding style in generated code

[Amos Kong]

Not a serious issue, but it's helpful if we can fix it.

V2: split change of scripts/qapi-visit.py to a split patch,
    eat space by using a special char as Markus suggested
V3: update commitlog, update special string, fix of adding
    const replace string by pattern
V4: fix pattern to cleanup special string (Paolo)
V5: fix string checking bug (Luiz), update commitlog (Eric)
    add comments for c_type() function

Amos Kong (3):
  qapi: fix coding style in parameters list
  qapi: add const prefix to 'char *' insider c_type()
  qapi: Suppress unwanted space between type and identifier

 scripts/qapi-commands.py |  6 ++----
 scripts/qapi-visit.py    | 20 ++++++++++----------
 scripts/qapi.py          | 21 +++++++++++++++------
 3 files changed, 27 insertions(+), 20 deletions(-)

-- 
1.9.0

[Qemu-devel] [PATCH v5 1/3] qapi: fix coding style in parameters list

[Amos Kong]

A space after * when declaring a pointer type is redundant.

Signed-off-by: Amos Kong <akong@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi-visit.py | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 06a79f1..c129697 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -77,7 +77,7 @@ static void visit_type_%(full_name)s_field_%(c_name)s(Visitor *m, %(name)s **obj
 
     ret += mcgen('''
 
-static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s ** obj, Error **errp)
+static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s **obj, Error **errp)
 {
     Error *err = NULL;
 ''',
@@ -186,7 +186,7 @@ def generate_visit_struct(expr):
 
     ret += mcgen('''
 
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp)
+void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **errp)
 {
 ''',
                 name=name)
@@ -201,7 +201,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
 def generate_visit_list(name, members):
     return mcgen('''
 
-void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name, Error **errp)
+void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, Error **errp)
 {
     Error *err = NULL;
     GenericList *i, **prev;
@@ -230,7 +230,7 @@ out:
 def generate_visit_enum(name, members):
     return mcgen('''
 
-void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error **errp)
+void visit_type_%(name)s(Visitor *m, %(name)s *obj, const char *name, Error **errp)
 {
     visit_type_enum(m, (int *)obj, %(name)s_lookup, "%(name)s", name, errp);
 }
@@ -240,7 +240,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error **e
 def generate_visit_anon_union(name, members):
     ret = mcgen('''
 
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp)
+void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **errp)
 {
     Error *err = NULL;
 
@@ -327,7 +327,7 @@ def generate_visit_union(expr):
 
     ret += mcgen('''
 
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp)
+void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **errp)
 {
     Error *err = NULL;
 
@@ -399,13 +399,13 @@ def generate_declaration(name, members, genlist=True, builtin_type=False):
     if not builtin_type:
         ret += mcgen('''
 
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp);
+void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **errp);
 ''',
                     name=name)
 
     if genlist:
         ret += mcgen('''
-void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name, Error **errp);
+void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, Error **errp);
 ''',
                  name=name)
 
@@ -415,7 +415,7 @@ def generate_enum_declaration(name, members, genlist=True):
     ret = ""
     if genlist:
         ret += mcgen('''
-void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name, Error **errp);
+void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, Error **errp);
 ''',
                      name=name)
 
@@ -424,7 +424,7 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name,
 def generate_decl_enum(name, members, genlist=True):
     return mcgen('''
 
-void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error **errp);
+void visit_type_%(name)s(Visitor *m, %(name)s *obj, const char *name, Error **errp);
 ''',
                 name=name)
 
-- 
1.9.0

[Qemu-devel] [PATCH v5 2/3] qapi: add const prefix to 'char *' insider c_type()

[Amos Kong]

It's ugly to add const prefix for parameter type by an if statement
outside c_type(). This patch adds a parameter to do it.

Signed-off-by: Amos Kong <akong@redhat.com>
Suggested-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi-commands.py | 4 +---
 scripts/qapi.py          | 4 +++-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 7d93d01..34f200a 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -29,9 +29,7 @@ def type_visitor(name):
 def generate_command_decl(name, args, ret_type):
     arglist=""
     for argname, argtype, optional, structured in parse_args(args):
-        argtype = c_type(argtype)
-        if argtype == "char *":
-            argtype = "const char *"
+        argtype = c_type(argtype, is_param=True)
         if optional:
             arglist += "bool has_%s, " % c_var(argname)
         arglist += "%s %s, " % (argtype, c_var(argname))
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 86e9608..dc690bb 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -470,8 +470,10 @@ def find_enum(name):
 def is_enum(name):
     return find_enum(name) != None
 
-def c_type(name):
+def c_type(name, is_param=False):
     if name == 'str':
+        if is_param:
+            return 'const char *'
         return 'char *'
     elif name == 'int':
         return 'int64_t'
-- 
1.9.0

[Qemu-devel] [PATCH v5 3/3] qapi: Suppress unwanted space between type and identifier

[Amos Kong]

We always generate a space between type and identifier in parameter
and variable declarations, even when idiomatic C style doesn't have
a space there.  Suppress it.

Signed-off-by: Amos Kong <akong@redhat.com>
---
 scripts/qapi-commands.py |  2 +-
 scripts/qapi.py          | 19 +++++++++++++------
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 34f200a..4b49735 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -102,7 +102,7 @@ def gen_visitor_input_vars_decl(args):
 bool has_%(argname)s = false;
 ''',
                          argname=c_var(argname))
-        if c_type(argtype).endswith("*"):
+        if c_type(argtype).endswith("*" + eatspace):
             ret += mcgen('''
 %(argtype)s %(argname)s = NULL;
 ''',
diff --git a/scripts/qapi.py b/scripts/qapi.py
index dc690bb..562d560 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -470,11 +470,17 @@ def find_enum(name):
 def is_enum(name):
     return find_enum(name) != None
 
+eatspace = '\033EATSPACE.'
+
+# A special suffix is added in c_type() for pointer types, and it's
+# stripped in mcgen(). So please notice this when you check the return
+# value of c_type() outside mcgen().
 def c_type(name, is_param=False):
     if name == 'str':
         if is_param:
-            return 'const char *'
-        return 'char *'
+            return 'const char *' + eatspace
+        return 'char *' + eatspace
+
     elif name == 'int':
         return 'int64_t'
     elif (name == 'int8' or name == 'int16' or name == 'int32' or
@@ -488,15 +494,15 @@ def c_type(name, is_param=False):
     elif name == 'number':
         return 'double'
     elif type(name) == list:
-        return '%s *' % c_list_type(name[0])
+        return '%s *%s' % (c_list_type(name[0]), eatspace)
     elif is_enum(name):
         return name
     elif name == None or len(name) == 0:
         return 'void'
     elif name == name.upper():
-        return '%sEvent *' % camel_case(name)
+        return '%sEvent *%s' % (camel_case(name), eatspace)
     else:
-        return '%s *' % name
+        return '%s *%s' % (name, eatspace)
 
 def genindent(count):
     ret = ""
@@ -521,7 +527,8 @@ def cgen(code, **kwds):
     return '\n'.join(lines) % kwds + '\n'
 
 def mcgen(code, **kwds):
-    return cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
+    raw = cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
+    return re.sub(re.escape(eatspace) + ' *', '', raw)
 
 def basename(filename):
     return filename.split("/")[-1]
-- 
1.9.0

Re: [Qemu-devel] [PATCH v5 3/3] qapi: Suppress unwanted space between type and identifier

[Eric Blake]

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

On 05/22/2014 06:41 AM, Amos Kong wrote:
> We always generate a space between type and identifier in parameter
> and variable declarations, even when idiomatic C style doesn't have
> a space there.  Suppress it.
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
>  scripts/qapi-commands.py |  2 +-
>  scripts/qapi.py          | 19 +++++++++++++------
>  2 files changed, 14 insertions(+), 7 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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 --]

Re: [Qemu-devel] [PATCH v5 3/3] qapi: Suppress unwanted space between type and identifier

[Markus Armbruster]

Amos Kong <akong@redhat.com> writes:

> We always generate a space between type and identifier in parameter
> and variable declarations, even when idiomatic C style doesn't have
> a space there.  Suppress it.
>
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
>  scripts/qapi-commands.py |  2 +-
>  scripts/qapi.py          | 19 +++++++++++++------
>  2 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> index 34f200a..4b49735 100644
> --- a/scripts/qapi-commands.py
> +++ b/scripts/qapi-commands.py
> @@ -102,7 +102,7 @@ def gen_visitor_input_vars_decl(args):
>  bool has_%(argname)s = false;
>  ''',
>                           argname=c_var(argname))
> -        if c_type(argtype).endswith("*"):
> +        if c_type(argtype).endswith("*" + eatspace):
>              ret += mcgen('''
>  %(argtype)s %(argname)s = NULL;
>  ''',

Ugly.  A function is_c_ptr(argtype) would be cleaner.

There's similar code in gen_marshal_input():

    if ret_type:
        if c_type(ret_type).endswith("*"):
            retval = "    %s retval = NULL;" % c_type(ret_type)
        else:
            retval = "    %s retval;" % c_type(ret_type)
        ret += mcgen('''
%(retval)s
''',
                     retval=retval)

Don't you have to patch that, too?

> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index dc690bb..562d560 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -470,11 +470,17 @@ def find_enum(name):
>  def is_enum(name):
>      return find_enum(name) != None
>  
> +eatspace = '\033EATSPACE.'
> +
> +# A special suffix is added in c_type() for pointer types, and it's
> +# stripped in mcgen(). So please notice this when you check the return
> +# value of c_type() outside mcgen().
>  def c_type(name, is_param=False):
>      if name == 'str':
>          if is_param:
> -            return 'const char *'
> -        return 'char *'
> +            return 'const char *' + eatspace
> +        return 'char *' + eatspace
> +
>      elif name == 'int':
>          return 'int64_t'
>      elif (name == 'int8' or name == 'int16' or name == 'int32' or
> @@ -488,15 +494,15 @@ def c_type(name, is_param=False):
>      elif name == 'number':
>          return 'double'
>      elif type(name) == list:
> -        return '%s *' % c_list_type(name[0])
> +        return '%s *%s' % (c_list_type(name[0]), eatspace)
>      elif is_enum(name):
>          return name
>      elif name == None or len(name) == 0:
>          return 'void'
>      elif name == name.upper():
> -        return '%sEvent *' % camel_case(name)
> +        return '%sEvent *%s' % (camel_case(name), eatspace)
>      else:
> -        return '%s *' % name
> +        return '%s *%s' % (name, eatspace)
>  
>  def genindent(count):
>      ret = ""
> @@ -521,7 +527,8 @@ def cgen(code, **kwds):
>      return '\n'.join(lines) % kwds + '\n'
>  
>  def mcgen(code, **kwds):
> -    return cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
> +    raw = cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
> +    return re.sub(re.escape(eatspace) + ' *', '', raw)
>  
>  def basename(filename):
>      return filename.split("/")[-1]

Rest looks good.

Re: [Qemu-devel] [PATCH v5 1/3] qapi: fix coding style in parameters list

[Markus Armbruster]

Amos Kong <akong@redhat.com> writes:

> A space after * when declaring a pointer type is redundant.

That's not why we clean it up.  We clean it up because it's not our
style.  PATCH 3/3's commit message explains it nicely.

> Signed-off-by: Amos Kong <akong@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

My R-by stands, of course.

Re: [Qemu-devel] [PATCH v5 3/3] qapi: Suppress unwanted space between type and identifier

[Luiz Capitulino]

On Mon, 02 Jun 2014 17:43:58 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Amos Kong <akong@redhat.com> writes:
> 
> > We always generate a space between type and identifier in parameter
> > and variable declarations, even when idiomatic C style doesn't have
> > a space there.  Suppress it.
> >
> > Signed-off-by: Amos Kong <akong@redhat.com>
> > ---
> >  scripts/qapi-commands.py |  2 +-
> >  scripts/qapi.py          | 19 +++++++++++++------
> >  2 files changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> > index 34f200a..4b49735 100644
> > --- a/scripts/qapi-commands.py
> > +++ b/scripts/qapi-commands.py
> > @@ -102,7 +102,7 @@ def gen_visitor_input_vars_decl(args):
> >  bool has_%(argname)s = false;
> >  ''',
> >                           argname=c_var(argname))
> > -        if c_type(argtype).endswith("*"):
> > +        if c_type(argtype).endswith("*" + eatspace):
> >              ret += mcgen('''
> >  %(argtype)s %(argname)s = NULL;
> >  ''',
> 
> Ugly.  A function is_c_ptr(argtype) would be cleaner.
> 
> There's similar code in gen_marshal_input():
> 
>     if ret_type:
>         if c_type(ret_type).endswith("*"):
>             retval = "    %s retval = NULL;" % c_type(ret_type)
>         else:
>             retval = "    %s retval;" % c_type(ret_type)
>         ret += mcgen('''
> %(retval)s
> ''',
>                      retval=retval)
> 
> Don't you have to patch that, too?

Amos, have you answered this comment already? This series is pending on this.

> 
> > diff --git a/scripts/qapi.py b/scripts/qapi.py
> > index dc690bb..562d560 100644
> > --- a/scripts/qapi.py
> > +++ b/scripts/qapi.py
> > @@ -470,11 +470,17 @@ def find_enum(name):
> >  def is_enum(name):
> >      return find_enum(name) != None
> >  
> > +eatspace = '\033EATSPACE.'
> > +
> > +# A special suffix is added in c_type() for pointer types, and it's
> > +# stripped in mcgen(). So please notice this when you check the return
> > +# value of c_type() outside mcgen().
> >  def c_type(name, is_param=False):
> >      if name == 'str':
> >          if is_param:
> > -            return 'const char *'
> > -        return 'char *'
> > +            return 'const char *' + eatspace
> > +        return 'char *' + eatspace
> > +
> >      elif name == 'int':
> >          return 'int64_t'
> >      elif (name == 'int8' or name == 'int16' or name == 'int32' or
> > @@ -488,15 +494,15 @@ def c_type(name, is_param=False):
> >      elif name == 'number':
> >          return 'double'
> >      elif type(name) == list:
> > -        return '%s *' % c_list_type(name[0])
> > +        return '%s *%s' % (c_list_type(name[0]), eatspace)
> >      elif is_enum(name):
> >          return name
> >      elif name == None or len(name) == 0:
> >          return 'void'
> >      elif name == name.upper():
> > -        return '%sEvent *' % camel_case(name)
> > +        return '%sEvent *%s' % (camel_case(name), eatspace)
> >      else:
> > -        return '%s *' % name
> > +        return '%s *%s' % (name, eatspace)
> >  
> >  def genindent(count):
> >      ret = ""
> > @@ -521,7 +527,8 @@ def cgen(code, **kwds):
> >      return '\n'.join(lines) % kwds + '\n'
> >  
> >  def mcgen(code, **kwds):
> > -    return cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
> > +    raw = cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
> > +    return re.sub(re.escape(eatspace) + ' *', '', raw)
> >  
> >  def basename(filename):
> >      return filename.split("/")[-1]
> 
> Rest looks good.
> 

Re: [Qemu-devel] [PATCH v5 3/3] qapi: Suppress unwanted space between type and identifier

[Amos Kong]

On Mon, Jun 02, 2014 at 05:43:58PM +0200, Markus Armbruster wrote:
> Amos Kong <akong@redhat.com> writes:
> 
> > We always generate a space between type and identifier in parameter
> > and variable declarations, even when idiomatic C style doesn't have
> > a space there.  Suppress it.
> >
> > Signed-off-by: Amos Kong <akong@redhat.com>
> > ---
> >  scripts/qapi-commands.py |  2 +-
> >  scripts/qapi.py          | 19 +++++++++++++------
> >  2 files changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> > index 34f200a..4b49735 100644
> > --- a/scripts/qapi-commands.py
> > +++ b/scripts/qapi-commands.py
> > @@ -102,7 +102,7 @@ def gen_visitor_input_vars_decl(args):
> >  bool has_%(argname)s = false;
> >  ''',
> >                           argname=c_var(argname))
> > -        if c_type(argtype).endswith("*"):
> > +        if c_type(argtype).endswith("*" + eatspace):
> >              ret += mcgen('''
> >  %(argtype)s %(argname)s = NULL;
> >  ''',
> 
> Ugly.  A function is_c_ptr(argtype) would be cleaner.

def is_c_ptr(argtype):
    suffix = "*" + eatspace
    return c_type(argtype).endswith(suffix)
 
> There's similar code in gen_marshal_input():
> 
>     if ret_type:
>         if c_type(ret_type).endswith("*"):
>             retval = "    %s retval = NULL;" % c_type(ret_type)
>         else:
>             retval = "    %s retval;" % c_type(ret_type)
>         ret += mcgen('''
> %(retval)s
> ''',
>                      retval=retval)

Thanks, I searched by grep in the past, but I still lost one :( 

[amos@z qemu]$ git grep c_type|grep endswith
scripts/qapi-commands.py:        if c_type(argtype).endswith("*"):
scripts/qapi-commands.py:        if c_type(ret_type).endswith("*"):
 
> Don't you have to patch that, too?
> 
> > diff --git a/scripts/qapi.py b/scripts/qapi.py
> > index dc690bb..562d560 100644
> > --- a/scripts/qapi.py
> > +++ b/scripts/qapi.py
> > @@ -470,11 +470,17 @@ def find_enum(name):
> >  def is_enum(name):
> >      return find_enum(name) != None
> >  
> > +eatspace = '\033EATSPACE.'
> > +
> > +# A special suffix is added in c_type() for pointer types, and it's
> > +# stripped in mcgen(). So please notice this when you check the return
> > +# value of c_type() outside mcgen().
> >  def c_type(name, is_param=False):
> >      if name == 'str':
> >          if is_param:
> > -            return 'const char *'
> > -        return 'char *'
> > +            return 'const char *' + eatspace
> > +        return 'char *' + eatspace
> > +
> >      elif name == 'int':
> >          return 'int64_t'
> >      elif (name == 'int8' or name == 'int16' or name == 'int32' or
> > @@ -488,15 +494,15 @@ def c_type(name, is_param=False):
> >      elif name == 'number':
> >          return 'double'
> >      elif type(name) == list:
> > -        return '%s *' % c_list_type(name[0])
> > +        return '%s *%s' % (c_list_type(name[0]), eatspace)
> >      elif is_enum(name):
> >          return name
> >      elif name == None or len(name) == 0:
> >          return 'void'
> >      elif name == name.upper():
> > -        return '%sEvent *' % camel_case(name)
> > +        return '%sEvent *%s' % (camel_case(name), eatspace)
> >      else:
> > -        return '%s *' % name
> > +        return '%s *%s' % (name, eatspace)
> >  
> >  def genindent(count):
> >      ret = ""
> > @@ -521,7 +527,8 @@ def cgen(code, **kwds):
> >      return '\n'.join(lines) % kwds + '\n'
> >  
> >  def mcgen(code, **kwds):
> > -    return cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
> > +    raw = cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
> > +    return re.sub(re.escape(eatspace) + ' *', '', raw)
> >  
> >  def basename(filename):
> >      return filename.split("/")[-1]
> 
> Rest looks good.

-- 
			Amos.

Re: [Qemu-devel] [PATCH v5 3/3] qapi: Suppress unwanted space between type and identifier

[Markus Armbruster]

Amos Kong <akong@redhat.com> writes:

> On Mon, Jun 02, 2014 at 05:43:58PM +0200, Markus Armbruster wrote:
>> Amos Kong <akong@redhat.com> writes:
>> 
>> > We always generate a space between type and identifier in parameter
>> > and variable declarations, even when idiomatic C style doesn't have
>> > a space there.  Suppress it.
>> >
>> > Signed-off-by: Amos Kong <akong@redhat.com>
>> > ---
>> >  scripts/qapi-commands.py |  2 +-
>> >  scripts/qapi.py          | 19 +++++++++++++------
>> >  2 files changed, 14 insertions(+), 7 deletions(-)
>> >
>> > diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
>> > index 34f200a..4b49735 100644
>> > --- a/scripts/qapi-commands.py
>> > +++ b/scripts/qapi-commands.py
>> > @@ -102,7 +102,7 @@ def gen_visitor_input_vars_decl(args):
>> >  bool has_%(argname)s = false;
>> >  ''',
>> >                           argname=c_var(argname))
>> > -        if c_type(argtype).endswith("*"):
>> > +        if c_type(argtype).endswith("*" + eatspace):
>> >              ret += mcgen('''
>> >  %(argtype)s %(argname)s = NULL;
>> >  ''',
>> 
>> Ugly.  A function is_c_ptr(argtype) would be cleaner.
>
> def is_c_ptr(argtype):
>     suffix = "*" + eatspace
>     return c_type(argtype).endswith(suffix)

Either that, or a suitable conditional matching (relevant parts of)
c_type().  Your choice.

>> There's similar code in gen_marshal_input():
>> 
>>     if ret_type:
>>         if c_type(ret_type).endswith("*"):
>>             retval = "    %s retval = NULL;" % c_type(ret_type)
>>         else:
>>             retval = "    %s retval;" % c_type(ret_type)
>>         ret += mcgen('''
>> %(retval)s
>> ''',
>>                      retval=retval)
>
> Thanks, I searched by grep in the past, but I still lost one :( 

That's why we do reviews :)

[...]