Am 25.02.2012 16:41, schrieb Michael Roth: > On Fri, Feb 24, 2012 at 11:22:06AM -0600, Anthony Liguori wrote: >> According to git bisect and qemu-test, this breaks: >> >> qemu-system-x86_64 -kernel bin/vmlinuz-3.0 -initrd >> .tmp-26227/initramfs-26227.img.gz -append console=ttyS0 seed=1498 >> -nographic -enable-kvm -device virtio-balloon-pci,id=balloon0 >> -pidfile .tmp-26227/pidfile-26227.pid -qmp >> unix:.tmp-26227/qmpsock-26227.sock,server,nowait >> qemu-system-x86_64: Parameter 'id' expects int8_t >> Aborted > > Sorry, put way too much faith in the unit tests catching this. > > The issue is we currently use set_int* for both uint* and int* > properties. In this case the default uint8_t property value was > (uint8_t)-1 = 255, which we'd stick in a qobject and feed to the > visitors. Before, we'd just read that back into an int64_t container and > let it be re-interpreted as -1 or 255 depending on the property type. > > Now, we still fall back to visit_type_int() for QmpInputVisitor, but in > the case of visit_type_int8() we check that the value falls within the > signed range, which isn't the case for 255. > > There's a few other places where we hit similar issues. The 2 possible > solutions are: > > 1) Loosen the range checks in qapi-visit-core.c so that we ignore > signedness and only check that (uintX_t)value is small enough to fit > in X bytes, or > > 2) Add set_uint*/get_uint* accessors for uint* qdev properties. > > 1 is less code, and more forgiving of cases were we might use int*/uint* > interchangeably, but 2 I think is more correct and tightens up the > bounds checking for qdev and whatever else we use QmpInputVisitor for.
I'm not too deep into visitors yet but 2) sounds better to me. I've seen a couple of places where command line args are not properly checked before passing them on (will send patches for those I remember) so having the checks close to where the values came from sounds good. Paolo did provide separate object_property_set_[u]int* accessors so we should be good in QOM land when not fiddling with these things at such a "deep" level. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg