On 04/14/2016 09:22 AM, Markus Armbruster wrote: > Eric Blake <ebl...@redhat.com> writes: > >> The visitor interface for mapping between QObject/QemuOpts/string >> and QAPI is scandalously under-documented, making changes to visitor >> core, individual visitors, and users of visitors difficult to >> coordinate. Among other questions: when is it safe to pass NULL, >> vs. when a string must be provided; which visitors implement which >> callbacks; the difference between concrete and virtual visits. >> >> Correct this by retrofitting proper contracts, and document where some >> of the interface warts remain (for example, we may want to modify >> visit_end_* to require the same 'obj' as the visit_start counterpart, >> so the dealloc visitor can be simplified). Later patches in this >> series will tackle some, but not all, of these warts. >> >> Add assertions to (partially) enforce the contract. Some of these >> were only made possible by recent cleanup commits. >> >> +/* >> + * The QAPI schema defines both a set of C data types, and a QMP wire >> + * format. A QAPI object is formed as a directed acyclic graph of >> + * QAPI values. > > I understand what you're trying to say, but I find the value / object > dichotomy odd. For me, A QAPI object isn't a DAG, it's a node in a DAG. > Perhaps: "QAPI objects can contain references to other QAPI objects, > resulting in a directed acyclic graph."
Thanks for a lot of good comments. I'm replying only to the questions
you left amidst all the good review.
>> +++ b/qapi/qapi-visit-core.c
>> @@ -23,6 +23,10 @@
>> void visit_start_struct(Visitor *v, const char *name, void **obj,
>> size_t size, Error **errp)
>> {
>> + if (obj) {
>> + assert(size);
>
> Yes, because the generator puts a dummy member into empty structs.
>
>> + assert(v->type != VISITOR_OUTPUT || *obj);
>
> Can you point me to the spot in the contract that requires this?
Translation of the assert: If you are using an output visitor, and not
doing a virtual walk (obj is non-NULL), then the object must be
completely built (*obj is non-NULL). For an input visitor, *obj is NULL
on entry (we're allocating it, after all); for the dealloc visitor, *obj
may or may not be NULL (since we handle cleanup of partial allocation).
In the text, "output visitors (QMP and string) take a completed QAPI
graph", but maybe I can further clarify that a completed object means
that *obj is non-NULL and all 'has_member' and 'member' members are
complete.
>
> Unlike last time, my remarks are pretty much only about how to say
> things, not about what to say. Progress!
Yay! Hopefully I'll post v15 soon and we can get this in at the start
of the 2.7 cycle.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
