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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list