Amos Kong <ak...@redhat.com> writes:
> On Fri, Apr 25, 2014 at 02:52:09PM +0200, Markus Armbruster wrote:
>> Amos Kong <ak...@redhat.com> 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 <ak...@redhat.com>
>> > ---
>> > 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.
[...]
