Re: [Qemu-devel] [PATCH 00/16] introduce OptsVisitor, rebase -net/-netdev parsing

[Michael Roth <mdroth@linux.vnet.ibm.com>]

On Thu, Jun 07, 2012 at 01:29:54PM +0200, Laszlo Ersek wrote:
> On 06/06/12 22:09, Michael Roth wrote:
> > On Wed, Jun 06, 2012 at 06:49:19PM +0200, Laszlo Ersek wrote:
> 
> >> The fallback (*v->type_int)() call stores an int64_t, according to its
> >> prototype ("interface contract"). IMHO it shouldn't try to communicate a
> >> mathematical value outside of [INT64_MIN, INT64_MAX]; it should report
> > 
> > But the contract with visit_type_int() is maintained: it's just that
> > visit_type_uint64() is casting it's uint64_t value to int64_t (and
> > back) to make use of the fallback. It's slightly dirty, but fairly common
> > throughout the tree.
> 
> (I'm going theoretical :))
> 
> Sorry, I didn't mean what happens "around" the type_int method; I meant
> what happens inside it.
> 
> A visitor type takes some "external type" (a bag of data, structured or
> unstructured) and provides functions with scalar target types (among
> other things). One defines a native C struct in the JSON (... I'm making
> some leaps here), the generated code traverses that C type, and probes
> the "bag of data" with the corresponding visitor. A type_int call made
> to the visitor says "hey I need an int64_t for this node of the target C
> struct, with this 'path' and 'name' locator inside the external object".
> If the 'path' (= eg. visitor stack) and 'name' identify a piece of info
> in the external object that can't be represented in the requested target
> type, the visitor should report an error. I don't see much difference
> between the decimal representation of 2^63 and the string "donkey" in
> this regard if the target C "node" is an int64_t.
> 
> (Musing block ends :))

Yes, that's a good point, care must be taken when implementing a visitor to not
rely on on the fallback unless the serialized representation is compatible with
the original type. Otherwise an explicit interface for that type should
be implemented.

For QEMU <-> QEMU serialization/deserialization, there are actually unit
tests in qom-next that'll fail if this condition does not hold.

For QEMU <-> X serialization/deserialization, such as a visitor which
implements a wire encoding (QMP being the only example currently), we need to
take care that the wire encoding is compatible with the representation
expected by the other end (according to the QAPI schema or whatever other
means we use to document it). This holds for QMP/JSON, and we'll need to take
care that it holds for anything that's added in the future.

But in that case, the restriction is not a matter of whether a value
passed to visit_type_int() is within a certain range, but rather that
the encoding itself is compatible with the documented type, so enforcing
this constraint is a broader matter that cannot be checked generically, and
must be handled instead by unit tests which employ intimate knowledge of
the Visitor that's being tested, such as with tests/test-qmp-output-visitor.c.

> 
> Of course I'm fine with dropping the fourth hunk.
> 
> Thanks,
> Laszlo
>