On Tue, Jun 28, 2016 at 03:03:15PM +0200, Michal Privoznik wrote:
On 28.06.2016 13:48, Daniel P. Berrange wrote:
On Tue, Jun 28, 2016 at 01:43:56PM +0200, Michal Privoznik wrote:
When working with typed params calling pure free() over an array of params is not enough. Some items in the array may be type of string in which case they have a pointer to an allocated memory too. Therefore we should use virTypedParamsFree() which does all the necessary.
Moreover, some methods (set_interface_parameters() and set_block_iotune()) were missing any free at all.
==16768== 1 bytes in 1 blocks are definitely lost in loss record 2 of 7,378 ==16768== at 0x4C29F80: malloc (vg_replace_malloc.c:296) ==16768== by 0x52603B9: strdup (strdup.c:42) ==16768== by 0x66222E4: virStrdup (in /usr/lib64/libvirt.so.0.2000.0) ==16768== by 0x662B92A: virTypedParamsDeserialize (in /usr/lib64/libvirt.so.0.2000.0) ==16768== by 0x671D7F2: remoteDomainGetNumaParameters (in /usr/lib64/libvirt.so.0.2000.0) ==16768== by 0x66CA1BE: virDomainGetNumaParameters (in /usr/lib64/libvirt.so.0.2000.0) ==16768== by 0x630CF74: XS_Sys__Virt__Domain_get_numa_parameters (Virt.xs:4224) ==16768== by 0x4EF920A: Perl_pp_entersub (in /usr/lib64/libperl.so.5.20.2) ==16768== by 0x4EF1A42: Perl_runops_standard (in /usr/lib64/libperl.so.5.20.2) ==16768== by 0x4E8BE58: perl_run (in /usr/lib64/libperl.so.5.20.2) ==16768== by 0x400D9A: main (in /usr/bin/perl)
==16768== 672 bytes in 1 blocks are definitely lost in loss record 6,993 of 7,378 ==16768== at 0x4C29F80: malloc (vg_replace_malloc.c:296) ==16768== by 0x4ED6984: Perl_safesysmalloc (in /usr/lib64/libperl.so.5.20.2) ==16768== by 0x63164DF: XS_Sys__Virt__Domain_set_interface_parameters (Virt.xs:5024) ==16768== by 0x4EF920A: Perl_pp_entersub (in /usr/lib64/libperl.so.5.20.2) ==16768== by 0x4EF1A42: Perl_runops_standard (in /usr/lib64/libperl.so.5.20.2) ==16768== by 0x4E8BE58: perl_run (in /usr/lib64/libperl.so.5.20.2) ==16768== by 0x400D9A: main (in /usr/bin/perl)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- Virt.xs | 90 +++++++++++++++++++++++++++++++++-------------------------------- 1 file changed, 46 insertions(+), 44 deletions(-)
Please mention which APIs had leaks fixed in the Changes file too.
diff --git a/Virt.xs b/Virt.xs index cf4e5bc..3b59f8a 100644 --- a/Virt.xs +++ b/Virt.xs @@ -2246,12 +2246,12 @@ get_node_memory_parameters(conn, flags=0) Newx(params, nparams, virTypedParameter);
if (virNodeGetMemoryParameters(conn, params, &nparams, flags) < 0) { - Safefree(params); + virTypedParamsFree(params, nparams);
This change is not safe, because 'Newx' is not guaranteed to correspond to a plain 'malloc' call. Any 'Newx' call *must* be matched by Safefree.
Ah, okay. Finding a good docs on perl C API is just painful.
FWIW, there are man pages which tell you most of the info, albeit in a fairly terse manner. 'perlapi' is the API reference in particular and says [quote] Newx The XSUB-writer's interface to the C "malloc" function. Memory obtained by this should ONLY be freed with "Safefree". [/quote] The 'perlguts' man page is more of a tutorial like thing describing Perl internals concepts. The 'perlxs' man page talks about the specific horrors of writing XS modules, aka perl extensions in C. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|