On 10/01/2015 06:40 AM, Markus Armbruster wrote: > Eric Blake <ebl...@redhat.com> writes: > >> qapi-commands has a nice helper gen_err_check(), but did not > > but does not use > >> use it everywhere. In fact, using it in more places makes it >> easier to reduce the lines of code used for generating error >> checks. This in turn will make it easier for later patches >> to consolidate another common pattern among the generators. > > Does this message need updating now you're using gen_err_check() less > aggressively?
No - even though it was used less aggressively, it still made merging
gen_visit_fields() easier, and "using it in more places" does not imply
"in all possible places". So I think, other than the tense correction,
we are okay.
>
>> The generated code has one less blank line in qapi-event.c
>> functions, but has no semantic difference.
>
> Is it a stylistic improvement or the opposite?
>
Hmm. In qapi-visit.c, there are no blank lines between
visit_start_struct() and the matching visit_end_struct(). In
qapi-event.c, pre-patch we had (roughly):
visit_start_struct()
check err
visit fields
...
visit_end_struct()
check err
This patch deleted the first blank line (after visit_start_struct()) but
not the second (prior to visit_end_struct()), so the post-patch looks a
bit lopsided. But removing both blank lines doesn't seem too bad to me,
if you wanted to squash this in (not sure the line numbers are right):
diff --git i/scripts/qapi-event.py w/scripts/qapi-event.py
index b5e4d59..720486f 100644
--- i/scripts/qapi-event.py
+++ w/scripts/qapi-event.py
@@ -73,7 +73,6 @@ def gen_event_send(name, arg_type):
ret += gen_err_check()
ret += gen_visit_fields(arg_type.members, need_cast=True)
ret += mcgen('''
-
visit_end_struct(v, &err);
if (err) {
goto out;
Or the alternative is to keep generated output unchanged, by keeping the
newline:
diff --git i/scripts/qapi-event.py w/scripts/qapi-event.py
index b5e4d59..d261a46 100644
--- i/scripts/qapi-event.py
+++ w/scripts/qapi-event.py
@@ -71,6 +71,7 @@ def gen_event_send(name, arg_type):
''',
name=name)
ret += gen_err_check()
+ ret += '\n'
ret += gen_visit_fields(arg_type.members, need_cast=True)
ret += mcgen('''
I'm fine if you want to do either (or nothing) when turning this into a
pull request.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
