Re: [Qemu-devel] [PATCH 18/19] qapi: Fix excessive QAPISchemaEntity.check() recursion

[Eric Blake]

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.

> 
> 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.

> 
> 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>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature