Le Tuesday 17 June 2014 11:24:09, Peter Krempa a écrit :
> In virVBoxSnapshotConfRemoveFakeDisks and
> virVBoxSnapshotConfDiskIsInMediaRegistry the disk array constructed from
> all the disks would be leaked at the end of the function and on
> allocation errors. Also the temporary disk list would be leaked.
>
> Add a cleanup section and free the memory properly.
>
> Found by coverity.
> ---
> src/vbox/vbox_snapshot_conf.c | 54
> +++++++++++++++++++++++++++++++++++-------- 1 file changed, 44
> insertions(+), 10 deletions(-)
>
> diff --git a/src/vbox/vbox_snapshot_conf.c b/src/vbox/vbox_snapshot_conf.c
> index 69c7e42..49fd2b6 100644
> --- a/src/vbox/vbox_snapshot_conf.c
> +++ b/src/vbox/vbox_snapshot_conf.c
> @@ -1471,28 +1471,45 @@
> virVBoxSnapshotConfRemoveFakeDisks(virVBoxSnapshotConfMachinePtr machine)
> size_t diskSize = 0;
> virVBoxSnapshotConfHardDiskPtr *tempList = NULL;
> virVBoxSnapshotConfHardDiskPtr *diskList = NULL;
> +
> if (VIR_ALLOC_N(diskList, 0) < 0)
> - return ret;
> + return -1;
>
> for (i = 0; i < machine->mediaRegistry->ndisks; i++) {
> tempSize =
> virVBoxSnapshotConfAllChildren(machine->mediaRegistry->disks[i],
> &tempList); if (VIR_EXPAND_N(diskList, diskSize, tempSize) < 0)
> - return ret;
> - for (j = 0; j < tempSize; j++) {
> + goto cleanup;
> +
> + for (j = 0; j < tempSize; j++)
> diskList[diskSize - tempSize + j] = tempList[j];
> - }
> +
> + VIR_FREE(tempList);
> }
> +
> for (i = 0; i < diskSize; i++) {
> if (strstr(diskList[i]->location, "fake") != NULL) {
> if (virVBoxSnapshotConfRemoveHardDisk(machine->mediaRegistry,
> diskList[i]->uuid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Unable to remove hard disk %s from media
> registry"), diskList[i]->location);
> - return ret;
> + goto cleanup;
> }
> }
> }
> +
> ret = 0;
> +
> + cleanup:
> + for (i = 0; i < diskSize; i++)
> + virVboxSnapshotConfHardDiskFree(diskList[i]);
> + VIR_FREE(diskList);
> +
> + if (tempList) {
> + for (i = 0; i < tempSize; i++)
> + virVboxSnapshotConfHardDiskFree(tempList[i]);
> + VIR_FREE(tempList);
> + }
Same comments than the previous patch
> +
> return ret;
> }
>
> @@ -1513,24 +1530,41 @@
> virVBoxSnapshotConfDiskIsInMediaRegistry(virVBoxSnapshotConfMachinePtr
> machine, size_t diskSize = 0;
> virVBoxSnapshotConfHardDiskPtr *tempList = NULL;
> virVBoxSnapshotConfHardDiskPtr *diskList = NULL;
> +
> if (VIR_ALLOC_N(diskList, 0) < 0)
> - return ret;
> + return -1;
>
> for (i = 0; i < machine->mediaRegistry->ndisks; i++) {
> tempSize =
> virVBoxSnapshotConfAllChildren(machine->mediaRegistry->disks[i],
> &tempList); if (VIR_EXPAND_N(diskList, diskSize, tempSize) < 0)
> - return ret;
> - for (j = 0; j < tempSize; j++) {
> + goto cleanup;
> +
> + for (j = 0; j < tempSize; j++)
> diskList[diskSize - tempSize + j] = tempList[j];
> - }
> +
> + VIR_FREE(tempList);
> }
> +
> for (i = 0; i < diskSize; i++) {
> if (STREQ(diskList[i]->location, location)) {
> ret = 1;
> - return ret;
> + goto cleanup;
> }
> }
> +
> ret = 0;
> +
> + cleanup:
> + for (i = 0; i < diskSize; i++)
> + virVboxSnapshotConfHardDiskFree(diskList[i]);
> + VIR_FREE(diskList);
> +
> + if (tempList) {
> + for (i = 0; i < tempSize; i++)
> + virVboxSnapshotConfHardDiskFree(tempList[i]);
> + VIR_FREE(tempList);
> + }
Same comments than the previous patch
> +
> return ret;
> }
--
Yohan BELLEGUIC
Software Engineer - diateam : Architectes de l'information
Phone : +33 (0)2 98 050 050 Fax : +33 (0)2 98 050 05
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list