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

["Andreas Färber" <afaerber@suse.de>]

Am 06.06.2012 22:09, schrieb Michael Roth:
> 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 :)
[snip]

I've squashed the first three hunks on my qom-next-1 branch and am
listening what comes out of the discussion for the fourth one.

Do we have an actual example where this discussion makes a difference?
The RAM "size" property for sun4m and sun4u seem to be the only uses of
DEFINE_PROP_UINT64(). For anything else I haven't seen patches yet, so I
think we could defer that to a follow-up patch? I'd like to have
qom-next pulled sooner rather than later. ;-)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg