[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC v2 33/47] qapi: Clean up after recent conver
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH RFC v2 33/47] qapi: Clean up after recent conversions to QAPISchemaVisitor |
Date: |
Thu, 23 Jul 2015 10:48:49 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 |
On 07/01/2015 02:22 PM, Markus Armbruster wrote:
> Drop helper functions that are now unused.
>
> Make pylint reasonably happy.
>
> Rename generate_FOO() functions to gen_FOO() for consistency.
>
> Use more consistent and sensible variable names.
>
> Consistently use c_ for mapping keys when their value is a C
> identifier or type.
>
> Simplify gen_enum() and gen_visit_union()
>
> Consistently use single quotes for C text string literals.
Not sure if it is worth splitting this into pieces. Fortunately, there
are no changes to generated output.
>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
> scripts/qapi-commands.py | 109 +++++++++++++++++++------------------
> scripts/qapi-event.py | 117 +++++++++++++++++++---------------------
> scripts/qapi-types.py | 68 ++++++++++++-----------
> scripts/qapi-visit.py | 121 ++++++++++++++++++++---------------------
> scripts/qapi.py | 138
> +++++++++--------------------------------------
> 5 files changed, 229 insertions(+), 324 deletions(-)
>
> diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> index d3bddb6..5d11032 100644
> --- a/scripts/qapi-commands.py
> +++ b/scripts/qapi-commands.py
> @@ -15,20 +15,20 @@
> from qapi import *
> import re
>
> -def generate_command_decl(name, args, ret_type):
> - arglist=""
> +def gen_command_decl(name, args, rets):
I can see how 'args' is plural (even if it is a single string for the
name of a type containing the args), but should it be 'ret' instead of
'rets'?
> @@ -40,34 +40,37 @@ if (%(err)s) {
> ''',
> err=err)
>
> -def gen_sync_call(name, args, ret_type):
> - ret = ""
> - arglist=""
> - retval=""
> - if ret_type:
> - retval = "retval = "
> +def gen_call(name, args, rets):
At least you're consistent on naming it 'rets',
> + ret = ''
and the naming lets you distinguish between the parameter (the type
describing returned fields) and the local string (the generated C code
holding the return information).
> @@ -82,45 +76,46 @@ def generate_event_implement(api_name, event_name,
> params):
>
> + # Ugly: need to cast away the const
> if memb.type.name == "str":
> - var_type = "(char **)"
> + cast = "(char **)"
And to think I called it out in a previous patch. So you noticed it too :)
Don't you want to use '(char **)' here, since it is a literal string
destined for generated C?
> else:
> - var_type = ""
> + cast = ""
and '' here?
> +++ b/scripts/qapi-types.py
> @@ -16,22 +16,22 @@ from qapi import *
> def gen_fwd_object_or_array(name):
> return mcgen('''
>
> -typedef struct %(name)s %(name)s;
> +typedef struct %(c_name)s %(c_name)s;
> ''',
> - name=c_name(name))
> + c_name=c_name(name))
>
> def gen_array(name, element_type):
> return mcgen('''
>
> -struct %(name)s {
> +struct %(c_name)s {
> union {
> %(c_type)s value;
> uint64_t padding;
> };
> - struct %(name)s *next;
> + struct %(c_name)s *next;
May be some churn here if you like my comment earlier in the series that
this 'struct' is redundant.
> +++ b/scripts/qapi-visit.py
> -def generate_visit_struct_fields(name, members, base = None):
> +def gen_visit_struct_fields(name, base, members):
> struct_fields_seen.add(name)
> - type=base.c_name(), c_name=c_name('base'))
> + c_type=base.c_name(), c_name=c_name('base'))
Possible churn here based on my earlier comments about c_name(constant)
being constant.
Fairly big, but aside from some minor '' quoting issues, I didn't see
anything wrong.
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH RFC v2 27/47] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs, (continued)
[Qemu-devel] [PATCH RFC v2 24/47] tests/qapi-schema: Convert test harness to QAPISchemaVisitor, Markus Armbruster, 2015/07/01
[Qemu-devel] [PATCH RFC v2 41/47] qom: Don't use 'gen': false for qom-get, qom-set, object-add, Markus Armbruster, 2015/07/01
[Qemu-devel] [PATCH RFC v2 33/47] qapi: Clean up after recent conversions to QAPISchemaVisitor, Markus Armbruster, 2015/07/01
- Re: [Qemu-devel] [PATCH RFC v2 33/47] qapi: Clean up after recent conversions to QAPISchemaVisitor,
Eric Blake <=
[Qemu-devel] [PATCH RFC v2 19/47] qapi: Generated code cleanup, Markus Armbruster, 2015/07/01
[Qemu-devel] [PATCH RFC v2 45/47] qapi: New QMP command query-schema for QMP schema introspection, Markus Armbruster, 2015/07/01