Eric Blake <ebl...@redhat.com> writes:
> On 07/01/2015 02:21 PM, Markus Armbruster wrote:
>> gen_sync_call()'s parameter indent is useless: gen_sync_call() uses it
>> only as optional argument for push_indent() and pop_indent(), their
>> default is four, and gen_sync_call()'s only caller passes four.
>>
>> gen_visitor_input_containers_decl()'s parameter obj is always
>> "QOBJECT(args)".
>
> It might be nice to call out that several other functions are also
> stripped of unused arguments. I was assuming that only two functions
> (and their callers) would be modified, but the patch touched more:
I'll rephrase the commit message to make this more obvious.
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>> scripts/qapi-commands.py | 27 +++++++++++++--------------
>> scripts/qapi-event.py | 1 -
>> scripts/qapi-types.py | 1 -
>> scripts/qapi-visit.py | 47
>> ++++++++++++++++++++++-------------------------
>> scripts/qapi.py | 1 -
>> 5 files changed, 35 insertions(+), 42 deletions(-)
>
>> @@ -161,7 +160,7 @@ qapi_dealloc_visitor_cleanup(md);
>> pop_indent()
>> return ret.rstrip()
>>
>> -def gen_marshal_output(name, args, ret_type, middle_mode):
>> +def gen_marshal_output(name, ret_type):
>> if not ret_type:
>> return ""
>
> For example, gen_marshal_output() was not mentioned in the commit message.
>
>>
>> @@ -194,14 +193,14 @@ out:
>>
>> return ret
>>
>> -def gen_marshal_input_decl(name, args, ret_type, middle_mode):
>> +def gen_marshal_input_decl(name, middle_mode):
>
> Or gen_marshal_input_decl()
>
>> +++ b/scripts/qapi-event.py
>> @@ -199,7 +199,6 @@ const char *%(event_enum_name)s_lookup[] = {
>> ''',
>> event_enum_name = event_enum_name)
>>
>> - i = 0
>> for string in event_enum_strings:
>
> I guess the subject line covered deletion of unused internal variables.
>
>> +++ b/scripts/qapi-visit.py
>
>> @@ -441,44 +440,42 @@
>> fdecl.write(guardend("QAPI_VISIT_BUILTIN_VISITOR_DECL"))
>
>> elif expr.has_key('union'):
>> ret = generate_visit_union(expr)
>> - ret += generate_visit_list(expr['union'], expr['data'])
>> + ret += generate_visit_list(expr['union'])
>> fdef.write(ret)
>>
>> enum_define = discriminator_find_enum_define(expr)
>> ret = ""
>> if not enum_define:
>> - ret = generate_decl_enum('%sKind' % expr['union'],
>> - expr['data'].keys())
>> - ret += generate_declaration(expr['union'], expr['data'])
>> + ret = generate_decl_enum('%sKind' % expr['union'])
>> + ret += generate_declaration(expr['union'])
>
> As long as you are touching this, generate_decl_enum(expr['union'] +
> 'Kind') would read nicer.
It'll go away in PATCH 27. Let's keep this patch simple.
>> fdecl.write(ret)
>> elif expr.has_key('alternate'):
>> ret = generate_visit_alternate(expr['alternate'], expr['data'])
>> - ret += generate_visit_list(expr['alternate'], expr['data'])
>> + ret += generate_visit_list(expr['alternate'])
>> fdef.write(ret)
>>
>> - ret = generate_decl_enum('%sKind' % expr['alternate'],
>> - expr['data'].keys())
>> - ret += generate_declaration(expr['alternate'], expr['data'])
>> + ret = generate_decl_enum('%sKind' % expr['alternate'])
>
> Same here.
Likewise.
> At any rate, no change to generated output. So assuming you are happy
> with the commit message, or take my advice to improve it, the code
> cleanup itself is fine.
>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
Thanks!
