From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45662) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zuacz-0008VI-IJ for qemu-devel@nongnu.org; Fri, 06 Nov 2015 01:36:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zuacv-00045c-FE for qemu-devel@nongnu.org; Fri, 06 Nov 2015 01:36:01 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44090) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zuacv-00045U-4r for qemu-devel@nongnu.org; Fri, 06 Nov 2015 01:35:57 -0500 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id 8839BC0A1B16 for ; Fri, 6 Nov 2015 06:35:56 +0000 (UTC) From: Eric Blake Date: Thu, 5 Nov 2015 23:35:24 -0700 Message-Id: <1446791754-23823-1-git-send-email-eblake@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] [PATCH v10 00/30] qapi member collision (post-introspection cleanups, subset C') List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: armbru@redhat.com No pending prerequisites; based on qemu.git master Also available as a tag at this location: git fetch git://repo.or.cz/qemu/ericb.git qapi-cleanupv9c and will soon be part of my branch with the rest of the v5 series, at: http://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/qapi v10 notes: Split several patches, redo the middle patches from Markus to be back in the order they were first posted, some fallout change to my patches due to the nicer pattern of minimizing conditionals inside .check(), by instead calling .check_clash() as needed. Change data->err magic in tests to instead use a new helper error_free_or_abort(). Add a patch that would prevent qapi case-insensitive clashes. I am redoing my subset boundaries slightly: patches 23-27 of v9 (updating the alternate layout) will be delayed to subset D, and 2 other patches previously posted in subset D are now here (turning qapi clash checking into actual error messages), so the subject line of this cover letter is slightly different. Hopefully, we are converging on something that will be ready for a pull request, especially for the earlier patches of this subset. Backport diff in relation to v9: 001/30:[----] [--] 'qapi: Use generated TestStruct machinery in tests' 002/30:[----] [--] 'qapi: Strengthen test of TestStructList' 003/30:[down] 'qobject: Protect against use-after-free in qobject_decref(= )' 004/30:[down] 'qapi: Share test_init code in test-qmp-input*' 005/30:[0001] [FC] 'qapi: Plug leaks in test-qmp-*' 006/30:[down] 'qapi: Simplify non-error testing in test-qmp-*' 007/30:[down] 'qapi: Simplify error cleanup in test-qmp-*' 008/30:[----] [--] 'qapi: More tests of alternate output' 009/30:[0010] [FC] 'qapi: Test failure in middle of array parse' 010/30:[0025] [FC] 'qapi: More tests of input arrays' 011/30:[----] [--] 'qapi: Provide nicer array names in introspection' 012/30:[0014] [FC] 'qapi-introspect: Document lack of sorting' 013/30:[----] [--] 'qapi: Track simple union tag in object.local_members' 014/30:[----] [--] 'qapi-types: Consolidate gen_struct() and gen_union()' 015/30:[----] [--] 'qapi-types: Simplify gen_struct_field[s]' 016/30:[0005] [FC] 'qapi: Drop obsolete tag value collision assertions' 017/30:[0002] [FC] 'qapi: Simplify QAPISchemaObjectTypeMember.check()' 018/30:[down] 'qapi: Clean up after previous commit' 019/30:[----] [-C] 'qapi: Fix up commit 7618b91's clash sanity checking c= hange' 020/30:[0002] [FC] 'qapi: Eliminate QAPISchemaObjectType.check() variable= members' 021/30:[----] [--] 'qapi: Factor out QAPISchemaObjectTypeMember.check_cla= sh()' 022/30:[----] [-C] 'qapi: Simplify QAPISchemaObjectTypeVariants.check()' 023/30:[0023] [FC] 'qapi: Check for qapi collisions of flat union branche= s' 024/30:[0013] [FC] 'qapi: Factor out QAPISchemaObjectType.check_clash()' 025/30:[down] 'qapi: Hoist tag collision check to Variants.check()' 026/30:[----] [--] 'qapi: Remove outdated tests related to QMP/branch col= lisions' 027/30:[down] 'qapi: Track owner of each object member' 028/30:[down] 'qapi: Detect collisions in C member names' 029/30:[down] 'cpu: Convert CpuInfo into flat union' 030/30:[down] 'qapi: Forbid case-insensitive clashes' v9 notes: https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg00652.html More patches added, and several reorganized. Lots of new patches from Markus, although not in the order originally proposed. The first 8 patches are fairly straightforward, and could probably be taken as-is. Patch 9 is a rewrite of v8 4/17, but in the opposite direction (document that no sorting is done, rather than attempting to sort), so it may need further fine-tuning. Patches 12-21 represents a fusion of Markus' and my attempts to rewrite v5 7/17 into a more-reviewable set of patches, and caused further churn later in the series. Patch 23 still uses tag_member.type =3D=3D None; I ran out of time to work on Markus' idea of providing an instance of QAPISchemaBuiltinType to fill the role for 'qtype_code' without being exposed through .json files and without breaking the invariant of a valid member.type after check(), and wanted to get the rest of the series started under revew. So I may need a followup patch or even a v10 of the later half of this series after exploring that idea more. v8 notes: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg06674.html Minor changes when rebasing to latest, improved commit messages in a few places, plus the addition of a few new patches. Markus started reviewing v7, but hadn't gotten very far. v7 notes: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04112.html Patches 1-3 of the previous round have moved to subset B; patch 7 is gone, and a couple of new patches are present in this round. The latest version of subset B made it much easier to reason about collision detection (namely, tag values can't collide with QMP values thanks to a named rather than anonymous union in the C type), and I like how things turned out. I suspect the hardest part of the review will be patches 5/14 and 7/14, although none of this has really had much review in any earlier versions. v6 notes: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg01980.html Add some patches and rebase onto work on subset B. Rearrange some patches from v5 (this set includes 17-20, 23, 25-27). Backport diff gets a bit confused by one patch title changing. Not much direct review comments, although some of the changes here are updated based on comments made on other patches in the v5 series. Subset D (and more?) will come later. In v5: https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg05410.html I _did_ rearrange patches to try and group related features: 1-2: Groundwork cleanups 3-5: Add more test cases 6-16: Front-end cleanups 17-18: Introspection output cleanups 19-20: 'alternate' type cleanups 21-29: qapi visitor cleanups 30-45: qapi-ify netdev_add 46: add qapi shorthand for flat unions Lots of fixes based on additional testing, and rebased to track other changes that happened in the meantime. The series is huge; I can split off smaller portions as requested. In v4: https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg02580.html add some more clean up patches rebase to Markus' recent work pull in part of Zolt=C3=A1n's work to make netdev_add a flat union, further enhancing it to be introspectible I might be able to rearrange some of these patches, or separate it into smaller independent series, if requested; but I'm posting now to get review started. In v3: https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg02059.html redo cleanup of dealloc of partial struct add patches to make all visit_type_*() avoid leaks on failure add patches to allow boxed command arguments and events In v2: https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg00900.html rebase to Markus' v3 series rework how comments are emitted for fields inherited from base additional patches added for deleting colliding 'void *data' documentation updates to match code changes v1 was here: https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg05266.html https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg05325.html Eric Blake (23): qapi: Use generated TestStruct machinery in tests qapi: Strengthen test of TestStructList qobject: Protect against use-after-free in qobject_decref() qapi: Share test_init code in test-qmp-input* qapi: Plug leaks in test-qmp-* qapi: Simplify non-error testing in test-qmp-* qapi: Simplify error cleanup in test-qmp-* qapi: More tests of alternate output qapi: Test failure in middle of array parse qapi: More tests of input arrays qapi: Provide nicer array names in introspection qapi-introspect: Document lack of sorting qapi: Track simple union tag in object.local_members qapi-types: Consolidate gen_struct() and gen_union() qapi-types: Simplify gen_struct_field[s] qapi: Check for qapi collisions of flat union branches qapi: Factor out QAPISchemaObjectType.check_clash() qapi: Hoist tag collision check to Variants.check() qapi: Remove outdated tests related to QMP/branch collisions qapi: Track owner of each object member qapi: Detect collisions in C member names cpu: Convert CpuInfo into flat union qapi: Forbid case-insensitive clashes Markus Armbruster (7): qapi: Drop obsolete tag value collision assertions qapi: Simplify QAPISchemaObjectTypeMember.check() qapi: Clean up after previous commit qapi: Fix up commit 7618b91's clash sanity checking change qapi: Eliminate QAPISchemaObjectType.check() variable members qapi: Factor out QAPISchemaObjectTypeMember.check_clash() qapi: Simplify QAPISchemaObjectTypeVariants.check() cpus.c | 31 ++- docs/qapi-code-gen.txt | 31 ++- hmp.c | 30 ++- include/qapi/qmp/qobject.h | 1 + qapi-schema.json | 120 +++++++-- qapi/introspect.json | 17 +- scripts/qapi-introspect.py | 8 +- scripts/qapi-types.py | 68 ++---- scripts/qapi-visit.py | 9 +- scripts/qapi.py | 135 ++++++---- tests/Makefile | 4 +- tests/qapi-schema/args-case-clash.err | 1 + ...{union-clash-type.exit =3D> args-case-clash.exit} | 0 tests/qapi-schema/args-case-clash.json | 5 + .../{union-clash-type.out =3D> args-case-clash.out} | 0 tests/qapi-schema/args-name-clash.err | 1 + tests/qapi-schema/args-name-clash.exit | 2 +- tests/qapi-schema/args-name-clash.json | 6 +- tests/qapi-schema/args-name-clash.out | 6 - tests/qapi-schema/flat-union-clash-branch.err | 0 tests/qapi-schema/flat-union-clash-branch.exit | 1 - tests/qapi-schema/flat-union-clash-branch.json | 18 -- tests/qapi-schema/flat-union-clash-branch.out | 14 -- tests/qapi-schema/flat-union-clash-type.err | 1 - tests/qapi-schema/flat-union-clash-type.exit | 1 - tests/qapi-schema/flat-union-clash-type.json | 14 -- tests/qapi-schema/flat-union-clash-type.out | 0 tests/qapi-schema/qapi-schema-test.json | 6 +- tests/qapi-schema/qapi-schema-test.out | 15 +- tests/qapi-schema/union-clash-data.out | 1 + tests/qapi-schema/union-clash-type.err | 1 - tests/qapi-schema/union-clash-type.json | 9 - tests/qapi-schema/union-empty.out | 1 + tests/test-qmp-commands.c | 8 +- tests/test-qmp-common.h | 22 ++ tests/test-qmp-event.c | 1 + tests/test-qmp-input-strict.c | 131 +++------- tests/test-qmp-input-visitor.c | 272 +++++++++------= ------ tests/test-qmp-output-visitor.c | 151 +++--------- tests/test-visitor-serialization.c | 76 ++---- 40 files changed, 570 insertions(+), 648 deletions(-) create mode 100644 tests/qapi-schema/args-case-clash.err rename tests/qapi-schema/{union-clash-type.exit =3D> args-case-clash.exi= t} (100%) create mode 100644 tests/qapi-schema/args-case-clash.json rename tests/qapi-schema/{union-clash-type.out =3D> args-case-clash.out}= (100%) delete mode 100644 tests/qapi-schema/flat-union-clash-branch.err delete mode 100644 tests/qapi-schema/flat-union-clash-branch.exit delete mode 100644 tests/qapi-schema/flat-union-clash-branch.json delete mode 100644 tests/qapi-schema/flat-union-clash-branch.out delete mode 100644 tests/qapi-schema/flat-union-clash-type.err delete mode 100644 tests/qapi-schema/flat-union-clash-type.exit delete mode 100644 tests/qapi-schema/flat-union-clash-type.json delete mode 100644 tests/qapi-schema/flat-union-clash-type.out delete mode 100644 tests/qapi-schema/union-clash-type.err delete mode 100644 tests/qapi-schema/union-clash-type.json create mode 100644 tests/test-qmp-common.h --=20 2.4.3