qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: marcandre.lureau@redhat.com, DirtY.iCE.hu@gmail.com,
	qemu-devel@nongnu.org, ehabkost@redhat.com,
	Michael Roth <mdroth@linux.vnet.ibm.com>
Subject: Re: [Qemu-devel] [PATCH v5 09/46] qapi: Use consistent generated code patterns
Date: Fri, 25 Sep 2015 13:06:57 -0600	[thread overview]
Message-ID: <56059B51.4050701@redhat.com> (raw)
In-Reply-To: <87h9misalf.fsf@blackfin.pond.sub.org>

[-- Attachment #1: Type: text/plain, Size: 1801 bytes --]

On 09/25/2015 10:54 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> We had some pointless differences in the generated code for visit,
>> command marshalling, and events; unifying them makes it easier for
>> future patches to consolidate to common helper functions.
>> - Consistently name the error variable 'err'
> 
> Before the patch, qapi-event.py consistently uses local_err, and
> qapi-commands.py uses both names.  I like your change.
> 
>> - Consistently use the labels 'out' and 'out_obj'
> 
> Before, qapi-event.py consistently uses 'clean', and qapi-visit.py uses
> both 'out_obj' and 'out_end'.  Good change.
> 
>> - If allocation fails, jump to the right label rather than
>> indenting everything else
> 
> Good idea.  Just one instance, in gen_visit_union().
> 
> Not mentioned: name the Visitor * parameter 'v' instead of 'm'.

Whoops - I merged several patches from v4 into just this one for v5, but
forgot to update the commit message to match.

>  No idea
> where that 'm' comes from, but it has spilled over into qom/object.c and
> qom/qom-object.c.  Let's ignore that for now.

I didn't even notice that.

> 
>> No change in semantics to the generated code.
> 
> Yes, but you need to update docs/qapi-code-gen.txt.

Indeed.

> 
> Pretty mechanical changes.  They look good to me, but as always, this
> kind is easier to review when you do exactly one mechanical change per
> patch.

Sounds like my merge was premature; I'll split this back out into
separate patches, and update the docs to match (where things are
affected - our docs don't give any examples of the C code for visiting a
union).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

  reply	other threads:[~2015-09-25 19:07 UTC|newest]

Thread overview: 108+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-21 21:57 [Qemu-devel] [PATCH v5 00/46] post-introspection cleanups, and qapi-ify netdev_add Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 01/46] qapi: Sort qapi-schema tests Eric Blake
2015-09-23 14:26   ` Eric Blake
2015-09-23 15:09     ` Markus Armbruster
2015-09-23 15:19       ` Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 02/46] qapi: Clean up qapi.py per pep8 Eric Blake
2015-09-22 14:00   ` Markus Armbruster
2015-09-22 14:58     ` Eric Blake
2015-09-23  9:20       ` Markus Armbruster
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 03/46] qapi: Test for C member name collisions Eric Blake
2015-09-22 15:23   ` Markus Armbruster
2015-09-22 17:52     ` Eric Blake
2015-09-23  9:43       ` Markus Armbruster
2015-09-23 12:45         ` Eric Blake
2015-09-23 14:02           ` Markus Armbruster
2015-09-23 14:19             ` Eric Blake
2015-09-23 15:12               ` Markus Armbruster
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 04/46] qapi: Add tests for empty unions Eric Blake
2015-09-24 14:16   ` Markus Armbruster
2015-09-24 15:52     ` Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 05/46] qapi: Test use of 'number' within alternates Eric Blake
2015-09-24 14:36   ` Markus Armbruster
2015-09-24 16:00     ` Eric Blake
2015-09-24 16:29       ` Markus Armbruster
2015-09-25 22:32         ` Eric Blake
2015-09-28  9:26           ` Markus Armbruster
2015-09-25 22:50         ` Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 06/46] qapi: Improve 'include' error message Eric Blake
2015-09-24 14:39   ` Markus Armbruster
2015-09-24 16:04     ` Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 07/46] qapi: Don't pass pre-existing error to later call Eric Blake
2015-09-24 14:58   ` Markus Armbruster
2015-09-24 16:14     ` Eric Blake
2015-09-26 21:05       ` Eric Blake
2015-09-28  9:14         ` Markus Armbruster
2015-10-06 21:10           ` [Qemu-devel] [RFC PATCH] qapi: split visit_end_struct() into pieces Eric Blake
2015-10-07 12:00             ` Markus Armbruster
2015-10-07 13:08               ` Markus Armbruster
2015-10-07 14:57               ` Eric Blake
2015-10-07 15:23                 ` Markus Armbruster
2015-09-26 21:41     ` [Qemu-devel] [PATCH v5 07/46] qapi: Don't pass pre-existing error to later call Eric Blake
2015-09-27  2:26       ` Eric Blake
2015-09-28  9:24       ` Markus Armbruster
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 08/46] qapi: Reuse code for flat union base validation Eric Blake
2015-09-25 16:30   ` Markus Armbruster
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 09/46] qapi: Use consistent generated code patterns Eric Blake
2015-09-25 16:54   ` Markus Armbruster
2015-09-25 19:06     ` Eric Blake [this message]
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 10/46] qapi: Merge generation of per-member visits Eric Blake
2015-09-28  6:17   ` Markus Armbruster
2015-09-28 15:40     ` Eric Blake
2015-09-29  7:37       ` Markus Armbruster
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 11/46] qapi: Don't use info as witness of implicit object type Eric Blake
2015-09-28 12:43   ` Markus Armbruster
2015-09-29  3:58     ` Eric Blake
2015-09-29  7:51       ` Markus Armbruster
2015-09-30  4:13         ` [Qemu-devel] [RFC PATCH] qapi: Use callback to determine visit filtering Eric Blake
2015-10-01  6:12           ` Markus Armbruster
2015-10-01 14:09             ` Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 12/46] qapi: Track location that created an implicit type Eric Blake
2015-09-28 12:56   ` Markus Armbruster
2015-09-29  4:03     ` Eric Blake
2015-09-29  8:02       ` Markus Armbruster
2015-09-30 16:02         ` Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 13/46] qapi: Track owner of each object member Eric Blake
2015-09-30 16:06   ` Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 14/46] qapi: Detect collisions in C member names Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 15/46] qapi: Defer duplicate member checks to schema check() Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 16/46] qapi: Detect base class loops Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 17/46] qapi: Provide nicer array names in introspection Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 18/46] qapi-introspect: Guarantee particular sorting Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 19/46] qapi: Simplify visiting of alternate types Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 20/46] qapi: Fix alternates that accept 'number' but not 'int' Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 21/46] qmp: Fix reference-counting of qnull on empty output visit Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 22/46] qapi: Don't abuse stack to track qmp-output root Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 23/46] qapi: Remove dead visitor code Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 24/46] qapi: Document visitor interfaces Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 25/46] qapi: Plug leaks in test-qmp-input-visitor Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 26/46] qapi: Test failure in middle of array parse Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 27/46] qapi: Simplify visits of optional fields Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 28/46] qapi: Rework deallocation of partial struct Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 29/46] qapi: Change visit_type_FOO() to no longer return partial objects Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 30/46] net: use Netdev instead of NetClientOptions in client init Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 31/46] qapi: use 'type' in generated C code to match QMP union wire form Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 32/46] qapi: Hide tag_name data member of variants Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 33/46] vnc: hoist allocation of VncBasicInfo to callers Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 34/46] qapi: Unbox base members Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 35/46] qapi-visit: Remove redundant functions for flat union base Eric Blake
2015-09-23 20:55   ` Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 36/46] qapi: Avoid use of 'data' member of qapi unions Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 37/46] qapi: Forbid empty unions and useless alternates Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 38/46] qapi: Drop useless 'data' member of unions Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 39/46] qapi: Plumb in 'box' to qapi generator lower levels Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 40/46] qapi: Implement boxed structs for commands/events Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 41/46] qapi: Support boxed unions Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 42/46] qapi: support implicit structs in OptsVisitor Eric Blake
2015-09-21 21:57 ` [Qemu-devel] [PATCH v5 43/46] qapi: Change Netdev into a flat union Eric Blake
2015-09-21 21:58 ` [Qemu-devel] [PATCH v5 44/46] net: Use correct type for bool flag Eric Blake
2015-09-21 21:58 ` [Qemu-devel] [PATCH v5 45/46] net: Complete qapi-fication of netdev_add Eric Blake
2015-09-23 15:40   ` Paolo Bonzini
2015-09-23 16:37     ` Eric Blake
2015-09-25 16:48       ` Paolo Bonzini
2015-09-28  9:31         ` Markus Armbruster
2015-09-28 11:29           ` Paolo Bonzini
2015-09-21 21:58 ` [Qemu-devel] [PATCH v5 46/46] qapi: Allow anonymous base for flat union Eric Blake
2015-09-23 20:59   ` Eric Blake
2015-09-28 13:07 ` [Qemu-devel] [PATCH v5 00/46] post-introspection cleanups, and qapi-ify netdev_add Markus Armbruster
2015-09-29  3:43   ` Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56059B51.4050701@redhat.com \
    --to=eblake@redhat.com \
    --cc=DirtY.iCE.hu@gmail.com \
    --cc=armbru@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).