[libvirt] [PATCH 2/3] vbox: snapshot: Avoid memleak in virVBoxSnapshotConfAllChildren

Yohan Belleguic yohan.belleguic at diateam.net
Tue Jun 17 09:49:53 UTC 2014


Le Tuesday 17 June 2014 11:24:08, Peter Krempa a écrit :
> On re-allocation failure the function would leak already allocated
> memory.
> ---
>  src/vbox/vbox_snapshot_conf.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/src/vbox/vbox_snapshot_conf.c b/src/vbox/vbox_snapshot_conf.c
> index 30ac6fe..69c7e42 100644
> --- a/src/vbox/vbox_snapshot_conf.c
> +++ b/src/vbox/vbox_snapshot_conf.c
> @@ -483,18 +483,24 @@
> virVBoxSnapshotConfAllChildren(virVBoxSnapshotConfHardDiskPtr disk, for (i
> = 0; i < disk->nchildren; i++) {
>          tempSize = virVBoxSnapshotConfAllChildren(disk->children[i],
> &tempList); if (VIR_EXPAND_N(ret, returnSize, tempSize) < 0)
> -            return 0;
> +            goto error;
> 
>          for (j = 0; j < tempSize; j++) {
>              ret[returnSize - tempSize + j] = tempList[j];
>          }
>      }
>      if (VIR_EXPAND_N(ret, returnSize, 1) < 0)
> -        return 0;
> +        goto error;
> 
>      ret[returnSize - 1] = disk;
>      *list = ret;
>      return returnSize;
> +
> + error:
> +    for (i = 0; i < returnSize; i++)
> +        virVboxSnapshotConfHardDiskFree(ret[i]);
> +    VIR_FREE(ret);
> +    return 0;
>  }

I think we don't have to use virVboxSnapshotConfHardDiskFree because there is 
not memory allocation in this function. It's just a pointer recopy, so if the 
disks are freed, this might lead to a segfault or a bad behaviour.
> 
>  void

-- 
Yohan BELLEGUIC
Software Engineer - diateam : Architectes de l'information
Phone : +33 (0)2 98 050 050 Fax : +33 (0)2 98 050 05




More information about the libvir-list mailing list