[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 18/19] qapi: Fix excessive QAPISchemaEntity.chec
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 18/19] qapi: Fix excessive QAPISchemaEntity.check() recursion |
Date: |
Mon, 23 Sep 2019 14:01:05 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 9/14/19 10:35 AM, Markus Armbruster wrote:
>> Entity checking goes back to commit ac88219a6c "qapi: New QAPISchema
>> intermediate representation", v2.5.0. It's designed to work as
>> follows: QAPISchema.check() calls .check() for all the schema's
>> entities. An entity's .check() recurses into another entity's
>> .check() only if the C struct generated for the former contains the C
>> struct generated for the latter (pointers don't count). This is used
>> to detect "object contains itself".
>>
>> There are two instances of this:
>>
>> * An object's C struct contains its base's C struct
>>
>> QAPISchemaObjectType.check() calls self.base.check()
>>
>> * An object's C struct contains its variants' C structs
>>
>> QAPISchemaObjectTypeVariants().check calls v.type.check(). Since
>> commit b807a1e1e3 "qapi: Check for QAPI collisions involving variant
>> members", v2.6.0.
>>
>
>> Sadly, this design has since eroded:
>>
>
>> * A QAPISchemaEntity's .module becomes valid at .check(). This is
>> because computing it needs info and schema.fname, and because array
>> types get it from their element type instead.
>>
>> Make it a property just like .ifcond.
>
> Extra space.
Will fix.
>> Additionally, have QAPISchemaEntity.check() assert it gets called at
>> most once, so the design invariant will stick this time. Neglecting
>> that was entirely my fault.
>
> Lengthy explanation, but it makes sense.
Writing the commit message was more work than writing the patch...
>>
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>> scripts/qapi/common.py | 74 +++++++++++++++++++++++++++++-------------
>> 1 file changed, 52 insertions(+), 22 deletions(-)
>>
>
> Took me a bit longer to read this (and refresh myself on what
> '@property' actually does in Python, but you're not the first user of it
> in qemu), but it matches the commit message.
>
> Reviewed-by: Eric Blake <address@hidden>
Thanks!
- [Qemu-devel] [PATCH 08/19] qapi: Improve reporting of lexical errors, (continued)
- [Qemu-devel] [PATCH 08/19] qapi: Improve reporting of lexical errors, Markus Armbruster, 2019/09/14
- [Qemu-devel] [PATCH 11/19] qapi: Reject blank 'if' conditions in addition to empty ones, Markus Armbruster, 2019/09/14
- [Qemu-devel] [PATCH 06/19] tests/qapi-schema: Demonstrate suboptimal lexical errors, Markus Armbruster, 2019/09/14
- [Qemu-devel] [PATCH 04/19] tests/qapi-schema: Demonstrate broken discriminator errors, Markus Armbruster, 2019/09/14
- [Qemu-devel] [PATCH 18/19] qapi: Fix excessive QAPISchemaEntity.check() recursion, Markus Armbruster, 2019/09/14
- [Qemu-devel] [PATCH 19/19] qapi: Assert .visit() and .check_clash() run only after .check(), Markus Armbruster, 2019/09/14
- [Qemu-devel] [PATCH 16/19] qapi: Delete useless check_exprs() code for simple union kind, Markus Armbruster, 2019/09/14
- [Qemu-devel] [PATCH 15/19] qapi: Clean up around check_known_keys(), Markus Armbruster, 2019/09/14
- [Qemu-devel] [PATCH 17/19] qapi: Fix to .check() empty structs just once, Markus Armbruster, 2019/09/14
- [Qemu-devel] [PATCH 07/19] qapi: Use quotes more consistently in frontend error messages, Markus Armbruster, 2019/09/14