[libvirt] [libvirt-perl][PATCH 3/3] Substitute Safefree with virTypedParamsFree

[Daniel P. Berrange]

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 at 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 :|