On 04/18/2014 09:56 PM, Amos Kong wrote:
> 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.

Second sentence is awkward; maybe:

Avoiding the space when appropriate makes us match the coding style.

> 
>   char * value;
>         ^
>   qapi_free_NameInfo(NameInfo * obj)
>                                ^
> It's fussy to add checking in each mcgen(), this patch just addes

s/addes/adds/

> the necessary space in c_type(), and remove original space in mcgen().

s/remove/removes the/

> 
> Signed-off-by: Amos Kong <ak...@redhat.com>
> ---
>  scripts/qapi-types.py | 10 +++++-----
>  scripts/qapi-visit.py | 20 ++++++++++----------
>  scripts/qapi.py       | 14 +++++++-------
>  3 files changed, 22 insertions(+), 22 deletions(-)

>      union {
> -        %(type)s value;
> +        %(type)svalue;

Code itself does what it claims.  It's a bit harder to read the
generator without the space, and I might have added a comment to
c_type() explaining that the output string includes the space except for
pointer types.  I'll let others decide whether to take the patch, but
I'm comfortable if the commit message is fixed and you want to add:

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

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to