On Mon, Mar 15, 2010 at 03:56:55PM +0100, Wolfgang Mauerer wrote:
Jim Meyering wrote:
Clang found something that might be a real bug. I suspect that ...drive.controller will always be at least one, it can - explanation below.
but we should not have to dive into the code trying to figure that out. It's easier/better here just to handle the potential trouble:
clang saw that if it *was* zero, then the following "for" loop would not be entered, and "cont" would not be initialized. On the very next statement "cont" (uninitialized) would be dereferenced. (...) * src/qemu/qemu_driver.c (qemudDomainAttachSCSIDisk): Handle the (theoretical) case of an empty controller list, so that clang does not think the subsequent dereference of "cont" would dereference an undefined variable (due to preceding loop not iterating even once). --- src/qemu/qemu_driver.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7f7c459..efb1857 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5671,18 +5671,24 @@ static int qemudDomainAttachSCSIDisk(struct qemud_driver *driver, (...) if (!(drivestr = qemuBuildDriveStr(disk, 0, qemuCmdFlags))) goto error;
+ if (disk->info.addr.drive.controller <= 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("no drive controller for %s"), disk->dst); + goto error; + } + for (i = 0 ; i <= disk->info.addr.drive.controller ; i++) { (...)
disk->info.addr.drive.controller does not denote the number of available controllers, but an index -- which can very well be zero, and the loop is always entered. Besides, checking for < 0 in the test does not make sense since _virDomainDeviceDriveAddress.controller is unsigned.
Since this commit breaks SCSI disk hotplug on controller 0, please revert it.
Agreed, this is definitely broken. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|