<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
<title></title>
</head>
<body bgcolor="#ffffff" text="#000000">
On 01/06/2012 04:36 PM, Osier Yang wrote:
<blockquote cite="mid:4F06B2AB.4020705@redhat.com" type="cite">On
2011年12月30日 14:57, <a class="moz-txt-link-abbreviated" href="mailto:ajia@redhat.com">ajia@redhat.com</a> wrote:
<br>
<blockquote type="cite">From: Alex Jia<a class="moz-txt-link-rfc2396E" href="mailto:ajia@redhat.com"><ajia@redhat.com></a>
<br>
<br>
The parameter 'device_weight' is a string, however, the
'VIR_TYPED_PARAM_STRING'
<br>
type condition is missed by libvirt_virDomain{Set,
Get}BlkioParameters bindings,
<br>
the result is we can't get or change 'device_weight' value.
<br>
<br>
* python/libvirt-override.c: Add missing
'VIR_TYPED_PARAM_STRING' condition into
<br>
libvirt_virDomain{Set, Get}BlkioParameters bindings and free
allocated memory.
<br>
<br>
<a class="moz-txt-link-freetext" href="https://bugzilla.redhat.com/show_bug.cgi?id=770795">https://bugzilla.redhat.com/show_bug.cgi?id=770795</a>
<br>
<br>
<br>
Signed-off-by: Alex Jia<a class="moz-txt-link-rfc2396E" href="mailto:ajia@redhat.com"><ajia@redhat.com></a>
<br>
---
<br>
python/libvirt-override.c | 20 ++++++++++++++++++++
<br>
1 files changed, 20 insertions(+), 0 deletions(-)
<br>
<br>
diff --git a/python/libvirt-override.c
b/python/libvirt-override.c
<br>
index d2aad0f..38a35b8 100644
<br>
--- a/python/libvirt-override.c
<br>
+++ b/python/libvirt-override.c
<br>
@@ -726,6 +726,10 @@
libvirt_virDomainSetBlkioParameters(PyObject *self
ATTRIBUTE_UNUSED,
<br>
}
<br>
break;
<br>
<br>
+ case VIR_TYPED_PARAM_STRING:
<br>
+ params[i].value.s = PyString_AsString(val);
<br>
+ break;
<br>
+
<br>
default:
<br>
free(params);
<br>
return VIR_PY_INT_FAIL;
<br>
@@ -735,6 +739,11 @@
libvirt_virDomainSetBlkioParameters(PyObject *self
ATTRIBUTE_UNUSED,
<br>
LIBVIRT_BEGIN_ALLOW_THREADS;
<br>
i_retval = virDomainSetBlkioParameters(domain, params,
nparams, flags);
<br>
LIBVIRT_END_ALLOW_THREADS;
<br>
+
<br>
+ for(i=0; i< nparams; i++)
<br>
+ if (params[i].type == VIR_TYPED_PARAM_STRING)
<br>
+ free(params[i].value.s);
<br>
</blockquote>
<br>
IMHO this is not right, anything returned by PyString_AsString
should
<br>
*not* be deallocated. It doesn't returns a copy but a pointer
reference.
<br>
Note that Python still wants to see "params[i].value.s", and I
guess
<br>
Python will be responsiable to do the cleanup. But I'm not quite
<br>
convinced about that, not formiliar with Python's memory
management.
<br>
</blockquote>
Yeah, I know python.org says "PyString_AsString() <tt
class="descname"><span class="highlight"></span></tt>must not be
deallocated, because its <br>
pointer
refers to the internal buffer of string<em></em>, not a copy." in
here, I want o free the contents <br>
of string, may be free(val) is more clear.<br>
<blockquote cite="mid:4F06B2AB.4020705@redhat.com" type="cite">
<br>
<blockquote type="cite">+
<br>
if (i_retval< 0) {
<br>
free(params);
<br>
return VIR_PY_INT_FAIL;
<br>
@@ -811,6 +820,10 @@
libvirt_virDomainGetBlkioParameters(PyObject *self
ATTRIBUTE_UNUSED,
<br>
val = PyBool_FromLong((long)params[i].value.b);
<br>
break;
<br>
<br>
+ case VIR_TYPED_PARAM_STRING:
<br>
+ val = PyString_FromString((char
*)params[i].value.s);
<br>
+ break;
<br>
+
<br>
default:
<br>
free(params);
<br>
Py_DECREF(info);
<br>
@@ -819,7 +832,14 @@
libvirt_virDomainGetBlkioParameters(PyObject *self
ATTRIBUTE_UNUSED,
<br>
<br>
key = libvirt_constcharPtrWrap(params[i].field);
<br>
PyDict_SetItem(info, key, val);
<br>
+ Py_DECREF(key);
<br>
+ Py_DECREF(val);
<br>
</blockquote>
<br>
Why it needs to descrease the reference count of "key" and "val"?
<br>
</blockquote>
PyDict_SetItem() puts something in a dictionary is "storing" it,
therefore PyDict_SetItem() INCREFs both the key and the value.
<br>
<blockquote cite="mid:4F06B2AB.4020705@redhat.com" type="cite">
<br>
<blockquote type="cite"> }
<br>
+
<br>
+ for(i=0; i< nparams; i++)
<br>
+ if (params[i].type == VIR_TYPED_PARAM_STRING)
<br>
+ free(params[i].value.s);
<br>
+
<br>
</blockquote>
<br>
params[i].value.s also needs to be free()'ed in default branch,
before
<br>
the function returns.
<br>
</blockquote>
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 :)<br>
<br>
Thanks for your comments again,<br>
Alex<br>
<code></code><code></code>
<blockquote cite="mid:4F06B2AB.4020705@redhat.com" type="cite">
<br>
<blockquote type="cite"> free(params);
<br>
return(info);
<br>
}
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>