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

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

On Wed, Jun 06, 2012 at 06:49:19PM +0200, Laszlo Ersek wrote:
> On 06/06/12 18:14, Michael Roth wrote:
> > On Wed, Jun 06, 2012 at 05:30:03PM +0200, Laszlo Ersek wrote:
> 
> >>   value < 0
> > 
> > I think this last one will cause problems though, since uint64_t's
> > within the valid range for visit_type_uint64() will fail due to being
> > interpreted as < 0 when cast to int64_t. With the others we can detect
> > these cases since the max unsigned value doesn't exceed the max signed
> > value of the intermediate type we're storing to.
> 
> 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.

> an error in this case. If a visitor genuinely wants to parse and store
> 2^63 for example, it should implement the "type_uint64" interface.

This doesn't buy us much, since ultimately that type_uint64 interface is
just gonna, in the case of QMP*Visitor anyway, end up casting back/forth
between a int64_t anyway so we can stick it into a QObject as a Qint.

So really we end up doing the same thing as before, except we do it in
more places.

> 
> Anyway I've been called "inflexible" before. I can send a v2 if you wish.

Your offer to send a v2 suggests otherwise :)

> 
> Thanks,
> Laszlo
>