Re: [PATCH libvirt-python 06/17] Remove use of VIR_FREE from code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/09/2013 10:01 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
> 
> We don't have access to the libvirt memory APIs to replace

s/to replace/so replace/

> VIR_FREE with free().
> 
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
>  libvirt-lxc-override.c  |   4 +-
>  libvirt-override.c      | 228 ++++++++++++++++++++++++------------------------
>  libvirt-qemu-override.c |   4 +-
>  typewrappers.c          |   4 +-
>  4 files changed, 120 insertions(+), 120 deletions(-)

Most of your conversions occur right before a variable goes out of
scope, and are therefore safe.  However...

> @@ -2482,13 +2482,13 @@ libvirt_virDomainSnapshotListNames(PyObject *self,
>              py_retval = NULL;
>              goto cleanup;
>          }
> -        VIR_FREE(names[i]);
> +        free(names[i]);
>      }
>  
>  cleanup:
>      for (i = 0; i < c_retval; i++)
> -        VIR_FREE(names[i]);
> -    VIR_FREE(names);
> +        free(names[i]);

Ouch - double free.  VIR_FREE(names[i]) set names[i] to NULL, so that
even if we abort early, the cleanup will fix the remaining array entries
without choking on the already freed entries in the first half of the
array.  But with your rewrite, since you fail to set to NULL, an early
abort will call free(names[0]) twice if we fail during the i==1 iteration.

> @@ -2583,13 +2583,13 @@ libvirt_virDomainSnapshotListChildrenNames(PyObject *self,
>              py_retval = NULL;
>              goto cleanup;
>          }
> -        VIR_FREE(names[i]);
> +        free(names[i]);
>      }
>  
>  cleanup:
>      for (i = 0; i < c_retval; i++)
> -        VIR_FREE(names[i]);
> -    VIR_FREE(names);
> +        free(names[i]);

and again.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]