[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC v2 19/47] qapi: Generated code cleanup
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH RFC v2 19/47] qapi: Generated code cleanup |
Date: |
Tue, 21 Jul 2015 11:43:22 -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:
> Clean up white-space, brace placement, and superfluous
Incomplete sentence. I bet it's because your editor line-wrapped, and
the rest of your sentence was something like '#ifndef in a .c file.'
(see [1] below), then git ate the line thinking it was a comment. I
think I'm okay with cramming all of these cleanups into one patch rather
than trying to do one style of cleanup per patch (blank lines, {
placement, and guard cleanup), but the commit message could do a better
job of explaining things.
>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
> scripts/qapi-commands.py | 1 +
> scripts/qapi-event.py | 3 +--
> scripts/qapi-types.py | 66
> +++++++++++++++++++++++-------------------------
> scripts/qapi-visit.py | 1 +
> 4 files changed, 34 insertions(+), 37 deletions(-)
Missing changes to docs/qapi-code-gen.txt to reflect the improvements.
Since having docs that are stale compared to implementation can be
misleading, I'd like to wait for v2 before giving my R-b; but see also
[2] below.
Overall, I'm a fan of the cleanups.
I'm going to intersperse diffs of the generated files that were caused
by some of these changes:
>
> diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> index cfbd59c..223216d 100644
> --- a/scripts/qapi-commands.py
> +++ b/scripts/qapi-commands.py
> @@ -222,6 +222,7 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
> ret += mcgen('''
>
> (void)args;
> +
> ''')
This and similar hunts avoids extra blank lines. Not worth showing a
diff to the generated files.
>
> ret += gen_sync_call(name, args, ret_type)
> diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
> index 88b0620..7f238df 100644
> --- a/scripts/qapi-event.py
> +++ b/scripts/qapi-event.py
> @@ -167,8 +167,7 @@ extern const char *%(event_enum_name)s_lookup[];
> event_enum_name = event_enum_name)
>
> enum_decl = mcgen('''
> -typedef enum %(event_enum_name)s
> -{
> +typedef enum %(event_enum_name)s {
Several hunks like this; gets us generally closer to qemu style; with
generated diffs like:
qapi-types.h:
-typedef struct int32List
-{
+typedef struct int32List {
union {
int32_t value;
uint64_t padding;
> @@ -105,7 +103,8 @@ struct %(name)s
>
> def generate_enum_lookup(name, values):
> ret = mcgen('''
> -const char * const %(name)s_lookup[] = {
> +
> +const char *const %(name)s_lookup[] = {
[2] generated diffs like this:
qapi-types.c:
-const char * const OnOffAuto_lookup[] = {
+const char *const OnOffAuto_lookup[] = {
Hmm - we already failed to update docs/qapi-code-gen.txt in the past; we
added a const in commit 2e4450ff that is missing from the documentation.
> @@ -335,22 +333,22 @@ for typename in builtin_types.keys():
> fdecl.write(guardend("QAPI_TYPES_BUILTIN_STRUCT_DECL"))
>
> for expr in exprs:
> - ret = "\n"
> + ret = ""
> if expr.has_key('struct'):
> ret += generate_fwd_struct(expr['struct'])
> elif expr.has_key('enum'):
> - ret += generate_enum(expr['enum'], expr['data']) + "\n"
> + ret += generate_enum(expr['enum'], expr['data'])
> ret += generate_fwd_enum_struct(expr['enum'])
> fdef.write(generate_enum_lookup(expr['enum'], expr['data']))
More work at avoiding extra blank lines.
> @@ -370,34 +368,32 @@ fdecl.write(guardend("QAPI_TYPES_BUILTIN_CLEANUP_DECL"))
> # have the functions defined, so we use -b option to provide control
> # over these cases
> if do_builtins:
> - fdef.write(guardstart("QAPI_TYPES_BUILTIN_CLEANUP_DEF"))
> for typename in builtin_types.keys():
> fdef.write(generate_type_cleanup(typename + "List"))
> - fdef.write(guardend("QAPI_TYPES_BUILTIN_CLEANUP_DEF"))
[1] this is the hunk whose description got corrupted in your commit
message. It gives a diff like this:
qapi-types.c:
-
-#ifndef QAPI_TYPES_BUILTIN_CLEANUP_DEF
-#define QAPI_TYPES_BUILTIN_CLEANUP_DEF
-
-
void qapi_free_int32List(int32List *obj)
We conditionally declare the functions in the .h, but the .c is not
compiled multiple times, so there is no need for a header-style guard.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH RFC v2 41/47] qom: Don't use 'gen': false for qom-get, qom-set, object-add, (continued)
[Qemu-devel] [PATCH RFC v2 19/47] qapi: Generated code cleanup, Markus Armbruster, 2015/07/01
- Re: [Qemu-devel] [PATCH RFC v2 19/47] qapi: Generated code cleanup,
Eric Blake <=
[Qemu-devel] [PATCH RFC v2 45/47] qapi: New QMP command query-schema for QMP schema introspection, Markus Armbruster, 2015/07/01
- Re: [Qemu-devel] [PATCH RFC v2 45/47] qapi: New QMP command query-schema for QMP schema introspection, Eric Blake, 2015/07/23
- Re: [Qemu-devel] [PATCH RFC v2 45/47] qapi: New QMP command query-schema for QMP schema introspection, Markus Armbruster, 2015/07/28
- Re: [Qemu-devel] [PATCH RFC v2 45/47] qapi: New QMP command query-schema for QMP schema introspection, Eric Blake, 2015/07/28
- Re: [Qemu-devel] [PATCH RFC v2 45/47] qapi: New QMP command query-schema for QMP schema introspection, Markus Armbruster, 2015/07/29
- Re: [Qemu-devel] [PATCH RFC v2 45/47] qapi: New QMP command query-schema for QMP schema introspection, Eric Blake, 2015/07/29
- Re: [Qemu-devel] [PATCH RFC v2 45/47] qapi: New QMP command query-schema for QMP schema introspection, Markus Armbruster, 2015/07/29
[Qemu-devel] [PATCH RFC v2 44/47] qapi: Pseudo-type '**' is now unused, drop it, Markus Armbruster, 2015/07/01