6:43 a.m.
I've noticed these while trying to write support for the new
libvirt APIs (but Dan was quicker :-)).
Michal Privoznik (3):
Prefer virTypedParameterPtr over virTypedParameter *
get/set_memory_parameters: Favour virTypedParameter over
virMemoryParameter
Substitute Safefree with virTypedParamsFree
Virt.xs | 146 ++++++++++++++++++++++++++++++++--------------------------------
1 file changed, 74 insertions(+), 72 deletions(-)
--
2.8.4
6:43 a.m.
New subject: [libvirt] [libvirt-perl][PATCH 1/3] Prefer virTypedParameterPtr over virTypedParameter *
There's no functional change, this is just a cosmetic fix.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
Virt.xs | 50 +++++++++++++++++++++++++-------------------------
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/Virt.xs b/Virt.xs
index aed7f6f..99ab150 100644
--- a/Virt.xs
+++ b/Virt.xs
@@ -159,7 +159,7 @@ _populate_constant_ull(HV *stash, const char *name, unsigned long long
val)
#define REGISTER_CONSTANT_ULL(name, key) _populate_constant_ull(stash, #key, name)
static HV *
-vir_typed_param_to_hv(virTypedParameter *params, int nparams)
+vir_typed_param_to_hv(virTypedParameterPtr params, int nparams)
{
HV *ret = (HV *)sv_2mortal((SV*)newHV());
unsigned int i;
@@ -210,7 +210,7 @@ vir_typed_param_to_hv(virTypedParameter *params, int nparams)
static int
-vir_typed_param_from_hv(HV *newparams, virTypedParameter *params, int nparams)
+vir_typed_param_from_hv(HV *newparams, virTypedParameterPtr params, int nparams)
{
unsigned int i;
char * ptr;
@@ -274,7 +274,7 @@ vir_typed_param_from_hv(HV *newparams, virTypedParameter *params, int
nparams)
static void
vir_typed_param_add_string_list_from_hv(HV *newparams,
- virTypedParameter **params,
+ virTypedParameterPtr *params,
int *nparams,
const char *key)
{
@@ -282,7 +282,7 @@ vir_typed_param_add_string_list_from_hv(HV *newparams,
return;
}
SSize_t nstr, i;
- virTypedParameter *localparams = *params;
+ virTypedParameterPtr localparams = *params;
SV **val = hv_fetch(newparams, key, strlen(key), 0);
AV *av = (AV*)(SvRV(*val));
@@ -2236,7 +2236,7 @@ get_node_memory_parameters(conn, flags=0)
virConnectPtr conn;
unsigned int flags;
PREINIT:
- virTypedParameter *params;
+ virTypedParameterPtr params;
int nparams;
CODE:
nparams = 0;
@@ -2262,7 +2262,7 @@ set_node_memory_parameters(conn, newparams, flags=0)
HV *newparams;
unsigned int flags;
PREINIT:
- virTypedParameter *params;
+ virTypedParameterPtr params;
int nparams;
PPCODE:
nparams = 0;
@@ -3918,7 +3918,7 @@ get_job_stats(dom, flags=0)
unsigned int flags;
PREINIT:
int type;
- virTypedParameter *params;
+ virTypedParameterPtr params;
int nparams;
HV *paramsHv;
SV *typeSv;
@@ -4017,7 +4017,7 @@ block_copy(dom, path, destxml, newparams, flags=0)
HV *newparams;
unsigned long flags;
PREINIT:
- virTypedParameter *params;
+ virTypedParameterPtr params;
int nparams;
PPCODE:
nparams = 3;
@@ -4092,7 +4092,7 @@ get_scheduler_parameters(dom, flags=0)
virDomainPtr dom;
unsigned int flags;
PREINIT:
- virTypedParameter *params;
+ virTypedParameterPtr params;
int nparams;
char *type;
CODE:
@@ -4124,7 +4124,7 @@ set_scheduler_parameters(dom, newparams, flags=0)
HV *newparams;
unsigned int flags;
PREINIT:
- virTypedParameter *params;
+ virTypedParameterPtr params;
int nparams;
char *type;
PPCODE:
@@ -4186,7 +4186,7 @@ set_memory_parameters(dom, newparams, flags=0)
HV *newparams;
unsigned int flags;
PREINIT:
- virTypedParameter *params;
+ virTypedParameterPtr params;
int nparams;
PPCODE:
nparams = 0;
@@ -4212,7 +4212,7 @@ get_numa_parameters(dom, flags=0)
virDomainPtr dom;
unsigned int flags;
PREINIT:
- virTypedParameter *params;
+ virTypedParameterPtr params;
int nparams;
CODE:
nparams = 0;
@@ -4238,7 +4238,7 @@ set_numa_parameters(dom, newparams, flags=0)
HV *newparams;
unsigned int flags;
PREINIT:
- virTypedParameter *params;
+ virTypedParameterPtr params;
int nparams;
PPCODE:
nparams = 0;
@@ -4264,7 +4264,7 @@ get_blkio_parameters(dom, flags=0)
virDomainPtr dom;
unsigned int flags;
PREINIT:
- virTypedParameter *params;
+ virTypedParameterPtr params;
int nparams;
CODE:
nparams = 0;
@@ -4290,7 +4290,7 @@ set_blkio_parameters(dom, newparams, flags=0)
HV *newparams;
unsigned int flags;
PREINIT:
- virTypedParameter *params;
+ virTypedParameterPtr params;
int nparams;
PPCODE:
nparams = 0;
@@ -4317,7 +4317,7 @@ get_perf_events(dom, flags=0)
virDomainPtr dom;
unsigned int flags;
PREINIT:
- virTypedParameter *params = NULL;
+ virTypedParameterPtr params = NULL;
int nparams = 0;
CODE:
if (virDomainGetPerfEvents(dom, ¶ms, &nparams, flags) < 0) {
@@ -4337,7 +4337,7 @@ set_perf_events(dom, newparams, flags=0)
HV *newparams;
unsigned int flags;
PREINIT:
- virTypedParameter *params = NULL;
+ virTypedParameterPtr params = NULL;
int nparams = 0;
PPCODE:
if (virDomainGetPerfEvents(dom, ¶ms, &nparams, flags) < 0) {
@@ -4448,7 +4448,7 @@ get_guest_vcpus(dom, flags=0)
virDomainPtr dom;
unsigned int flags;
PREINIT:
- virTypedParameter *params;
+ virTypedParameterPtr params;
unsigned int nparams;
CODE:
if (virDomainGetGuestVcpus(dom, ¶ms, &nparams, flags) < 0) {
@@ -4643,7 +4643,7 @@ _migrate(dom, destcon, newparams, flags=0)
HV *newparams;
unsigned long flags;
PREINIT:
- virTypedParameter *params;
+ virTypedParameterPtr params;
int nparams;
CODE:
nparams = 15;
@@ -4734,7 +4734,7 @@ _migrate_to_uri(dom, desturi, newparams, flags=0)
HV *newparams;
unsigned long flags;
PREINIT:
- virTypedParameter *params;
+ virTypedParameterPtr params;
int nparams;
PPCODE:
nparams = 15;
@@ -4937,7 +4937,7 @@ get_block_iotune(dom, disk, flags=0)
const char *disk;
unsigned int flags;
PREINIT:
- virTypedParameter *params;
+ virTypedParameterPtr params;
int nparams;
CODE:
nparams = 0;
@@ -4964,7 +4964,7 @@ set_block_iotune(dom, disk, newparams, flags=0)
HV *newparams;
unsigned int flags;
PREINIT:
- virTypedParameter *params;
+ virTypedParameterPtr params;
int nparams;
PPCODE:
nparams = 0;
@@ -4988,7 +4988,7 @@ get_interface_parameters(dom, intf, flags=0)
const char *intf;
unsigned int flags;
PREINIT:
- virTypedParameter *params;
+ virTypedParameterPtr params;
int nparams;
CODE:
nparams = 0;
@@ -5015,7 +5015,7 @@ set_interface_parameters(dom, intf, newparams, flags=0)
HV *newparams;
unsigned int flags;
PREINIT:
- virTypedParameter *params;
+ virTypedParameterPtr params;
int nparams;
PPCODE:
nparams = 0;
@@ -5040,7 +5040,7 @@ block_stats(dom, path, flags=0)
unsigned int flags;
PREINIT:
virDomainBlockStatsStruct stats;
- virTypedParameter *params;
+ virTypedParameterPtr params;
int nparams;
unsigned int i;
const char *field;
--
2.8.4
6:45 a.m.
New subject: [libvirt] [libvirt-perl][PATCH 1/3] Prefer virTypedParameterPtr over virTypedParameter *
On Tue, Jun 28, 2016 at 01:43:54PM +0200, Michal Privoznik wrote:
There's no functional change, this is just a cosmetic fix.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
Virt.xs | 50 +++++++++++++++++++++++++-------------------------
1 file changed, 25 insertions(+), 25 deletions(-)
ACK
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 :|
6:43 a.m.
New subject: [libvirt] [libvirt-perl][PATCH 2/3] get/set_memory_parameters: Favour virTypedParameter over virMemoryParameter
The libvirt-domain.h header file suggests using virTypedParameter
as virMemoryParameter is just ancient alias for it.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
Virt.xs | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Virt.xs b/Virt.xs
index 99ab150..cf4e5bc 100644
--- a/Virt.xs
+++ b/Virt.xs
@@ -4160,14 +4160,14 @@ get_memory_parameters(dom, flags=0)
virDomainPtr dom;
unsigned int flags;
PREINIT:
- virMemoryParameter *params;
+ virTypedParameterPtr params;
int nparams;
CODE:
nparams = 0;
if (virDomainGetMemoryParameters(dom, NULL, &nparams, flags) < 0)
_croak_error();
- Newx(params, nparams, virMemoryParameter);
+ Newx(params, nparams, virTypedParameter);
if (virDomainGetMemoryParameters(dom, params, &nparams, flags) < 0) {
Safefree(params);
@@ -4193,7 +4193,7 @@ set_memory_parameters(dom, newparams, flags=0)
if (virDomainGetMemoryParameters(dom, NULL, &nparams, flags) < 0)
_croak_error();
- Newx(params, nparams, virMemoryParameter);
+ Newx(params, nparams, virTypedParameter);
if (virDomainGetMemoryParameters(dom, params, &nparams, flags) < 0) {
Safefree(params);
--
2.8.4
6:45 a.m.
New subject: [libvirt] [libvirt-perl][PATCH 2/3] get/set_memory_parameters: Favour virTypedParameter over virMemoryParameter
On Tue, Jun 28, 2016 at 01:43:55PM +0200, Michal Privoznik wrote:
The libvirt-domain.h header file suggests using virTypedParameter
as virMemoryParameter is just ancient alias for it.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
Virt.xs | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
ACK
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 :|
6:43 a.m.
New subject: [libvirt] [libvirt-perl][PATCH 3/3] Substitute Safefree with virTypedParamsFree
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(a)redhat.com>
---
Virt.xs | 90 +++++++++++++++++++++++++++++++++--------------------------------
1 file changed, 46 insertions(+), 44 deletions(-)
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);
_croak_error();
}
RETVAL = vir_typed_param_to_hv(params, nparams);
- Safefree(params);
+ virTypedParamsFree(params, nparams);
OUTPUT:
RETVAL
@@ -2272,7 +2272,7 @@ set_node_memory_parameters(conn, newparams, flags=0)
Newx(params, nparams, virTypedParameter);
if (virNodeGetMemoryParameters(conn, params, &nparams, flags) < 0) {
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
@@ -2280,7 +2280,7 @@ set_node_memory_parameters(conn, newparams, flags=0)
if (virNodeSetMemoryParameters(conn, params, nparams, flags) < 0)
_croak_error();
- Safefree(params);
+ virTypedParamsFree(params, nparams);
@@ -3924,13 +3924,13 @@ get_job_stats(dom, flags=0)
SV *typeSv;
PPCODE:
if (virDomainGetJobStats(dom, &type, ¶ms, &nparams, flags) < 0)
{
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
typeSv = newSViv(type);
paramsHv = vir_typed_param_to_hv(params, nparams);
- Safefree(params);
+ virTypedParamsFree(params, nparams);
EXTEND(SP, 2);
PUSHs(newRV_noinc((SV*)typeSv));
@@ -4038,11 +4038,11 @@ block_copy(dom, path, destxml, newparams, flags=0)
nparams = vir_typed_param_from_hv(newparams, params, nparams);
if (virDomainBlockCopy(dom, path, destxml, params, nparams, flags) < 0) {
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
- Safefree(params);
+ virTypedParamsFree(params, nparams);
void
@@ -4103,17 +4103,17 @@ get_scheduler_parameters(dom, flags=0)
Newx(params, nparams, virTypedParameter);
if (flags) {
if (virDomainGetSchedulerParametersFlags(dom, params, &nparams, flags) <
0) {
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
} else {
if (virDomainGetSchedulerParameters(dom, params, &nparams) < 0) {
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
}
RETVAL = vir_typed_param_to_hv(params, nparams);
- Safefree(params);
+ virTypedParamsFree(params, nparams);
OUTPUT:
RETVAL
@@ -4135,12 +4135,12 @@ set_scheduler_parameters(dom, newparams, flags=0)
Newx(params, nparams, virTypedParameter);
if (flags) {
if (virDomainGetSchedulerParametersFlags(dom, params, &nparams, flags) <
0) {
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
} else {
if (virDomainGetSchedulerParameters(dom, params, &nparams) < 0) {
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
}
@@ -4152,7 +4152,7 @@ set_scheduler_parameters(dom, newparams, flags=0)
if (virDomainSetSchedulerParameters(dom, params, nparams) < 0)
_croak_error();
}
- Safefree(params);
+ virTypedParamsFree(params, nparams);
HV *
@@ -4170,12 +4170,12 @@ get_memory_parameters(dom, flags=0)
Newx(params, nparams, virTypedParameter);
if (virDomainGetMemoryParameters(dom, params, &nparams, flags) < 0) {
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
RETVAL = vir_typed_param_to_hv(params, nparams);
- Safefree(params);
+ virTypedParamsFree(params, nparams);
OUTPUT:
RETVAL
@@ -4196,7 +4196,7 @@ set_memory_parameters(dom, newparams, flags=0)
Newx(params, nparams, virTypedParameter);
if (virDomainGetMemoryParameters(dom, params, &nparams, flags) < 0) {
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
@@ -4204,7 +4204,7 @@ set_memory_parameters(dom, newparams, flags=0)
if (virDomainSetMemoryParameters(dom, params, nparams, flags) < 0)
_croak_error();
- Safefree(params);
+ virTypedParamsFree(params, nparams);
HV *
@@ -4222,12 +4222,12 @@ get_numa_parameters(dom, flags=0)
Newx(params, nparams, virTypedParameter);
if (virDomainGetNumaParameters(dom, params, &nparams, flags) < 0) {
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
RETVAL = vir_typed_param_to_hv(params, nparams);
- Safefree(params);
+ virTypedParamsFree(params, nparams);
OUTPUT:
RETVAL
@@ -4248,7 +4248,7 @@ set_numa_parameters(dom, newparams, flags=0)
Newx(params, nparams, virTypedParameter);
if (virDomainGetNumaParameters(dom, params, &nparams, flags) < 0) {
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
@@ -4256,7 +4256,7 @@ set_numa_parameters(dom, newparams, flags=0)
if (virDomainSetNumaParameters(dom, params, nparams, flags) < 0)
_croak_error();
- Safefree(params);
+ virTypedParamsFree(params, nparams);
HV *
@@ -4274,12 +4274,12 @@ get_blkio_parameters(dom, flags=0)
Newx(params, nparams, virBlkioParameter);
if (virDomainGetBlkioParameters(dom, params, &nparams, flags) < 0) {
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
RETVAL = vir_typed_param_to_hv(params, nparams);
- Safefree(params);
+ virTypedParamsFree(params, nparams);
OUTPUT:
RETVAL
@@ -4300,7 +4300,7 @@ set_blkio_parameters(dom, newparams, flags=0)
Newx(params, nparams, virBlkioParameter);
if (virDomainGetBlkioParameters(dom, params, &nparams, flags) < 0) {
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
@@ -4309,7 +4309,7 @@ set_blkio_parameters(dom, newparams, flags=0)
if (virDomainSetBlkioParameters(dom, params, nparams,
flags) < 0)
_croak_error();
- Safefree(params);
+ virTypedParamsFree(params, nparams);
HV *
@@ -4321,12 +4321,12 @@ get_perf_events(dom, flags=0)
int nparams = 0;
CODE:
if (virDomainGetPerfEvents(dom, ¶ms, &nparams, flags) < 0) {
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
RETVAL = vir_typed_param_to_hv(params, nparams);
- Safefree(params);
+ virTypedParamsFree(params, nparams);
OUTPUT:
RETVAL
@@ -4341,7 +4341,7 @@ set_perf_events(dom, newparams, flags=0)
int nparams = 0;
PPCODE:
if (virDomainGetPerfEvents(dom, ¶ms, &nparams, flags) < 0) {
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
@@ -4349,7 +4349,7 @@ set_perf_events(dom, newparams, flags=0)
if (virDomainSetPerfEvents(dom, params, nparams, flags) < 0)
_croak_error();
- Safefree(params);
+ virTypedParamsFree(params, nparams);
unsigned long
@@ -4719,10 +4719,10 @@ _migrate(dom, destcon, newparams, flags=0)
* if it is possible todo so
*/
if ((RETVAL = virDomainMigrate3(dom, destcon, params, nparams, flags)) == NULL) {
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
- Safefree(params);
+ virTypedParamsFree(params, nparams);
OUTPUT:
RETVAL
@@ -4810,10 +4810,10 @@ _migrate_to_uri(dom, desturi, newparams, flags=0)
* if it is possible todo so
*/
if (virDomainMigrateToURI3(dom, desturi, params, nparams, flags) < 0) {
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
- Safefree(params);
+ virTypedParamsFree(params, nparams);
void
@@ -4947,12 +4947,12 @@ get_block_iotune(dom, disk, flags=0)
Newx(params, nparams, virTypedParameter);
if (virDomainGetBlockIoTune(dom, disk, params, &nparams, flags) < 0) {
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
RETVAL = vir_typed_param_to_hv(params, nparams);
- Safefree(params);
+ virTypedParamsFree(params, nparams);
OUTPUT:
RETVAL
@@ -4973,13 +4973,14 @@ set_block_iotune(dom, disk, newparams, flags=0)
Newx(params, nparams, virTypedParameter);
if (virDomainGetBlockIoTune(dom, disk, params, &nparams, flags) < 0) {
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
nparams = vir_typed_param_from_hv(newparams, params, nparams);
if (virDomainSetBlockIoTune(dom, disk, params, nparams, flags) < 0)
_croak_error();
+ virTypedParamsFree(params, nparams);
HV *
@@ -4998,12 +4999,12 @@ get_interface_parameters(dom, intf, flags=0)
Newx(params, nparams, virTypedParameter);
if (virDomainGetInterfaceParameters(dom, intf, params, &nparams, flags) < 0)
{
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
RETVAL = vir_typed_param_to_hv(params, nparams);
- Safefree(params);
+ virTypedParamsFree(params, nparams);
OUTPUT:
RETVAL
@@ -5024,13 +5025,14 @@ set_interface_parameters(dom, intf, newparams, flags=0)
Newx(params, nparams, virTypedParameter);
if (virDomainGetInterfaceParameters(dom, intf, params, &nparams, flags) < 0)
{
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
nparams = vir_typed_param_from_hv(newparams, params, nparams);
if (virDomainSetInterfaceParameters(dom, intf, params, nparams, flags) < 0)
_croak_error();
+ virTypedParamsFree(params, nparams);
HV *
@@ -5066,7 +5068,7 @@ block_stats(dom, path, flags=0)
Newx(params, nparams, virTypedParameter);
if (virDomainBlockStatsFlags(dom, path, params, &nparams, flags) < 0) {
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
@@ -5086,7 +5088,7 @@ block_stats(dom, path, flags=0)
(void)hv_store(RETVAL, field, strlen(field), val, 0);
}
}
- Safefree(params);
+ virTypedParamsFree(params, nparams);
}
OUTPUT:
RETVAL
@@ -5312,7 +5314,7 @@ get_cpu_stats(dom, start_cpu, ncpus, flags=0)
Newx(params, ncpus * nparams, virTypedParameter);
if ((ret = virDomainGetCPUStats(dom, params, nparams, start_cpu, ncpus, flags))
< 0) {
- Safefree(params);
+ virTypedParamsFree(params, nparams);
_croak_error();
}
@@ -5322,7 +5324,7 @@ get_cpu_stats(dom, start_cpu, ncpus, flags=0)
PUSHs(newRV_noinc((SV *)rec));
}
- Safefree(params);
+ virTypedParamsFree(params, nparams);
void
--
2.8.4
6:48 a.m.
New subject: [libvirt] [libvirt-perl][PATCH 3/3] Substitute Safefree with virTypedParamsFree
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(a)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.
If libvirt allocated the typed params array, it is fine to
call virTypedParamsFree which I did for the recent virDomainGetGuestVCPUs
API, but for most existing APIs we can't do this.
You'll need to create a local SafeTypedParamsFree() which iterates over
each element free'ing strings, and then calling Safefree on the array
itself.
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 :|
8:03 a.m.
New subject: [libvirt] [libvirt-perl][PATCH 3/3] Substitute Safefree with virTypedParamsFree
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(a)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.
If libvirt allocated the typed params array, it is fine to
call virTypedParamsFree which I did for the recent virDomainGetGuestVCPUs
API, but for most existing APIs we can't do this.
Okay. Understood.
You'll need to create a local SafeTypedParamsFree() which iterates over
each element free'ing strings, and then calling Safefree on the array
itself.
Will do. Thanks for the review.
Michal
8:26 a.m.
New subject: [libvirt] [libvirt-perl][PATCH 3/3] Substitute Safefree with virTypedParamsFree
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(a)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 :|
3069
days inactive
3069
days old
8 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Michal Privoznik