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