On 01/06/2012 04:36 PM, Osier Yang wrote:
On
2011年12月30日 14:57, ajia@xxxxxxxxxx wrote:
From: Alex Jia<ajia@xxxxxxxxxx>
The parameter 'device_weight' is a string, however, the
'VIR_TYPED_PARAM_STRING'
type condition is missed by libvirt_virDomain{Set,
Get}BlkioParameters bindings,
the result is we can't get or change 'device_weight' value.
* python/libvirt-override.c: Add missing
'VIR_TYPED_PARAM_STRING' condition into
libvirt_virDomain{Set, Get}BlkioParameters bindings and free
allocated memory.
https://bugzilla.redhat.com/show_bug.cgi?id=770795
Signed-off-by: Alex Jia<ajia@xxxxxxxxxx>
---
python/libvirt-override.c | 20 ++++++++++++++++++++
1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/python/libvirt-override.c
b/python/libvirt-override.c
index d2aad0f..38a35b8 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -726,6 +726,10 @@
libvirt_virDomainSetBlkioParameters(PyObject *self
ATTRIBUTE_UNUSED,
}
break;
+ case VIR_TYPED_PARAM_STRING:
+ params[i].value.s = PyString_AsString(val);
+ break;
+
default:
free(params);
return VIR_PY_INT_FAIL;
@@ -735,6 +739,11 @@
libvirt_virDomainSetBlkioParameters(PyObject *self
ATTRIBUTE_UNUSED,
LIBVIRT_BEGIN_ALLOW_THREADS;
i_retval = virDomainSetBlkioParameters(domain, params,
nparams, flags);
LIBVIRT_END_ALLOW_THREADS;
+
+ for(i=0; i< nparams; i++)
+ if (params[i].type == VIR_TYPED_PARAM_STRING)
+ free(params[i].value.s);
IMHO this is not right, anything returned by PyString_AsString
should
*not* be deallocated. It doesn't returns a copy but a pointer
reference.
Note that Python still wants to see "params[i].value.s", and I
guess
Python will be responsiable to do the cleanup. But I'm not quite
convinced about that, not formiliar with Python's memory
management.
Yeah, I know python.org says "PyString_AsString() must not be
deallocated, because its
pointer
refers to the internal buffer of string, not a copy." in
here, I want o free the contents
of string, may be free(val) is more clear.
+
if (i_retval< 0) {
free(params);
return VIR_PY_INT_FAIL;
@@ -811,6 +820,10 @@
libvirt_virDomainGetBlkioParameters(PyObject *self
ATTRIBUTE_UNUSED,
val = PyBool_FromLong((long)params[i].value.b);
break;
+ case VIR_TYPED_PARAM_STRING:
+ val = PyString_FromString((char
*)params[i].value.s);
+ break;
+
default:
free(params);
Py_DECREF(info);
@@ -819,7 +832,14 @@
libvirt_virDomainGetBlkioParameters(PyObject *self
ATTRIBUTE_UNUSED,
key = libvirt_constcharPtrWrap(params[i].field);
PyDict_SetItem(info, key, val);
+ Py_DECREF(key);
+ Py_DECREF(val);
Why it needs to descrease the reference count of "key" and "val"?
PyDict_SetItem() puts something in a dictionary is "storing" it,
therefore PyDict_SetItem() INCREFs both the key and the value.
}
+
+ for(i=0; i< nparams; i++)
+ if (params[i].type == VIR_TYPED_PARAM_STRING)
+ free(params[i].value.s);
+
params[i].value.s also needs to be free()'ed in default branch,
before
the function returns.
I don't think so, if 'default' branch is hit, it means
"params[i].type != VIR_TYPED_PARAM_STRING", in other words, codes
haven't called 'PyString_FromString()' method, hence we don't need
to free memory, please correct me if I'm wrong :)
Thanks for your comments again,
Alex
free(params);
return(info);
}
|