qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 2/2] qapi: add a special string in c_type()'s


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH v2 2/2] qapi: add a special string in c_type()'s result to each redundant space
Date: Mon, 28 Apr 2014 09:34:13 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux)

Amos Kong <address@hidden> writes:

> On Fri, Apr 25, 2014 at 02:52:09PM +0200, Markus Armbruster wrote:
>> Amos Kong <address@hidden> writes:
>
> Hi Markus,
>  
>> > Currently we always add a space after c_type in mcgen(), there is
>> > some redundant space in generated code. The space isn't needed for
>> > points by the coding style.
>> 
>> You mean pointers.
>> 
>> >
>> >   char * value;
>> >         ^
>> >   qapi_free_NameInfo(NameInfo * obj)
>> >                                ^
>> >
>> > This patch added a special string 'EATSPACE' in the end of c_type()'s
>> > result only for point type. The string and the following space will be
>> > striped in mcgen().
>> 
>> Suggest:
>> 
>>     qapi: Suppress unwanted space between type and identifier
>> 
>>     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.
>> 
>> This explains what you do and why, but not how.  A commit message should
>> always cover "what" and "why", but "how" can be ommitted in simple cases
>> like this one.
>> 
>> Use of "EATSPACE" as marker is unnecessarily fragile, as it could
>> legitimately occur in generated code.  Recommend to pick something that
>> can't, such as "\033EATSPACE."
>
> "\033EATSPACE." works, I will use it.
>  
>> > Signed-off-by: Amos Kong <address@hidden>
>> > ---
>> >  scripts/qapi-commands.py |  2 +-
>> >  scripts/qapi.py          | 11 ++++++-----
>> >  2 files changed, 7 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
>> > index 9734ab0..0ebb1b9 100644
>> > --- a/scripts/qapi-commands.py
>> > +++ b/scripts/qapi-commands.py
>> > @@ -26,7 +26,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)
>> > +        argtype = c_type(argtype).replace('EATSPACE', '')
>> >          if argtype == "char *":
>> >              argtype = "const char *"
>> >          if optional:
>> 
>> Ugly, and you make it uglier :)
>
> It's not just ugly, eatspace string is cleaned, so the space can be
> eaten.

I was too terse again, sorry.  I mean: generate_command_decl() messing
with the result of c_type() like this is ugly, because it requires it to
know exactly what c_type() does.

[...]



reply via email to

[Prev in Thread] Current Thread [Next in Thread]