On Fri, Sep 12, 2014 at 10:57:26AM +0200, Michal Privoznik wrote: > Up till bb3301ba the wrapper was freeing the passed strings for us. > However that changed after the commit. So now we don't free any > strings which results in memory leaks as reported upstream [1]: > > ==14265== 2,407 bytes in 1 blocks are definitely lost in loss record 1,457 of 1,550 > ==14265== at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) > ==14265== by 0x5C46624: xdr_string (in /usr/lib64/libc-2.17.so) > ==14265== by 0xCFD9FCD: xdr_remote_nonnull_string (remote_protocol.c:31) > ==14265== by 0xCFDC2C8: xdr_remote_domain_get_xml_desc_ret (remote_protocol.c:1617) > ==14265== by 0xCFF0811: virNetMessageDecodePayload (virnetmessage.c:407) > ==14265== by 0xCFE68FB: virNetClientProgramCall (virnetclientprogram.c:379) > ==14265== by 0xCFBE8B1: callFull.isra.2 (remote_driver.c:6578) > ==14265== by 0xCFC7F04: remoteDomainGetXMLDesc (remote_driver.c:6600) > ==14265== by 0xCF8167C: virDomainGetXMLDesc (libvirt.c:4380) > ==14265== by 0xCC2C4DF: libvirt_virDomainGetXMLDesc (libvirt.c:1141) > ==14265== by 0x4F12B93: PyEval_EvalFrameEx (in /usr/lib64/libpython2.7.so.1.0) > ==14265== by 0x4F141AC: PyEval_EvalCodeEx (in /usr/lib64/libpython2.7.so.1.0) > > The python documentation clearly advise us to call free() [2]. From > an example in their docs: > > PyObject *res; > char *buf = (char *) malloc(BUFSIZ); /* for I/O */ > > if (buf == NULL) > return PyErr_NoMemory(); > ...Do some I/O operation involving buf... > res = PyString_FromString(buf); > free(buf); /* malloc'ed */ > return res; > > Moreover, instead of using VIR_FREE() (which we are not exporting), > I'll just go with bare free(). > > 1: https://www.redhat.com/archives/libvir-list/2014-September/msg00736.html > 2: https://docs.python.org/2/c-api/memory.html > > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > --- > > While the fix seems okay, for some reason I'm seeing another valgrind warning > after the fix is applied: > > ==21247== Invalid read of size 4 > ==21247== at 0x4EBFB93: PyObject_Free (in /usr/lib64/libpython2.7.so.1.0) > ==21247== by 0x4EB613E: insertdict_by_entry (in /usr/lib64/libpython2.7.so.1.0) > ==21247== by 0x4EB7BAF: dict_set_item_by_hash_or_entry (in /usr/lib64/libpython2.7.so.1.0) > ==21247== by 0x4EBC4CB: _PyModule_Clear (in /usr/lib64/libpython2.7.so.1.0) > ==21247== by 0x4F2916A: PyImport_Cleanup (in /usr/lib64/libpython2.7.so.1.0) > ==21247== by 0x4F34B0D: Py_Finalize (in /usr/lib64/libpython2.7.so.1.0) > ==21247== by 0x4F460A5: Py_Main (in /usr/lib64/libpython2.7.so.1.0) > ==21247== by 0x54462BF: (below main) (in /lib64/libc-2.19.so) > ==21247== Address 0x69be020 is 2,016 bytes inside a block of size 4,390 free'd > ==21247== at 0x4C2B5AC: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) > ==21247== by 0x7F1EC2D: libvirt_virDomainGetXMLDesc (in /home/zippy/work/libvirt/libvirt-python.git/build/lib.linux-x86_64-2.7/libvirtmod.so) > ==21247== by 0x4F180BB: PyEval_EvalFrameEx (in /usr/lib64/libpython2.7.so.1.0) > ==21247== by 0x4F199FF: PyEval_EvalCodeEx (in /usr/lib64/libpython2.7.so.1.0) > ==21247== by 0x4F18AB4: PyEval_EvalFrameEx (in /usr/lib64/libpython2.7.so.1.0) > ==21247== by 0x4F199FF: PyEval_EvalCodeEx (in /usr/lib64/libpython2.7.so.1.0) > ==21247== by 0x4F19AF8: PyEval_EvalCode (in /usr/lib64/libpython2.7.so.1.0) > ==21247== by 0x4F32EBE: run_mod (in /usr/lib64/libpython2.7.so.1.0) > ==21247== by 0x4F340E1: PyRun_FileExFlags (in /usr/lib64/libpython2.7.so.1.0) > ==21247== by 0x4F352E6: PyRun_SimpleFileExFlags (in /usr/lib64/libpython2.7.so.1.0) > ==21247== by 0x4F4670E: Py_Main (in /usr/lib64/libpython2.7.so.1.0) > ==21247== by 0x54462BF: (below main) (in /lib64/libc-2.19.so) > > This happens when using our examples/dominfo.py. > > > generator.py | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/generator.py b/generator.py > index a798274..8ee046a 100755 > --- a/generator.py > +++ b/generator.py > @@ -708,12 +708,16 @@ def print_function_wrapper(module, name, output, export, include): > else: > c_call = "\n c_retval = %s(%s);\n" % (name, c_call) > ret_convert = " py_retval = libvirt_%sWrap((%s) c_retval);\n" % (n,c) > + if n == "charPtr": > + ret_convert = ret_convert + " free(c_retval);\n" > ret_convert = ret_convert + " return py_retval;\n" > elif ret[0] in py_return_types: > (f, t, n, c) = py_return_types[ret[0]] > c_return = " %s c_retval;\n" % (ret[0]) > c_call = "\n c_retval = %s(%s);\n" % (name, c_call) > ret_convert = " py_retval = libvirt_%sWrap((%s) c_retval);\n" % (n,c) > + if n == "charPtr": > + ret_convert = ret_convert + " free(c_retval);\n" > ret_convert = ret_convert + " return py_retval;\n" > else: > if ret[0] in skipped_types: 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list