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