[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 15/18] qapi: Share gen_err_check()
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v7 15/18] qapi: Share gen_err_check() |
Date: |
Thu, 1 Oct 2015 07:01:27 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 |
On 10/01/2015 06:40 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> 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
- [Qemu-devel] [PATCH v7 00/18] post-introspection cleanups, subset A, Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 01/18] qapi: Sort qapi-schema tests, Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 06/18] qapi: Avoid assertion failure on union 'type' collision, Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 16/18] qapi: Share gen_visit_fields(), Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 14/18] qapi: Consistent generated code: minimize push_indent() usage, Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 15/18] qapi: Share gen_err_check(), Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 02/18] qapi: Improve 'include' error message, Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 12/18] qapi: Consistent generated code: prefer common labels, Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 04/18] qapi: Clean up qapi.py per pep8, Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 07/18] qapi: Add tests for empty unions, Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 05/18] qapi: Test for various name collisions, Eric Blake, 2015/10/08