Re: [libvirt] [PATCH 1/2] vbox_storage: fix coverity issue with overwriting value

On 10/31/2014 12:45 PM, Pavel Hrdina wrote: > Coverity is complaining about overwriting value in 'rc' variable > without using the old value because it somehow doesn't recognize that > the value is used by MACRO. The 'rc' variable is there only for checking > return code so it's save to remove it and make coverity happy. > > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; > --- > src/vbox/vbox_storage.c | 36 ++++++++++++++++-------------------- > 1 file changed, 16 insertions(+), 20 deletions(-) > > diff --git a/src/vbox/vbox_storage.c b/src/vbox/vbox_storage.c > index 3610a35..0cf7a33 100644 > --- a/src/vbox/vbox_storage.c > +++ b/src/vbox/vbox_storage.c > @@ -551,7 +551,6 @@ static int vboxStorageVolDelete(virStorageVolPtr vol, unsigned int flags) > PRUint32 machineIdsSize = 0; > vboxArray machineIds = VBOX_ARRAY_INITIALIZER; > vboxIIDUnion hddIID; > - nsresult rc; > int ret = -1; > > if (!data->vboxObj) { > @@ -568,8 +567,9 @@ static int vboxStorageVolDelete(virStorageVolPtr vol, unsigned int flags) > } > > vboxIIDFromUUID(&hddIID, uuid); > - rc = gVBoxAPI.UIVirtualBox.GetHardDiskByIID(data->vboxObj, &hddIID, &hardDisk); > - if (NS_FAILED(rc)) > + if (NS_FAILED(gVBoxAPI.UIVirtualBox.GetHardDiskByIID(data->vboxObj, > + &hddIID, > + &hardDisk))) > goto cleanup; > > gVBoxAPI.UIMedium.GetState(hardDisk, &hddstate); > @@ -603,23 +603,22 @@ static int vboxStorageVolDelete(virStorageVolPtr vol, unsigned int flags) > vboxIIDFromArrayItem(&machineId, &machineIds, i); > > if (gVBoxAPI.getMachineForSession) { > - rc = gVBoxAPI.UIVirtualBox.GetMachine(data->vboxObj, &machineId, &machine); > - if (NS_FAILED(rc)) { > + if (NS_FAILED(gVBoxAPI.UIVirtualBox.GetMachine(data->vboxObj, > + &machineId, > + &machine))) { > virReportError(VIR_ERR_NO_DOMAIN, "%s", > _("no domain with matching uuid")); > break; > } > } > > - rc = gVBoxAPI.UISession.Open(data, &machineId, machine); > - > - if (NS_FAILED(rc)) { > + if (NS_FAILED(gVBoxAPI.UISession.Open(data, &machineId, machine))) { > vboxIIDUnalloc(&machineId); > continue; > } > > - rc = gVBoxAPI.UISession.GetMachine(data->vboxSession, &machine); > - if (NS_FAILED(rc)) > + if (NS_FAILED(gVBoxAPI.UISession.GetMachine(data->vboxSession, > + &machine))) > goto cleanupLoop; > > gVBoxAPI.UArray.vboxArrayGet(&hddAttachments, machine, > @@ -633,13 +632,12 @@ static int vboxStorageVolDelete(virStorageVolPtr vol, unsigned int flags) > if (!hddAttachment) > continue; > > - rc = gVBoxAPI.UIMediumAttachment.GetMedium(hddAttachment, &hdd); > - if (NS_FAILED(rc) || !hdd) > + if (NS_FAILED(gVBoxAPI.UIMediumAttachment.GetMedium(hddAttachment, > + &hdd)) || !hdd) > continue; > > VBOX_IID_INITIALIZE(&iid); > - rc = gVBoxAPI.UIMedium.GetId(hdd, &iid); > - if (NS_FAILED(rc)) { > + if (NS_FAILED(gVBoxAPI.UIMedium.GetId(hdd, &iid))) { > VBOX_MEDIUM_RELEASE(hdd); > continue; > } > @@ -658,9 +656,8 @@ static int vboxStorageVolDelete(virStorageVolPtr vol, unsigned int flags) > gVBoxAPI.UIMediumAttachment.GetPort(hddAttachment, &port); > gVBoxAPI.UIMediumAttachment.GetDevice(hddAttachment, &device); > > - rc = gVBoxAPI.UIMachine.DetachDevice(machine, controller, port, device); > - if (NS_SUCCEEDED(rc)) { > - rc = gVBoxAPI.UIMachine.SaveSettings(machine); > + if (NS_SUCCEEDED(gVBoxAPI.UIMachine.DetachDevice(machine, controller, port, device))) { > + ignore_value(gVBoxAPI.UIMachine.SaveSettings(machine)); While this does work (eg, the ignore_value), is that the intention? I asked the author : http://www.redhat.com/archives/libvir-list/2014-October/msg01050.html if the intention was to ignore the 'rc' value or not? The question being of course if the SaveSettings fails, should the rest of the code be executed? I don't know what to expect in vBox.. As painful as it is with the daily failing Coverity tests - I was hoping we could get an answer there first.

John > VIR_DEBUG("saving machine settings"); > deregister++; > VIR_DEBUG("deregistering hdd:%d", deregister); > @@ -683,9 +680,8 @@ static int vboxStorageVolDelete(virStorageVolPtr vol, unsigned int flags) > > if (machineIdsSize == 0 || machineIdsSize == deregister) { > IProgress *progress = NULL; > - rc = gVBoxAPI.UIHardDisk.DeleteStorage(hardDisk, &progress); > - > - if (NS_SUCCEEDED(rc) && progress) { > + if (NS_SUCCEEDED(gVBoxAPI.UIHardDisk.DeleteStorage(hardDisk, &progress)) && > + progress) { > gVBoxAPI.UIProgress.WaitForCompletion(progress, -1); > VBOX_RELEASE(progress); > DEBUGIID("HardDisk deleted, UUID", &hddIID); > -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list